Check for size overflow in tar header fields.

    This prevents surprising outputs being created, e.g. this cpio tar
    output with more than one file:

    tar cf suffix.tar AUTHORS
    dd if=/dev/zero seek=16G bs=1 count=0 of=suffix.tar
    echo suffix.tar | cpio -H tar -o | tar tvf -

    -rw-r--r-- 1000/1000       0 2019-08-30 16:40 suffix.tar
    -rw-r--r-- thomas/thomas 161 2019-08-30 16:40 AUTHORS

Patch attached, but also at https://cement.retrofitta.se/tmp/cpio-tar.patch

--
☢ Thomas ☢
commit bc285942df401ea332667ce89f821648cc27d9cd
Author: Thomas Habets <hab...@google.com>
Date:   Fri Aug 30 16:48:39 2019 +0100

    Check for size overflow in tar header fields.
    
    This prevents surprising outputs being created, e.g. this cpio tar
    output with more than one file:
    
    tar cf suffix.tar AUTHORS
    dd if=/dev/zero seek=16G bs=1 count=0 of=suffix.tar
    echo suffix.tar | cpio -H tar -o | tar tvf -
    
    -rw-r--r-- 1000/1000       0 2019-08-30 16:40 suffix.tar
    -rw-r--r-- thomas/thomas 161 2019-08-30 16:40 AUTHORS

diff --git a/src/copyout.c b/src/copyout.c
index 7532dac..22a3f9a 100644
--- a/src/copyout.c
+++ b/src/copyout.c
@@ -552,8 +552,7 @@ write_out_header (struct cpio_file_stat *file_hdr, int out_des)
 	  error (0, 0, _("%s: file name too long"), file_hdr->c_name);
 	  return 1;
 	}
