From 7554e3e42cd72f6f8304410c47fe6f8918e9bfd7 Mon Sep 17 00:00:00 2001
From: Sergey Poznyakoff <gray@gnu.org>
Date: Sun, 3 Nov 2019 23:59:39 +0200
Subject: Fix CVE-2019-14866

* src/copyout.c (to_ascii): Additional argument nul controls whether
to add the terminating nul character.
(field_width_error): Improve diagnostics: print the actual and the
maximum allowed field value.
* src/extern.h (to_ascii, field_width_error): New prototypes.
* src/tar.c (to_oct): Remove.
(to_oct_or_error): New function.
(TO_OCT): New macro.
(write_out_tar_header): Use TO_OCT and to_ascii. Return 0 on
success, 1 on error.
---
 src/copyout.c | 49 ++++++++++++++++++++++++----------------
 src/extern.h  | 15 +++++++++++--
 src/tar.c     | 72 +++++++++++++++++++++++++++--------------------------------
 3 files changed, 76 insertions(+), 60 deletions(-)

Index: cpio-2.11/src/copyout.c
===================================================================
--- cpio-2.11.orig/src/copyout.c
+++ cpio-2.11/src/copyout.c
@@ -279,26 +279,32 @@ writeout_final_defers (int out_des)
    so it should be moved to paxutils too.
    Allowed values for logbase are: 1 (binary), 2, 3 (octal), 4 (hex) */
 int
-to_ascii (char *where, uintmax_t v, size_t digits, unsigned logbase)
+to_ascii (char *where, uintmax_t v, size_t digits, unsigned logbase, bool nul)
 {
   static char codetab[] = "0123456789ABCDEF";
-  int i = digits;
-  
-  do
+
+  if (nul)
+    where[--digits] = 0;
+  while (digits > 0)
     {
-      where[--i] = codetab[(v & ((1 << logbase) - 1))];
+      where[--digits] = codetab[(v & ((1 << logbase) - 1))];
       v >>= logbase;
     }
-  while (i);
 
   return v != 0;
 }
 
-static void
-field_width_error (const char *filename, const char *fieldname)
+void
+field_width_error (const char *filename, const char *fieldname,
+		   uintmax_t value, size_t width, bool nul)
 {
-  error (0, 0, _("%s: field width not sufficient for storing %s"),
-	 filename, fieldname);
+  char valbuf[UINTMAX_STRSIZE_BOUND + 1];
+  char maxbuf[UINTMAX_STRSIZE_BOUND + 1];
+  error (0, 0, _("%s: value %s %s out of allowed range 0..%s"),
+	 filename, fieldname,
+	 STRINGIFY_BIGINT (value, valbuf),
+	 STRINGIFY_BIGINT (MAX_VAL_WITH_DIGITS (width - nul, LG_8),
+			   maxbuf));
 }
 
 static void
@@ -313,7 +319,7 @@ to_ascii_or_warn (char *where, uintmax_t
 		  unsigned logbase,
 		  const char *filename, const char *fieldname)
 {
-  if (to_ascii (where, n, digits, logbase))
+  if (to_ascii (where, n, digits, logbase, false))
     field_width_warning (filename, fieldname);
 }    
 
@@ -322,9 +328,9 @@ to_ascii_or_error (char *where, uintmax_
 		   unsigned logbase,
 		   const char *filename, const char *fieldname)
 {
-  if (to_ascii (where, n, digits, logbase))
+  if (to_ascii (where, n, digits, logbase, false))
     {
-      field_width_error (filename, fieldname);
+      field_width_error (filename, fieldname, n, digits, false);
       return 1;
     }
   return 0;
@@ -381,7 +387,7 @@ write_out_new_ascii_header (const char *
 			 _("name size")))
     return 1;
   p += 8;
-  to_ascii (p, file_hdr->c_chksum & 0xffffffff, 8, LG_16);
+  to_ascii (p, file_hdr->c_chksum & 0xffffffff, 8, LG_16, false);
 
   tape_buffered_write (ascii_header, out_des, sizeof ascii_header);
 
