Reject zip files that have trailing bytes after their CD.

bug: 15287086
Change-Id: I03219c4c2ca6afc9d417a35bd98ae682f478fc25

2 files changed, 38 insertions, 12 deletions

diff --git a/libziparchive/zip_archive.cc b/libziparchive/zip_archive.cc
index 6781ebe..a30b9a0 100644
--- a/libziparchive/zip_archive.cc
+++ b/libziparchive/zip_archive.cc
@@ -44,11 +44,13 @@
 /*
  * Zip file constants.
  */
-static const uint32_t kEOCDSignature    = 0x06054b50;
-static const uint32_t kEOCDLen          = 2;
-static const uint32_t kEOCDNumEntries   = 8;              // offset to #of entries in file
-static const uint32_t kEOCDSize         = 12;             // size of the central directory
-static const uint32_t kEOCDFileOffset   = 16;             // offset to central directory
+static const uint32_t kEOCDSignature     = 0x06054b50;
+static const uint32_t kEOCDLen           = 2;
+static const uint32_t kEOCDNumEntries    = 8;             // number of entries in the archive
+static const uint32_t kEOCDSize          = 12;            // size of the central directory
+static const uint32_t kEOCDFileOffset    = 16;            // offset to central directory
+static const uint32_t kEOCDCommentLen    = 20;            // length of the EOCD comment
+static const uint32_t kEOCDComment       = 22;            // offset of the EOCD comment
 
 static const uint32_t kMaxCommentLen    = 65535;          // longest possible in ushort
 static const uint32_t kMaxEOCDSearch    = (kMaxCommentLen + kEOCDLen);
@@ -378,6 +380,13 @@ static int32_t MapCentralDirectory0(int fd, const char* debug_file_name,
   const uint16_t num_entries = get2LE(eocd_ptr + kEOCDNumEntries);
   const off64_t dir_size = get4LE(eocd_ptr + kEOCDSize);
   const off64_t dir_offset = get4LE(eocd_ptr + kEOCDFileOffset);
+  const uint16_t comment_length = get2LE(eocd_ptr + kEOCDCommentLen);
+
+  if (eocd_offset + comment_length + kEOCDCommentOffset != file_length) {
+    ALOGW("Zip: %" PRId64 " extraneous bytes at the end of the central directory",
+          (int64_t) (file_length - (eocd_offset + comment_length + kEOCDCommentOffset)));
+    return kInvalidFile;
+  }
 
   if (dir_offset + dir_size > eocd_offset) {
     ALOGW("Zip: bad offsets (dir %" PRId64 ", size %" PRId64 ", eocd %" PRId64 ")",
diff --git a/libziparchive/zip_archive_test.cc b/libziparchive/zip_archive_test.cc
index 2eb9318..dbf7ebf 100644
--- a/libziparchive/zip_archive_test.cc
+++ b/libziparchive/zip_archive_test.cc
@@ -140,11 +140,7 @@ TEST(ziparchive, ExtractToMemory) {
   CloseArchive(handle);
 }
 
-TEST(ziparchive, EmptyEntries) {
-  char temp_file_pattern[] = "empty_entries_test_XXXXXX";
-  int fd = mkstemp(temp_file_pattern);
-  ASSERT_NE(-1, fd);
-  const uint32_t data[] = {
+static const uint32_t kEmptyEntriesZip[] = {
       0x04034b50, 0x0000000a, 0x63600000, 0x00004438, 0x00000000, 0x00000000,
       0x00090000, 0x6d65001c, 0x2e797470, 0x55747874, 0x03000954, 0x52e25c13,
       0x52e25c24, 0x000b7875, 0x42890401, 0x88040000, 0x50000013, 0x1e02014b,
@@ -152,8 +148,13 @@ TEST(ziparchive, EmptyEntries) {
       0x00001800, 0x00000000, 0xa0000000, 0x00000081, 0x706d6500, 0x742e7974,
       0x54557478, 0x13030005, 0x7552e25c, 0x01000b78, 0x00428904, 0x13880400,
       0x4b500000, 0x00000605, 0x00010000, 0x004f0001, 0x00430000, 0x00000000 };
-  const ssize_t file_size = 168;
-  ASSERT_EQ(file_size, TEMP_FAILURE_RETRY(write(fd, data, file_size)));
+
+TEST(ziparchive, EmptyEntries) {
+  char temp_file_pattern[] = "empty_entries_test_XXXXXX";
+  int fd = mkstemp(temp_file_pattern);
+  ASSERT_NE(-1, fd);
+  const ssize_t file_size = sizeof(kEmptyEntriesZip);
+  ASSERT_EQ(file_size, TEMP_FAILURE_RETRY(write(fd, kEmptyEntriesZip, file_size)));
 
   ZipArchiveHandle handle;
   ASSERT_EQ(0, OpenArchiveFd(fd, "EmptyEntriesTest", &handle));
@@ -177,6 +178,22 @@ TEST(ziparchive, EmptyEntries) {
   close(output_fd);
 }
 
+TEST(ziparchive, TrailerAfterEOCD) {
+  char temp_file_pattern[] = "trailer_after_eocd_test_XXXXXX";
+  int fd = mkstemp(temp_file_pattern);
+  ASSERT_NE(-1, fd);
+
+  // Create a file with 8 bytes of random garbage.
+  static const uint8_t trailer[] = { 'A' ,'n', 'd', 'r', 'o', 'i', 'd', 'z' };
+  const ssize_t file_size = sizeof(kEmptyEntriesZip);
+  const ssize_t trailer_size = sizeof(trailer);
+  ASSERT_EQ(file_size, TEMP_FAILURE_RETRY(write(fd, kEmptyEntriesZip, file_size)));
+  ASSERT_EQ(trailer_size, TEMP_FAILURE_RETRY(write(fd, trailer, trailer_size)));
+
+  ZipArchiveHandle handle;
+  ASSERT_GT(0, OpenArchiveFd(fd, "EmptyEntriesTest", &handle));
+}
+
 TEST(ziparchive, ExtractToFile) {
   char kTempFilePattern[] = "zip_archive_input_XXXXXX";
   int fd = mkstemp(kTempFilePattern);