Ignore out-of-range gid/uid/size/ino and harden AFIO parsing (#2258)

The fuzzer constructed an AFIO (CPIO variant) archive that had a
rediculously large ino value, which caused an overflow of a signed
64-bit intermediate.

There are really three issues here:
* The CPIO parser was using a signed int64 as an intermediate type for
parsing numbers in all cases. I've addressed the overflow here by using
a uint64_t in the parser core, but left the resulting values as int64_t.
* The AFIO header parsing had no guards against rediculously large
values; it now rejects an archive when the ino or size fields (which are
allowed to be up to 16 hex digits long) overflow int64_t to produce a
negative value.
* The archive_entry would accept negative values for gid/uid/size/ino.
I've altered those so that these fields treat any negative value as zero
for these fields.

There was one test that actually verified that we could read a field
with size = -1. I've updated that to verify that the resulting size is
zero instead.

OSS-Fuzz Issue: 70019

diff --git a/libarchive/archive_entry.c b/libarchive/archive_entry.c
index ef322341a9ed0a9cdd1dc9ba43446d681b871096..f68fee65d521cba6ec35f85d85eb4dad0ce68337 100644 (file)
--- a/libarchive/archive_entry.c
+++ b/libarchive/archive_entry.c
@@ -930,6 +930,9 @@ archive_entry_copy_fflags_text_w(struct archive_entry *entry,
 void
 archive_entry_set_gid(struct archive_entry *entry, la_int64_t g)
 {
+       if (g < 0) {
+               g = 0;
+       }
        entry->stat_valid = 0;
        entry->ae_stat.aest_gid = g;
        entry->ae_set |= AE_SET_GID;
@@ -980,6 +983,9 @@ _archive_entry_copy_gname_l(struct archive_entry *entry,
 void
 archive_entry_set_ino(struct archive_entry *entry, la_int64_t ino)
 {
+       if (ino < 0) {
+               ino = 0;
+       }
        entry->stat_valid = 0;
        entry->ae_set |= AE_SET_INO;
        entry->ae_stat.aest_ino = ino;
@@ -988,6 +994,9 @@ archive_entry_set_ino(struct archive_entry *entry, la_int64_t ino)
 void
 archive_entry_set_ino64(struct archive_entry *entry, la_int64_t ino)
 {
+       if (ino < 0) {
+               ino = 0;
+       }
        entry->stat_valid = 0;
        entry->ae_set |= AE_SET_INO;
        entry->ae_stat.aest_ino = ino;
@@ -1343,6 +1352,9 @@ archive_entry_set_rdevminor(struct archive_entry *entry, dev_t m)
 void
 archive_entry_set_size(struct archive_entry *entry, la_int64_t s)
 {
+       if (s < 0) {
+               s = 0;
+       }
        entry->stat_valid = 0;
        entry->ae_stat.aest_size = s;
        entry->ae_set |= AE_SET_SIZE;
@@ -1464,6 +1476,9 @@ _archive_entry_copy_symlink_l(struct archive_entry *entry,
 void
 archive_entry_set_uid(struct archive_entry *entry, la_int64_t u)
 {
+       if (u < 0) {
+               u = 0;
+       }
        entry->stat_valid = 0;
        entry->ae_stat.aest_uid = u;
        entry->ae_set |= AE_SET_UID;

diff --git a/libarchive/archive_read_support_format_cpio.c b/libarchive/archive_read_support_format_cpio.c
index dcff23f694a72c053ca6b0d958f3a2924b778a4e..69752cbb0cacf55a7d589ab5611280550bbd29c4 100644 (file)
--- a/libarchive/archive_read_support_format_cpio.c
+++ b/libarchive/archive_read_support_format_cpio.c
@@ -834,6 +834,7 @@ static int
 header_afiol(struct archive_read *a, struct cpio *cpio,
     struct archive_entry *entry, size_t *namelength, size_t *name_pad)
 {
+       int64_t t;
        const void *h;
        const char *header;
 
@@ -850,7 +851,12 @@ header_afiol(struct archive_read *a, struct cpio *cpio,
 
        archive_entry_set_dev(entry, 
                (dev_t)atol16(header + afiol_dev_offset, afiol_dev_size));
-       archive_entry_set_ino(entry, atol16(header + afiol_ino_offset, afiol_ino_size));
+       t = atol16(header + afiol_ino_offset, afiol_ino_size);
+       if (t < 0) {
+               archive_set_error(&a->archive, 0, "Nonsensical ino value");
+               return (ARCHIVE_FATAL);
+       }
+       archive_entry_set_ino(entry, t);
        archive_entry_set_mode(entry,
                (mode_t)atol8(header + afiol_mode_offset, afiol_mode_size));
        archive_entry_set_uid(entry, atol16(header + afiol_uid_offset, afiol_uid_size));
@@ -863,8 +869,12 @@ header_afiol(struct archive_read *a, struct cpio *cpio,
        *namelength = (size_t)atol16(header + afiol_namesize_offset, afiol_namesize_size);
        *name_pad = 0; /* No padding of filename. */
 
-       cpio->entry_bytes_remaining =
-           atol16(header + afiol_filesize_offset, afiol_filesize_size);
+       t = atol16(header + afiol_filesize_offset, afiol_filesize_size);
+       if (t < 0) {
+               archive_set_error(&a->archive, 0, "Nonsensical file size");
+               return (ARCHIVE_FATAL);
+       }
+       cpio->entry_bytes_remaining = t;
        archive_entry_set_size(entry, cpio->entry_bytes_remaining);
        cpio->entry_padding = 0;
        __archive_read_consume(a, afiol_header_size);
@@ -1002,7 +1012,7 @@ be4(const unsigned char *p)
 static int64_t
 atol8(const char *p, unsigned char_cnt)
 {
-       int64_t l;
+       uint64_t l;
        int digit;
 
        l = 0;
@@ -1010,18 +1020,18 @@ atol8(const char *p, unsigned char_cnt)
                if (*p >= '0' && *p <= '7')
                        digit = *p - '0';
                else
-                       return (l);
+                       return ((int64_t)l);
                p++;
                l <<= 3;
                l |= digit;
        }
-       return (l);
+       return ((int64_t)l);
 }
 
 static int64_t
 atol16(const char *p, unsigned char_cnt)
 {
-       int64_t l;
+       uint64_t l;
        int digit;
 
        l = 0;
@@ -1033,12 +1043,12 @@ atol16(const char *p, unsigned char_cnt)
                else if (*p >= '0' && *p <= '9')
                        digit = *p - '0';
                else
-                       return (l);
+                       return ((int64_t)l);
                p++;
                l <<= 4;
                l |= digit;
        }
-       return (l);
+       return ((int64_t)l);
 }
 
 static int

diff --git a/libarchive/test/test_read_format_cpio_afio.c b/libarchive/test/test_read_format_cpio_afio.c
index 0eff8cfee856c103e9626c1cc9330a9946800823..5d2b4b4c8134c4999837c311679b66f33a0b0b65 100644 (file)
--- a/libarchive/test/test_read_format_cpio_afio.c
+++ b/libarchive/test/test_read_format_cpio_afio.c
@@ -65,15 +65,6 @@ static unsigned char archive[] = {
 0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,
 };
 
-/*
- * XXX This must be removed when we use int64_t for uid.
- */
-static int
-uid_size(void)
-{
-       return (sizeof(uid_t));
-}
-
 DEFINE_TEST(test_read_format_cpio_afio)
 {
        unsigned char *p;
@@ -106,8 +97,7 @@ DEFINE_TEST(test_read_format_cpio_afio)
         */
        assertEqualIntA(a, ARCHIVE_OK, archive_read_next_header(a, &ae));
        assertEqualInt(17, archive_entry_size(ae));
-       if (uid_size() > 4)
-               assertEqualInt(65536, archive_entry_uid(ae));
+       assertEqualInt(65536, archive_entry_uid(ae));
        assertEqualInt(archive_entry_is_encrypted(ae), 0);
        assertEqualIntA(a, archive_read_has_encrypted_entries(a), ARCHIVE_READ_FORMAT_ENCRYPTION_UNSUPPORTED);
        assertA(archive_filter_code(a, 0) == ARCHIVE_FILTER_NONE);
@@ -117,3 +107,21 @@ DEFINE_TEST(test_read_format_cpio_afio)
 
        free(p);
 }
+
+// From OSS Fuzz Issue 70019:
+static unsigned char archive2[] = "070727bbbBbbbBabbbbbbcbcbbbbbbm726f777f777ffffffff518402ffffbbbabDDDDDDDDD7c7Ddd7DDDDnDDDdDDDB7777s77777777777C7727:";
+
+DEFINE_TEST(test_read_format_cpio_afio_broken)
+{
+       struct archive *a;
+       struct archive_entry *ae;
+
+       assert((a = archive_read_new()) != NULL);
+       assertEqualIntA(a, ARCHIVE_OK, archive_read_support_filter_all(a));
+       assertEqualIntA(a, ARCHIVE_OK, archive_read_support_format_all(a));
+       assertEqualIntA(a, ARCHIVE_OK, archive_read_open_memory(a, archive2, sizeof(archive2)));
+       assertEqualIntA(a, ARCHIVE_FATAL, archive_read_next_header(a, &ae));
+       assertEqualInt(archive_filter_code(a, 0), ARCHIVE_FILTER_NONE);
+       assertEqualInt(archive_format(a), ARCHIVE_FORMAT_CPIO_AFIO_LARGE);
+       archive_read_free(a);
+}

diff --git a/libarchive/test/test_read_format_mtree.c b/libarchive/test/test_read_format_mtree.c
index 70730839386d2005f6915383a8bfefe05d5aaf9b..b4463298dde289f379e6cf5c35b41758517cd93b 100644 (file)
--- a/libarchive/test/test_read_format_mtree.c
+++ b/libarchive/test/test_read_format_mtree.c
@@ -158,7 +158,7 @@ test_read_format_mtree1(void)
        /* TODO: Mtree reader should probably return ARCHIVE_WARN for this. */
        assertEqualIntA(a, ARCHIVE_OK, archive_read_next_header(a, &ae));
        assertEqualString(archive_entry_pathname(ae), "dir2/toosmallfile");
-       assertEqualInt(archive_entry_size(ae), -1);
+       assertEqualInt(archive_entry_size(ae), 0);
        assertEqualInt(archive_entry_is_encrypted(ae), 0);
        assertEqualIntA(a, archive_read_has_encrypted_entries(a), ARCHIVE_READ_FORMAT_ENCRYPTION_UNSUPPORTED);