1 files changed, 42 insertions, 61 deletions

diff --git a/libarchive/archive_read_support_format_tar.c b/libarchive/archive_read_support_format_tar.c
index b977cb7400fb..071d766b74b7 100644
--- 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;
 	}