Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 67897d47 authored by Treehugger Robot's avatar Treehugger Robot Committed by Gerrit Code Review
Browse files

Merge changes Ie89f709b,Ibf543a7d,I1d8092a1 

* changes:
  Fix out of bound read in libziparchive
  Check filename memory bound when parsing ziparchive
  Fix out of bound access in libziparchive
parents 7dbf1a18 0fda1cf6

libziparchive/testdata/bad_filename.zip

0 → 100644
+4.02 KiB

File added.

No diff preview for this file type.

View file

libziparchive/testdata/crash.apk

0 → 100644
+154 B

File added.

No diff preview for this file type.

View file

libziparchive/zip_archive.cc

+18 −5
Original line number Original line Diff line number Diff line
@@ -316,6 +316,11 @@ static int32_t ParseZipArchive(ZipArchive* archive) { 
  archive->hash_table_size = RoundUpPower2(1 + (num_entries * 4) / 3);

  archive->hash_table_size = RoundUpPower2(1 + (num_entries * 4) / 3);

  archive->hash_table = reinterpret_cast<ZipString*>(calloc(archive->hash_table_size,

  archive->hash_table = reinterpret_cast<ZipString*>(calloc(archive->hash_table_size,

      sizeof(ZipString)));

      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

   * 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* const cd_end = cd_ptr + cd_length;

  const uint8_t* ptr = cd_ptr;

  const uint8_t* ptr = cd_ptr;

  for (uint16_t i = 0; i < num_entries; i++) {

  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 =

    const CentralDirectoryRecord* cdr =

        reinterpret_cast<const CentralDirectoryRecord*>(ptr);

        reinterpret_cast<const CentralDirectoryRecord*>(ptr);

    if (cdr->record_signature != CentralDirectoryRecord::kSignature) {

    if (cdr->record_signature != CentralDirectoryRecord::kSignature) {
@@ -331,11 +344,6 @@ static int32_t ParseZipArchive(ZipArchive* archive) { 
      return -1;

      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;

    const off64_t local_header_offset = cdr->local_file_header_offset;

    if (local_header_offset >= archive->directory_offset) {

    if (local_header_offset >= archive->directory_offset) {

      ALOGW("Zip: bad LFH offset %" PRId64 " at entry %" PRIu16,

      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 uint16_t comment_length = cdr->comment_length;

    const uint8_t* file_name = ptr + sizeof(CentralDirectoryRecord);

    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 */

    /* check that file name is valid UTF-8 and doesn't contain NUL (U+0000) characters */

    if (!IsValidEntryName(file_name, file_name_length)) {

    if (!IsValidEntryName(file_name, file_name_length)) {

      return -1;

      return -1;

libziparchive/zip_archive_test.cc

+10 −0
Original line number Original line Diff line number Diff line
@@ -40,6 +40,8 @@ static const std::string kMissingZip = "missing.zip"; 
static const std::string kValidZip = "valid.zip";

static const std::string kValidZip = "valid.zip";

static const std::string kLargeZip = "large.zip";

static const std::string kLargeZip = "large.zip";

static const std::string kBadCrcZip = "bad_crc.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::string kUpdateZip = "dummy-update.zip";





static const std::vector<uint8_t> kATxtContents {

static const std::vector<uint8_t> kATxtContents {
@@ -85,7 +87,15 @@ static void SetZipString(ZipString* zip_str, const std::string& str) { 
TEST(ziparchive, Open) {

TEST(ziparchive, Open) {

  ZipArchiveHandle handle;

  ZipArchiveHandle handle;

  ASSERT_EQ(0, OpenArchiveWrapper(kValidZip, &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);

  CloseArchive(handle);

}

}