ACPR/android_system_core
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

Merge changes Ie89f709b,Ibf543a7d,I1d8092a1

Browse source 
* changes:
  Fix out of bound read in libziparchive
  Check filename memory bound when parsing ziparchive
  Fix out of bound access in libziparchive
This commit is contained in:
Treehugger Robot 2017-06-30 15:38:13 +00:00 • committed by Gerrit Code Review
commit 67897d47ce
4 changed files with 28 additions and 5 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

BIN
libziparchive/testdata/bad_filename.zip vendored Normal file
View file

Binary file not shown.

BIN
libziparchive/testdata/crash.apk vendored Normal file
View file

Binary file not shown.

23
libziparchive/zip_archive.cc
View file

@ -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
@ -324,6 +329,14 @@ static int32_t ParseZipArchive(ZipArchive* archive) {
const uint8_t* const cd_end = cd_ptr + cd_length;
const uint8_t* ptr = cd_ptr;
for (uint16_t i = 0; i < num_entries; i++) {
if (ptr > cd_end - sizeof(CentralDirectoryRecord)) {
ALOGW("Zip: ran off the end (at %" PRIu16 ")", i);
#if defined(__ANDROID__)
android_errorWriteLog(0x534e4554, "36392138");
#endif
return -1;
}
const CentralDirectoryRecord* cdr =
reinterpret_cast<const CentralDirectoryRecord*>(ptr);
if (cdr->record_signature != CentralDirectoryRecord::kSignature) {
@ -331,11 +344,6 @@ static int32_t ParseZipArchive(ZipArchive* archive) {
return -1;
}
if (ptr + sizeof(CentralDirectoryRecord) > cd_end) {
ALOGW("Zip: ran off the end (at %" PRIu16 ")", i);
return -1;
}
const off64_t local_header_offset = cdr->local_file_header_offset;
if (local_header_offset >= archive->directory_offset) {
ALOGW("Zip: bad LFH offset %" PRId64 " at entry %" PRIu16,
@ -348,6 +356,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;

10
libziparchive/zip_archive_test.cc
View file

@ -40,6 +40,8 @@ static const std::string kMissingZip = "missing.zip";
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 {
@ -85,7 +87,15 @@ 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);
}
TEST(ziparchive, OutOfBound) {
ZipArchiveHandle handle;
ASSERT_EQ(-8, OpenArchiveWrapper(kCrashApk, &handle));
CloseArchive(handle);
}