@@ -398,7 +404,7 @@ write_out_old_ascii_header (dev_t dev, d
   char ascii_header[76];
   char *p = ascii_header;
   
-  to_ascii (p, file_hdr->c_magic, 6, LG_8);
+  to_ascii (p, file_hdr->c_magic, 6, LG_8, false);
   p += 6;
   to_ascii_or_warn (p, dev, 6, LG_8, file_hdr->c_name, _("device number"));
   p += 6;
@@ -502,7 +508,10 @@ write_out_binary_header (dev_t rdev,
   short_hdr.c_namesize = file_hdr->c_namesize & 0xFFFF;
   if (short_hdr.c_namesize != file_hdr->c_namesize)
     {
-      field_width_error (file_hdr->c_name, _("name size"));
+      char maxbuf[UINTMAX_STRSIZE_BOUND + 1];
+      error (0, 0, _("%s: value %s %s out of allowed range 0..%u"),
+	     file_hdr->c_name, _("name size"),
+	     STRINGIFY_BIGINT (file_hdr->c_namesize, maxbuf), 0xFFFFu);
       return 1;
     }
 		      
@@ -512,7 +521,10 @@ write_out_binary_header (dev_t rdev,
   if (((off_t)short_hdr.c_filesizes[0] << 16) + short_hdr.c_filesizes[1]
        != file_hdr->c_filesize)
     {
-      field_width_error (file_hdr->c_name, _("file size"));
+      char maxbuf[UINTMAX_STRSIZE_BOUND + 1];
+      error (0, 0, _("%s: value %s %s out of allowed range 0..%lu"),
+	     file_hdr->c_name, _("file size"),
+	     STRINGIFY_BIGINT (file_hdr->c_namesize, maxbuf), 0xFFFFFFFFlu);
       return 1;
     }
 		      
@@ -562,8 +574,7 @@ write_out_header (struct cpio_file_stat
 	  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,
Index: cpio-2.11/src/extern.h
===================================================================
--- cpio-2.11.orig/src/extern.h
+++ cpio-2.11/src/extern.h
@@ -116,6 +116,10 @@ void print_name_with_quoting (char *p);
 /* copyout.c */
 int write_out_header (struct cpio_file_stat *file_hdr, int out_des);
 void process_copy_out (void);
+int to_ascii (char *where, uintmax_t v, size_t digits, unsigned logbase,
+	      bool nul);
+void field_width_error (const char *filename, const char *fieldname,
+			uintmax_t value, size_t width, bool nul);
 
 /* copypass.c */
 void process_copy_pass (void);
@@ -144,7 +148,7 @@ int make_path (char *argpath, uid_t owne
 	       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);
@@ -202,9 +206,16 @@ void cpio_safer_name_suffix (char *name,
 			     bool absolute_names, bool strip_leading_dots);
 int cpio_create_dir (struct cpio_file_stat *file_hdr, int existing_dir);
 
-/* FIXME: These two defines should be defined in paxutils */
+/* FIXME: The following three should be defined in paxutils */
 #define LG_8  3
 #define LG_16 4
+/* The maximum uintmax_t value that can be represented with DIGITS digits,
+   assuming that each digit is BITS_PER_DIGIT wide.  */
+#define MAX_VAL_WITH_DIGITS(digits, bits_per_digit) \
+   ((digits) * (bits_per_digit) < sizeof (uintmax_t) * CHAR_BIT \
+    ? ((uintmax_t) 1 << ((digits) * (bits_per_digit))) - 1 \
+    : (uintmax_t) -1)
+
 
 uintmax_t from_ascii (char const *where, size_t digs, unsigned logbase);
 
Index: cpio-2.11/src/tar.c
===================================================================
--- cpio-2.11.orig/src/tar.c
+++ cpio-2.11/src/tar.c
@@ -1,6 +1,5 @@
 /* tar.c - read in write tar headers for cpio
-   Copyright (C) 1992, 2001, 2004, 2006, 2007, 2010 Free Software
-   Foundation, Inc.
+   Copyright (C) 1992-2019 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
@@ -79,36 +78,17 @@ stash_tar_filename (char *prefix, char *
   return hold_tar_filename;
 }
 
-/* Convert a number into a string of octal digits.
-   Convert long VALUE into a DIGITS-digit field at WHERE,
-   including a trailing space and room for a NUL.  DIGITS==3 means
-   1 digit, a space, and room for a NUL.
-
-   We assume the trailing NUL is already there and don't fill it in.
-   This fact is used by start_header and finish_header, so don't change it!
-
-   This is be equivalent to:
-   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 (uintmax_t value, size_t digits, char *where, char const *field,
+                char const *file)
 {
-  --digits;			/* Leave the trailing NUL slot alone.  */
-
-  /* Produce the digits -- at least one.  */
-  do
+  if (to_ascii (where, value, digits, LG_8, true))
     {
-      where[--digits] = '0' + (char) (value & 7); /* One octal digit.  */
-      value >>= 3;
+      field_width_error (file, field, value, digits, true);
+      return 1;
     }
-  while (digits > 0 && value != 0);
-
-  /* Add leading zeroes, if necessary.  */
-  while (digits > 0)
-    where[--digits] = '0';
+  return 0;
 }
-
 
 
 /* Compute and return a checksum for TAR_HDR,
@@ -134,10 +114,22 @@ tar_checksum (struct tar_header *tar_hdr
   return sum;
 }
 
+#define TO_OCT(file_hdr, c_fld, digits, tar_hdr, tar_field) \
+  do							    \
+    {							    \
+       if (to_oct_or_error (file_hdr -> c_fld,		    \
+			    digits,			    \
+			    tar_hdr -> tar_field,	    \
+			    #tar_field,			    \
+			    file_hdr->c_name))		    \
+	 return 1;					    \
+    }							    \
+  while (0)
+
 /* 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 +158,11 @@ write_out_tar_header (struct cpio_file_s
 
   /* 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);
+  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);
 
   switch (file_hdr->c_mode & CP_IFMT)
     {
@@ -182,7 +174,7 @@ write_out_tar_header (struct cpio_file_s
 	  strncpy (tar_hdr->linkname, file_hdr->c_tar_linkname,
 		   TARLINKNAMESIZE);
 	  tar_hdr->typeflag = LNKTYPE;
-	  to_oct (0, 12, tar_hdr->size);
+	  to_ascii (tar_hdr->size, 0, 12, LG_8, true);
 	}
       else
 	tar_hdr->typeflag = REGTYPE;
@@ -208,7 +200,7 @@ write_out_tar_header (struct cpio_file_s
 	 than TARLINKNAMESIZE.  */
       strncpy (tar_hdr->linkname, file_hdr->c_tar_linkname,
 	       TARLINKNAMESIZE);
-      to_oct (0, 12, tar_hdr->size);
+      to_ascii (tar_hdr->size, 0, 12, LG_8, true);
       break;
 #endif /* CP_IFLNK */
     }
@@ -227,13 +219,15 @@ write_out_tar_header (struct cpio_file_s
       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);
+      TO_OCT (file_hdr, c_rdev_maj, 8, tar_hdr, devmajor);
+      TO_OCT (file_hdr, c_rdev_min, 8, tar_hdr, devminor);
     }
 
-  to_oct (tar_checksum (tar_hdr), 8, tar_hdr->chksum);
+  to_ascii (tar_hdr->chksum, tar_checksum (tar_hdr), 8, LG_8, true);
 
   tape_buffered_write ((char *) &tar_rec, out_des, TARRECORDSIZE);
+
+  return 0;
 }
 
 /* Return nonzero iff all the bytes in BLOCK are NUL.
