[Bug-tar] tar-1.15.1+: numerous bugs in xheader.c

[Jim Meyering]

[ This is all regarding the latest code in cvs. ]

This expedition started when I found an unchecked call to calloc
in tar's xheader.c.  Here's what can result:

  # Create a sparse file
  $ dd bs=1 seek=1M of=big < /dev/null 2> /dev/null

  # Put it in a tar archive, using the sparse option and POSIX/pax format
  $ ./tar --sparse --format=posix -cf x.tar big

  # Corrupt the tar archive so that the unchecked calloc fails.
  $ perl -pi -e 's/26 (GNU.sparse.numblocks)=1/35 $1=4294967296/' x.tar
  # Remove NUL bytes to compensate for the 9 digits added above.
  $ perl -0777 -pi -e 's/(ctime=[^\n]+\n)\0{9}/$1/m' x.tar

Then, any attempt to list the archive or to extract anything from it
results in an attempt to dereference the NULL pointer:

  $ ./tar tf x.tar
  [Exit 139 (SEGV)]

Or, change the 2^32 to e.g., 2_32+1 to make tar write beyond
the end of an allocated buffer.

With the patch below, you'd get this instead:

  $ ./tar tf x.tar
  ./tar: Malformed header: invalid number of sparse blocks: 4294967296
  ./tar: Error is not recoverable: exiting now
  [Exit 2]

But then I noticed that since I'd chosen 2^32 as the number of blocks,
simply changing calloc to xcalloc didn't solve the problem.
The next problem is here:

  static void
  sparse_numblocks_decoder (struct tar_stat_info *st, char const *arg)
  {
    uintmax_t u;
    if (xstrtoumax (arg, NULL, 10, &u, "") == LONGINT_OK)
      {
        st->sparse_map_size = u;
        st->sparse_map = calloc(st->sparse_map_size, sizeof(st->sparse_map[0]));
        st->sparse_map_avail = 0;
      }
  }

This function assigns `u's value (of type uintmax_t) to
`sparse_map_size', which is of type size_t, so with u=2^32,
st->sparse_map_size ends up being zero on a system with a 32-bit size_t.
Another segfault.

Not only that, but if the string, `arg' is invalid (e.g., non-numeric),
this function simply returns, and that results in a similar failure,
because sparse_offset_decoder stores into st->sparse_map[0] even
though st->sparse_map is NULL
and    st->sparse_map_size is 0.

Some of the other *_decoder functions have similar problems.
IMHO, any header decoder/parsing failure should cause tar to fail.

I spotted another problem: sparse_numbytes_decoder can overflow
st->sparse_map_size (that happens when a contrived tar file contains
too many GNU.sparse.numbytes=N lines) and end up corrupting the heap.
I suggest rewriting it to use x2nrealloc (from gnulib's xmalloc.c)
rather than xrealloc.

In testing, I found that my change induced many failures,
each because of the new ``Malformed header: invalid time: ...' error.
That was because decode_time's first xstrtoumax call was always
failing for time strings like this: 1119018481.000000000.
I.e., the existing code totally ignored time strings like
that one, since the `.' was treated as an invalid suffix.

