From 48953a1b8fdcf1d6fa1aeeb40c57821d33fc87d2 Mon Sep 17 00:00:00 2001
From: Narayan Kamath <narayan@google.com>
Date: Fri, 24 Jan 2014 12:32:39 +0000
Subject: [PATCH] Fix entry handling for 0 length entries.

Two minor issues were fixed:
- The offset to entry data can be the same as the
  central directory offset when the last entry in the
  file has length 0 and is stored (not deflated). Fix
  a check that disallowed this. We already have a strict
  check that entry data must end before the central directory,
  so we're covered.
- We would attempt to map a segment of length 0 when writing
  an entry whose length is 0. We should just return early in
  this case.

bug: 12623277
Change-Id: I2a4ca0c4d170cc3cbf326e5ca13894acd9c434c9
---
 libziparchive/zip_archive.cc      |  9 ++++++-
 libziparchive/zip_archive_test.cc | 39 ++++++++++++++++++++++++++++++-
 2 files changed, 46 insertions(+), 2 deletions(-)

diff --git a/libziparchive/zip_archive.cc b/libziparchive/zip_archive.cc
index 8436d49be..a23d4ae0b 100644
--- a/libziparchive/zip_archive.cc
+++ b/libziparchive/zip_archive.cc
@@ -756,7 +756,7 @@ static int32_t FindEntry(const ZipArchive* archive, const int ent,
   }
 
   const off64_t data_offset = local_header_offset + kLFHLen + lfhNameLen + lfhExtraLen;
-  if (data_offset >= cd_offset) {
+  if (data_offset > cd_offset) {
     ALOGW("Zip: bad data offset %lld in zip", (off64_t) data_offset);
     return kInvalidOffset;
   }
@@ -1021,6 +1021,13 @@ int32_t ExtractEntryToFile(ZipArchiveHandle handle,
     return kIoError;
   }
 
+  // Don't attempt to map a region of length 0. We still need the
+  // ftruncate() though, since the API guarantees that we will truncate
+  // the file to the end of the uncompressed output.
+  if (declared_length == 0) {
+      return 0;
+  }
+
   android::FileMap* map  = MapFileSegment(fd, current_offset, declared_length,
                                           false, kTempMappingFileName);
   if (map == NULL) {
diff --git a/libziparchive/zip_archive_test.cc b/libziparchive/zip_archive_test.cc
index 022be5b6f..30822165c 100644
--- a/libziparchive/zip_archive_test.cc
+++ b/libziparchive/zip_archive_test.cc
@@ -140,8 +140,45 @@ 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[] = {
+      0x04034b50, 0x0000000a, 0x63600000, 0x00004438, 0x00000000, 0x00000000,
+      0x00090000, 0x6d65001c, 0x2e797470, 0x55747874, 0x03000954, 0x52e25c13,
+      0x52e25c24, 0x000b7875, 0x42890401, 0x88040000, 0x50000013, 0x1e02014b,
+      0x00000a03, 0x60000000, 0x00443863, 0x00000000, 0x00000000, 0x09000000,
+      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)));
+
+  ZipArchiveHandle handle;
+  ASSERT_EQ(0, OpenArchiveFd(fd, "EmptyEntriesTest", &handle));
+
+  ZipEntry entry;
+  ASSERT_EQ(0, FindEntry(handle, "empty.txt", &entry));
+  ASSERT_EQ(static_cast<uint32_t>(0), entry.uncompressed_length);
+  uint8_t buffer[1];
+  ASSERT_EQ(0, ExtractToMemory(handle, &entry, buffer, 1));
+
+  char output_file_pattern[] = "empty_entries_output_XXXXXX";
+  int output_fd = mkstemp(output_file_pattern);
+  ASSERT_NE(-1, output_fd);
+  ASSERT_EQ(0, ExtractEntryToFile(handle, &entry, output_fd));
+
+  struct stat stat_buf;
+  ASSERT_EQ(0, fstat(output_fd, &stat_buf));
+  ASSERT_EQ(0, stat_buf.st_size);
+
+  close(fd);
+  close(output_fd);
+}
+
 TEST(ziparchive, ExtractToFile) {
-  char kTempFilePattern[] = "zip_archive_test_XXXXXX";
+  char kTempFilePattern[] = "zip_archive_input_XXXXXX";
   int fd = mkstemp(kTempFilePattern);
   ASSERT_NE(-1, fd);
   const uint8_t data[8] = { '1', '2', '3', '4', '5', '6', '7', '8' };