Check filename memory bound when parsing ziparchive
Add a check to ensure the filename boundary doesn't exceed the mapped
memory region. Also add the corresponding unit test.
Bug: 28802225
Test: New unit test passes.
Change-Id: Ibf543a7da3d7898952e9eb332c84cdfc67cf5aa4
diff --git a/libziparchive/testdata/bad_filename.zip b/libziparchive/testdata/bad_filename.zip
new file mode 100644
index 0000000..294eaf5
--- /dev/null
+++ b/libziparchive/testdata/bad_filename.zip
Binary files differ
diff --git a/libziparchive/zip_archive.cc b/libziparchive/zip_archive.cc
index a1455b0..b5b929f 100644
--- a/libziparchive/zip_archive.cc
+++ b/libziparchive/zip_archive.cc
@@ -379,6 +379,11 @@
archive->hash_table_size = RoundUpPower2(1 + (num_entries * 4) / 3);
archive->hash_table = reinterpret_cast<ZipString*>(calloc(archive->hash_table_size,
sizeof(ZipString)));
+ if (archive->hash_table == nullptr) {
+ ALOGW("Zip: unable to allocate the %u-entry hash_table, entry size: %zu",
+ archive->hash_table_size, sizeof(ZipString));
+ return -1;
+ }
/*
* Walk through the central directory, adding entries to the hash
@@ -411,6 +416,11 @@
const uint16_t comment_length = cdr->comment_length;
const uint8_t* file_name = ptr + sizeof(CentralDirectoryRecord);
+ if (file_name + file_name_length > cd_end) {
+ ALOGW("Zip: file name boundary exceeds the central directory range, file_name_length: "
+ "%" PRIx16 ", cd_length: %zu", file_name_length, cd_length);
+ return -1;
+ }
/* check that file name is valid UTF-8 and doesn't contain NUL (U+0000) characters */
if (!IsValidEntryName(file_name, file_name_length)) {
return -1;
diff --git a/libziparchive/zip_archive_test.cc b/libziparchive/zip_archive_test.cc
index 42eb5ee..4f9dd26 100644
--- a/libziparchive/zip_archive_test.cc
+++ b/libziparchive/zip_archive_test.cc
@@ -37,6 +37,7 @@
static const std::string kLargeZip = "large.zip";
static const std::string kBadCrcZip = "bad_crc.zip";
static const std::string kCrashApk = "crash.apk";
+static const std::string kBadFilenameZip = "bad_filename.zip";
static const std::vector<uint8_t> kATxtContents {
'a', 'b', 'c', 'd', 'e', 'f', 'g', 'h',
@@ -81,7 +82,9 @@
TEST(ziparchive, Open) {
ZipArchiveHandle handle;
ASSERT_EQ(0, OpenArchiveWrapper(kValidZip, &handle));
+ CloseArchive(handle);
+ ASSERT_EQ(-1, OpenArchiveWrapper(kBadFilenameZip, &handle));
CloseArchive(handle);
}