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.
Merged-In: Ibf543a7da3d7898952e9eb332c84cdfc67cf5aa4
Change-Id: Ibf543a7da3d7898952e9eb332c84cdfc67cf5aa4
(cherry picked from commit bcc4431f24
)
This commit is contained in:
parent
fba1a36fd9
commit
9e020e2d11
3 changed files with 13 additions and 0 deletions
BIN
libziparchive/testdata/bad_filename.zip
vendored
Normal file
BIN
libziparchive/testdata/bad_filename.zip
vendored
Normal file
Binary file not shown.
|
@ -316,6 +316,11 @@ static int32_t ParseZipArchive(ZipArchive* archive) {
|
|||
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
|
||||
|
@ -348,6 +353,11 @@ static int32_t ParseZipArchive(ZipArchive* archive) {
|
|||
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;
|
||||
|
|
|
@ -41,6 +41,7 @@ static const std::string kValidZip = "valid.zip";
|
|||
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::string kUpdateZip = "dummy-update.zip";
|
||||
|
||||
static const std::vector<uint8_t> kATxtContents {
|
||||
|
@ -86,7 +87,9 @@ static void SetZipString(ZipString* zip_str, const std::string& str) {
|
|||
TEST(ziparchive, Open) {
|
||||
ZipArchiveHandle handle;
|
||||
ASSERT_EQ(0, OpenArchiveWrapper(kValidZip, &handle));
|
||||
CloseArchive(handle);
|
||||
|
||||
ASSERT_EQ(-1, OpenArchiveWrapper(kBadFilenameZip, &handle));
|
||||
CloseArchive(handle);
|
||||
}
|
||||
|
||||
|
|
Loading…
Reference in a new issue