[I'm sure you know already, but I have to say it... ]
All of this suggests there is a pressing need for additions to
the test suite.

Here's a patch that fixes all of the above problems:
[ It'd be nice to have a testing aid with which to create tar
  archives made with selected (invalid) values for given fields. ]

2005-06-17  Jim Meyering  <address@hidden>

        Carefully crafted invalid headers can cause buffer overrun.
        Invalid header fields go undiagnosed.
        Some valid time strings are ignored.

        * src/xheader.c (sparse_numblocks_decoder): Remove unchecked use
        of `calloc'.  Use xcalloc instead.
        (decode_time, gid_decoder, size_decoder, uid_decoder):
        (sparse_size_decoder, sparse_offset_decoder, sparse_numblocks_decoder):
        Ensure that the result of calling xstrtoumax is no larger than
        the maximum value for the target type.  Upon any failure, exit with
        a diagnostic.
        (sparse_numblocks_decoder): Avoid buffer overrun/heap corruption:
        use x2nrealloc, rather than `n *= 2' and xrealloc(p, n,....
        (decode_time): Rewrite to accept time strings like 1119018481.000000000.
        Before, such strings were always ignored.

Index: src/xheader.c
===================================================================
RCS file: /cvsroot/tar/tar/src/xheader.c,v
retrieving revision 1.26
diff -u -p -r1.26 xheader.c
--- src/xheader.c       21 May 2005 23:10:42 -0000      1.26
+++ src/xheader.c       17 Jun 2005 16:08:44 -0000
@@ -246,7 +246,7 @@ xheader_format_name (struct tar_stat_inf
        case 'd':
          if (st)
            {
-             dir = safer_name_suffix (dir_name (st->orig_file_name), 
+             dir = safer_name_suffix (dir_name (st->orig_file_name),
                                       false, absolute_names_option);
              len += strlen (dir) - 1;
            }
@@ -777,12 +777,25 @@ decode_time (char const *arg, time_t *se
 {
   uintmax_t u;
   char *p;
-  if (xstrtoumax (arg, &p, 10, &u, "") == LONGINT_OK)
+  if (xstrtoumax (arg, &p, 10, &u, NULL) == LONGINT_OK
+      && u <= TYPE_MAXIMUM (time_t))
     {
-      *secs = u;
-      if (*p == '.' && xstrtoumax (p+1, NULL, 10, &u, "") == LONGINT_OK)
-       *nsecs = u;
+      uintmax_t v;
+      if (*p == '\0')
+       {
+         *secs = u;
+         return;
+       }
+      if (*p == '.'
+         && xstrtoumax (p+1, NULL, 10, &v, "") == LONGINT_OK
+         && v <= TYPE_MAXIMUM (unsigned long))
+       {
+         *secs = u;
+         *nsecs = v;
+         return;
+       }
     }
+  FATAL_ERROR ((0, 0, _("Malformed header: invalid time: %s"), arg));
 }
 
 static void
@@ -833,8 +846,12 @@ static void
 gid_decoder (struct tar_stat_info *st, char const *arg)
 {
   uintmax_t u;
-  if (xstrtoumax (arg, NULL, 10, &u, "") == LONGINT_OK)
+  if (xstrtoumax (arg, NULL, 10, &u, "") == LONGINT_OK
+      && u <= TYPE_MAXIMUM (gid_t))
     st->stat.st_gid = u;
+  else
+    FATAL_ERROR ((0, 0,
+                 _("Malformed header: invalid group ID number: %s"), arg));
 }
 
 static void
@@ -915,8 +932,11 @@ static void
 size_decoder (struct tar_stat_info *st, char const *arg)
 {
   uintmax_t u;
-  if (xstrtoumax (arg, NULL, 10, &u, "") == LONGINT_OK)
+  if (xstrtoumax (arg, NULL, 10, &u, "") == LONGINT_OK
+      && u <= TYPE_MAXIMUM (off_t))
     st->archive_file_size = st->stat.st_size = u;
+  else
+    FATAL_ERROR ((0, 0, _("Malformed header: invalid archive size: %s"), arg));
 }
 
 static void
@@ -930,8 +950,11 @@ static void
 uid_decoder (struct tar_stat_info *st, char const *arg)
 {
   uintmax_t u;
-  if (xstrtoumax (arg, NULL, 10, &u, "") == LONGINT_OK)
+  if (xstrtoumax (arg, NULL, 10, &u, "") == LONGINT_OK
+      && u <= TYPE_MAXIMUM (uid_t))
     st->stat.st_uid = u;
+  else
+    FATAL_ERROR ((0, 0, _("Malformed header: invalid user ID number: %s"), 
arg));
 }
 
 static void
@@ -958,8 +981,11 @@ static void
 sparse_size_decoder (struct tar_stat_info *st, char const *arg)
 {
   uintmax_t u;
-  if (xstrtoumax (arg, NULL, 10, &u, "") == LONGINT_OK)
+  if (xstrtoumax (arg, NULL, 10, &u, "") == LONGINT_OK
+      && u <= TYPE_MAXIMUM (off_t))
     st->stat.st_size = u;
+  else
+    FATAL_ERROR ((0, 0, _("Malformed header: invalid sparse size: %s"), arg));
 }
 
 static void
@@ -974,12 +1000,15 @@ static void
 sparse_numblocks_decoder (struct tar_stat_info *st, char const *arg)
 {
   uintmax_t u;
-  if (xstrtoumax (arg, NULL, 10, &u, "") == LONGINT_OK)
+  if (xstrtoumax (arg, NULL, 10, &u, "") == LONGINT_OK && u <= SIZE_MAX)
     {
       st->sparse_map_size = u;
-      st->sparse_map = calloc(st->sparse_map_size, sizeof(st->sparse_map[0]));
+      st->sparse_map = xcalloc(st->sparse_map_size, sizeof(st->sparse_map[0]));
       st->sparse_map_avail = 0;
     }
+  else
+    FATAL_ERROR ((0, 0,
+       _("Malformed header: invalid number of sparse blocks: %s"), arg));
 }
 
 static void
@@ -994,8 +1023,11 @@ static void
 sparse_offset_decoder (struct tar_stat_info *st, char const *arg)
 {
   uintmax_t u;
-  if (xstrtoumax (arg, NULL, 10, &u, "") == LONGINT_OK)
+  if (xstrtoumax (arg, NULL, 10, &u, "") == LONGINT_OK
+      && u <= TYPE_MAXIMUM (off_t))
     st->sparse_map[st->sparse_map_avail].offset = u;
+  else
+    FATAL_ERROR ((0, 0, _("Malformed header: invalid sparse offset: %s"), 
arg));
 }
 
 static void
@@ -1015,12 +1047,15 @@ sparse_numbytes_decoder (struct tar_stat
       if (st->sparse_map_avail == st->sparse_map_size)
        {
          st->sparse_map_size *= 2;
-         st->sparse_map = xrealloc (st->sparse_map,
-                                    st->sparse_map_size
-                                    * sizeof st->sparse_map[0]);
+         st->sparse_map = x2nrealloc (st->sparse_map,
+                                      &st->sparse_map_size,
+                                      sizeof st->sparse_map[0]);
        }
       st->sparse_map[st->sparse_map_avail++].numbytes = u;
     }
+  else
+    FATAL_ERROR ((1, 0,
+                 _("Malformed header: invalid sparse byte count: %s"), arg));
 }
 
 struct xhdr_tab const xhdr_tab[] = {