Fix out of bound access in libziparchive

The boundary check of an invalid EOCD record may succeed due to the
overflow of uint32_t. Fix the check and add a unit test.

Test: Open the crash.apk and libziparchive reports the offset error as expected.

Bug: 31251826
Change-Id: I1d8092a19b73886a671bc9d291cfc27d65e3d236
This commit is contained in:
Tianjie Xu 2016-09-21 14:58:11 -07:00
parent 7c9800dfe6
commit ae8180c06d
3 changed files with 13 additions and 1 deletions

BIN
libziparchive/testdata/crash.apk vendored Normal file

Binary file not shown.

View File

@ -270,9 +270,14 @@ static int32_t MapCentralDirectory0(int fd, const char* debug_file_name,
* Grab the CD offset and size, and the number of entries in the
* archive and verify that they look reasonable.
*/
if (eocd->cd_start_offset + eocd->cd_size > eocd_offset) {
if (static_cast<off64_t>(eocd->cd_start_offset) + eocd->cd_size > eocd_offset) {
ALOGW("Zip: bad offsets (dir %" PRIu32 ", size %" PRIu32 ", eocd %" PRId64 ")",
eocd->cd_start_offset, eocd->cd_size, static_cast<int64_t>(eocd_offset));
#if defined(__ANDROID__)
if (eocd->cd_start_offset + eocd->cd_size <= eocd_offset) {
android_errorWriteLog(0x534e4554, "31251826");
}
#endif
return kInvalidOffset;
}
if (eocd->num_records == 0) {

View File

@ -36,6 +36,7 @@ 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::vector<uint8_t> kATxtContents {
'a', 'b', 'c', 'd', 'e', 'f', 'g', 'h',
@ -84,6 +85,12 @@ TEST(ziparchive, Open) {
CloseArchive(handle);
}
TEST(ziparchive, OutOfBound) {
ZipArchiveHandle handle;
ASSERT_EQ(-8, OpenArchiveWrapper(kCrashApk, &handle));
CloseArchive(handle);
}
TEST(ziparchive, OpenMissing) {
ZipArchiveHandle handle;
ASSERT_NE(0, OpenArchiveWrapper(kMissingZip, &handle));