From: Paul Eggert <eggert@cs.ucla.edu>
Date: Sat, 2 Apr 2005 06:39:19 +0000 (+0000)
Subject: (decode_record): Don't dump core when given
X-Git-Url: https://git.brokenzipper.com/gitweb?a=commitdiff_plain;h=95fa3422d7365bfec419cb6537da6d9e0efbab83;p=chaz%2Ftar

(decode_record): Don't dump core when given
a corrupted extended header.  Problem reported by Jim Meyering.
Also, check for other ways that the header might be invalid,
e.g., missing newline at end.  Do not allow keys with nulls.
Allow blanks before and after length, as POSIX requires.
Do not allow leading "-" in length.  Check for length overflow.
(xheader_decode, xheader_decode_global): Let decode_record
check for exhaustion of record.
(xheader_read): Null-terminate the extended record;
decode_record relies on this.
---

diff --git a/src/xheader.c b/src/xheader.c
index 0893bb3..0cb5bb2 100644
--- a/src/xheader.c
+++ b/src/xheader.c
@@ -1,6 +1,6 @@
 /* POSIX extended headers for tar.
 
-   Copyright (C) 2003, 2004 Free Software Foundation, Inc.
+   Copyright (C) 2003, 2004, 2005 Free Software Foundation, Inc.
 
    This program is free software; you can redistribute it and/or modify it
    under the terms of the GNU General Public License as published by the
@@ -463,51 +463,70 @@ xheader_protected_keyword_p (const char *keyword)
   return false;
 }
 
-/* Decodes a single extended header record. Advances P to the next
-   record.
-   Returns true on success, false otherwise. */
+/* Decode a single extended header record, advancing *PTR to the next record.
+   Return true on success, false otherwise.  */
 static bool
-decode_record (char **p,
+decode_record (char **ptr,
 	       void (*handler) (void *, char const *, char const *),
 	       void *data)
 {
-  size_t len;
+  char *start = *ptr;
+  char *p = start;
+  unsigned long int len;
+  char *len_lim;
   char const *keyword;
-  char *start = *p;
-  char endc;
+  char *nextp;
+  size_t len_max = extended_header.buffer + extended_header.size - start;
 
-  if (**p == 0)
-    return false;
+  while (*p == ' ' || *p == '\t')
+    p++;
 
-  len = strtoul (*p, p, 10);
-  if (**p != ' ')
+  if (! ISDIGIT (*p))
     {
-      ERROR ((0, 0,
-       _("Malformed extended header: missing whitespace after the length")));
+      if (*p)
+	ERROR ((0, 0, _("Malformed extended header: missing length")));
       return false;
     }
 
-  keyword = ++*p;
-  for (;*p < start + len; ++*p)
-    if (**p == '=')
-      break;
+  errno = 0;
+  len = strtoul (p, &len_lim, 10);
 
-  if (**p != '=')
+  if (len_max < len || (len == ULONG_MAX && errno == ERANGE))
     {
-      ERROR ((0, 0, _("Malformed extended header: missing equal sign")));
+      ERROR ((0, 0, _("Malformed extended header: length out of range")));
       return false;
     }
 
-  **p = 0;
+  nextp = start + len;
 
-  endc = start[len-1];
-  start[len-1] = 0;
+  for (p = len_lim; *p == ' ' || *p == '\t'; p++)
+    continue;
+  if (p == len_lim)
+    {
+      ERROR ((0, 0,
+	      _("Malformed extended header: missing blank after length")));
+      return false;
+    }
 
-  handler (data, keyword, *p + 1);
+  keyword = p;
+  p = strchr (p, '=');
+  if (! (p && p < nextp))
+    {
+      ERROR ((0, 0, _("Malformed extended header: missing equal sign")));
+      return false;
+    }
+
+  if (nextp[-1] != '\n')
+    {
+      ERROR ((0, 0, _("Malformed extended header: missing newline")));
+      return false;
+    }
 
-  start[len-1] = endc;
-  **p = '=';
-  *p = &start[len];
+  *p = nextp[-1] = '\0';
+  handler (data, keyword, p + 1);
+  *p = '=';
+  nextp[-1] = '\n';
+  *ptr = nextp;
   return true;
 }
 
@@ -546,11 +565,8 @@ xheader_decode (struct tar_stat_info *st)
   if (extended_header.size)
     {
       char *p = extended_header.buffer + BLOCKSIZE;
-      char *endp = &extended_header.buffer[extended_header.size-1];
-
-      while (p < endp)
-	if (!decode_record (&p, decx, st))
-	  break;
+      while (decode_record (&p, decx, st))
+	continue;
     }
   run_override_list (keyword_override_list, st);
 }
@@ -568,12 +584,10 @@ xheader_decode_global (void)
   if (extended_header.size)
     {
       char *p = extended_header.buffer + BLOCKSIZE;
-      char *endp = &extended_header.buffer[extended_header.size-1];
 
       xheader_list_destroy (&global_header_override_list);
-      while (p < endp)
-	if (!decode_record (&p, decg, &global_header_override_list))
-	  break;
+      while (decode_record (&p, decg, &global_header_override_list))
+	continue;
     }
 }
 
@@ -615,6 +629,7 @@ xheader_read (union block *p, size_t size)
   extended_header.size = size;
   nblocks = (size + BLOCKSIZE - 1) / BLOCKSIZE;
   extended_header.buffer = xmalloc (size + 1);
+  extended_header.buffer[size] = '\0';
 
   do
     {