-      write_out_tar_header (file_hdr, out_des); /* FIXME: No error checking */
-      return 0;
+      return write_out_tar_header (file_hdr, out_des);
 
     case arf_binary:
       return write_out_binary_header (makedev (file_hdr->c_rdev_maj,
diff --git a/src/extern.h b/src/extern.h
index 6fa2089..bb75b0b 100644
--- a/src/extern.h
+++ b/src/extern.h
@@ -144,7 +144,7 @@ int make_path (char *argpath, uid_t owner, gid_t group,
 	       const char *verbose_fmt_string);
 
 /* tar.c */
-void write_out_tar_header (struct cpio_file_stat *file_hdr, int out_des);
+int write_out_tar_header (struct cpio_file_stat *file_hdr, int out_des);
 int null_block (long *block, int size);
 void read_in_tar_header (struct cpio_file_stat *file_hdr, int in_des);
 int otoa (char *s, unsigned long *n);
diff --git a/src/tar.c b/src/tar.c
index 0a34845..98ff74d 100644
--- a/src/tar.c
+++ b/src/tar.c
@@ -91,8 +91,9 @@ stash_tar_filename (char *prefix, char *filename)
    sprintf (where, "%*lo ", digits - 2, value);
    except that sprintf fills in the trailing NUL and we don't.  */
 
-static void
-to_oct (register long value, register int digits, register char *where)
+static int
+to_oct_or_error (register long value, register int digits, register char *where,
+                 const char *filename, const char *fieldname)
 {
   --digits;			/* Leave the trailing NUL slot alone.  */
 
@@ -103,10 +104,17 @@ to_oct (register long value, register int digits, register char *where)
       value >>= 3;
     }
   while (digits > 0 && value != 0);
+  if (value > 0)
+    {
+      error (0, 0, _("%s: field width not sufficient for storing %s"),
+             filename, fieldname);
+      return 1;
+    }
 
   /* Add leading zeroes, if necessary.  */
   while (digits > 0)
     where[--digits] = '0';
+  return 0;
 }
 
 
@@ -137,7 +145,7 @@ tar_checksum (struct tar_header *tar_hdr)
 /* Write out header FILE_HDR, including the file name, to file
    descriptor OUT_DES.  */
 
-void
+int
 write_out_tar_header (struct cpio_file_stat *file_hdr, int out_des)
 {
   int name_len;
@@ -166,11 +174,16 @@ write_out_tar_header (struct cpio_file_stat *file_hdr, int out_des)
 
   /* Ustar standard (POSIX.1-1988) requires the mode to contain only 3 octal
      digits */
-  to_oct (file_hdr->c_mode & MODE_ALL, 8, tar_hdr->mode);
-  to_oct (file_hdr->c_uid, 8, tar_hdr->uid);
-  to_oct (file_hdr->c_gid, 8, tar_hdr->gid);
-  to_oct (file_hdr->c_filesize, 12, tar_hdr->size);
-  to_oct (file_hdr->c_mtime, 12, tar_hdr->mtime);
+  if (to_oct_or_error (file_hdr->c_mode & MODE_ALL, 8, tar_hdr->mode, file_hdr->c_name, _("mode")))
+    return 1;
+  if (to_oct_or_error (file_hdr->c_uid, 8, tar_hdr->uid, file_hdr->c_name, _("uid")))
+    return 1;
+  if (to_oct_or_error (file_hdr->c_gid, 8, tar_hdr->gid, file_hdr->c_name, _("gid")))
+    return 1;
+  if (to_oct_or_error (file_hdr->c_filesize, 12, tar_hdr->size, file_hdr->c_name, _("file size")))
+    return 1;
+  if (to_oct_or_error (file_hdr->c_mtime, 12, tar_hdr->mtime, file_hdr->c_name, _("modification time")))
+    return 1;
 
   switch (file_hdr->c_mode & CP_IFMT)
     {
@@ -182,7 +195,8 @@ write_out_tar_header (struct cpio_file_stat *file_hdr, int out_des)
 	  strncpy (tar_hdr->linkname, file_hdr->c_tar_linkname,
 		   TARLINKNAMESIZE);
 	  tar_hdr->typeflag = LNKTYPE;
-	  to_oct (0, 12, tar_hdr->size);
+	  if (to_oct_or_error (0, 12, tar_hdr->size, file_hdr->c_name, _("file size")))
+            return 1;
 	}
       else
 	tar_hdr->typeflag = REGTYPE;
@@ -208,7 +222,8 @@ write_out_tar_header (struct cpio_file_stat *file_hdr, int out_des)
 	 than TARLINKNAMESIZE.  */
       strncpy (tar_hdr->linkname, file_hdr->c_tar_linkname,
 	       TARLINKNAMESIZE);
-      to_oct (0, 12, tar_hdr->size);
+      if (to_oct_or_error (0, 12, tar_hdr->size, file_hdr->c_name, _("file size")))
+        return 1;
       break;
 #endif /* CP_IFLNK */
     }
@@ -227,13 +242,17 @@ write_out_tar_header (struct cpio_file_stat *file_hdr, int out_des)
       if (name)
 	strcpy (tar_hdr->gname, name);
 
-      to_oct (file_hdr->c_rdev_maj, 8, tar_hdr->devmajor);
-      to_oct (file_hdr->c_rdev_min, 8, tar_hdr->devminor);
+      if (to_oct_or_error (file_hdr->c_rdev_maj, 8, tar_hdr->devmajor, file_hdr->c_name, _("rdev major")))
+        return 1;
+      if (to_oct_or_error (file_hdr->c_rdev_min, 8, tar_hdr->devminor, file_hdr->c_name, _("rdev minor")))
+        return 1;
     }
 
-  to_oct (tar_checksum (tar_hdr), 8, tar_hdr->chksum);
+  if (to_oct_or_error (tar_checksum (tar_hdr), 8, tar_hdr->chksum, file_hdr->c_name, _("checksum")))
+    return 1;
 
   tape_buffered_write ((char *) &tar_rec, out_des, TARRECORDSIZE);
+  return 0;
 }
 
 /* Return nonzero iff all the bytes in BLOCK are NUL.

Reply via email to