Rework the valid_number_field test function to be more

lenient.  There are an enormous number of tar writing
programs, many which don't quite follow the specs.

Note:  I don't think I recall seeing any that left
number fields with trailing garbage; I may be wrong
about that, in which case, this would have to stop
at the first NUL (after the first octal character).

diff --git a/libarchive/archive_read_support_format_tar.c b/libarchive/archive_read_support_format_tar.c
index b977cb7400fb102506c472da6c90e5200f103181..071d766b74b7f464715702606f65e41fc76b29f6 100644 (file)
--- a/libarchive/archive_read_support_format_tar.c
+++ b/libarchive/archive_read_support_format_tar.c
@@ -297,58 +297,50 @@ archive_read_format_tar_cleanup(struct archive_read *a)
 /*
  * Validate number field
  *
- * Flags:
- * 1 - allow double \0 at field end
+ * This has to be pretty lenient in order to accomodate the enormous
+ * variety of tar writers in the world:
+ *  = POSIX ustar requires octal values with leading zeros and
+ *    specific termination on fields
+ *  = Many writers use different termination (in particular, libarchive
+ *    omits terminator bytes to squeeze one or two more digits)
+ *  = Many writers pad with space and omit leading zeros
+ *  = GNU tar and star write base-256 values if numbers are too
+ *    big to be represented in octal
+ *
+ * This should tolerate all variants in use.  It will reject a field
+ * where the writer just left garbage after a trailing NUL.
  */
 static int
-validate_number_field(const char* p_field, size_t i_size, int flags)
+validate_number_field(const char* p_field, size_t i_size)
 {
        unsigned char marker = (unsigned char)p_field[0];
-       /* octal? */
-       if ((marker >= '0' && marker <= '7') || marker == ' ') {
+       if (marker == 128 || marker == 255 || marker == 0) {
+               /* Base-256 marker, there's nothing we can check. */
+               return 1;
+       } else {
+               /* Must be octal */
                size_t i = 0;
-               int octal_found = 0;
-               for (i = 0; i < i_size; ++i) {
-                       switch (p_field[i])
-                       {
-                       case ' ':
-                               /* skip any leading spaces and trailing space */
-                               if (octal_found == 0 || i == i_size - 1) {
-                                       continue;
-                               }
-                               break;
-                       case '\0':
-                               /*
-                                * null should be allowed only at the end
-                                *
-                                * Perl Archive::Tar terminates some fields
-                                * with two nulls. We must allow this to stay
-                                * compatible.
-                                */
-                               if (i != i_size - 1) {
-                                       if (((flags & 1) == 0)
-                                           || i != i_size - 2)
-                                               return 0;
-                               }
-                               break;
-                       /* rest must be octal digits */
-                       case '0': case '1': case '2': case '3':
-                       case '4': case '5': case '6': case '7':
-                               ++octal_found;
-                               break;
+               /* Skip any leading spaces */
+               while (i < i_size && p_field[i] == ' ') {
+                       ++i;
+               }
+               /* Must be at least one octal digit. */
+               if (i >= i_size || p_field[i] < '0' || p_field[i] > '7') {
+                       return 0;
+               }
+               /* Skip remaining octal digits. */
+               while (i < i_size && p_field[i] >= '0' && p_field[i] <= '7') {
+                       ++i;
+               }
+               /* Any remaining characters must be space or NUL padding. */
+               while (i < i_size) {
+                       if (p_field[i] != ' ' && p_field[i] != 0) {
+                               return 0;
                        }
+                       ++i;
                }
-               return octal_found > 0;
-       }
-       /* base 256 (i.e. binary number) */
-       else if (marker == 128 || marker == 255 || marker == 0) {
-               /* nothing to check */
                return 1;
        }
-       /* not a number field */
-       else {
-               return 0;
-       }
 }
 
 static int
@@ -404,26 +396,15 @@ archive_read_format_tar_bid(struct archive_read *a, int best_bid)
 
        /*
         * Check format of mode/uid/gid/mtime/size/rdevmajor/rdevminor fields.
-        * These are usually octal numbers but GNU tar encodes "big" values as
-        * base256 and leading zeroes are sometimes replaced by spaces.
-        * Even the null terminator is sometimes omitted. Anyway, must be
-        * checked to avoid false positives.
-        *
-        * Perl Archive::Tar does not follow the spec and terminates mode, uid,
-        * gid, rdevmajor and rdevminor with a double \0. For compatibility
-        * reasons we allow this deviation.
         */
        if (bid > 0 && (
-           validate_number_field(header->mode, sizeof(header->mode), 1) == 0
-           || validate_number_field(header->uid, sizeof(header->uid), 1) == 0
-           || validate_number_field(header->gid, sizeof(header->gid), 1) == 0 
-           || validate_number_field(header->mtime, sizeof(header->mtime),
-           0) == 0
-           || validate_number_field(header->size, sizeof(header->size), 0) == 0
-           || validate_number_field(header->rdevmajor,
-           sizeof(header->rdevmajor), 1) == 0
-           || validate_number_field(header->rdevminor,
-           sizeof(header->rdevminor), 1) == 0)) {
+           validate_number_field(header->mode, sizeof(header->mode)) == 0
+           || validate_number_field(header->uid, sizeof(header->uid)) == 0
+           || validate_number_field(header->gid, sizeof(header->gid)) == 0
+           || validate_number_field(header->mtime, sizeof(header->mtime)) == 0
+           || validate_number_field(header->size, sizeof(header->size)) == 0
+           || validate_number_field(header->rdevmajor, sizeof(header->rdevmajor)) == 0
+           || validate_number_field(header->rdevminor, sizeof(header->rdevminor)) == 0)) {
                bid = 0;
        }