From 6fdfd58fe1e67e438fc85bb28e684a8fd3aa3dcb Mon Sep 17 00:00:00 2001 From: Tianjie Xu Date: Wed, 5 Apr 2017 14:46:27 -0700 Subject: [PATCH] Do Not Merge: Fix out of bound read in libziparchive We should check the boundary of central directory before checking its signature. Swap the order of these two checks. Bug: 36392138 Test: libziparchive doesn't read the signature after boundary check fails. Change-Id: Ie89f709bb2d1ccb647116fb7ccb1e23c943e5ab8 (cherry picked from commit 74464a1361562d4042a67c5d66bfcf396ee7e59c) --- libziparchive/zip_archive.cc | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/libziparchive/zip_archive.cc b/libziparchive/zip_archive.cc index 54d866c26..f852cff42 100644 --- a/libziparchive/zip_archive.cc +++ b/libziparchive/zip_archive.cc @@ -622,6 +622,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 + goto bail; + } + const CentralDirectoryRecord* cdr = reinterpret_cast(ptr); if (cdr->record_signature != CentralDirectoryRecord::kSignature) { @@ -629,11 +637,6 @@ static int32_t ParseZipArchive(ZipArchive* archive) { goto bail; } - if (ptr + sizeof(CentralDirectoryRecord) > cd_end) { - ALOGW("Zip: ran off the end (at %" PRIu16 ")", i); - goto bail; - } - 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, (int64_t)local_header_offset, i);