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

Commit b17bf521 authored by Elliott Hughes's avatar Elliott Hughes
Browse files

libziparchive: report errors on over-long names. 

Switch FindEntry and the ZipString constructor to std::string_view. This
lets us accept an over-long name so that we can reject it as too long.

Also fastboot changes to track the API change.

Bug: http://b/129068177
Test: treehugger
Change-Id: I7df7acd1fe2c46380b789c25f8909e0553e2d55e
parent 7046bcb4

fastboot/fastboot.cpp

+2 −4
Original line number Diff line number Diff line
@@ -502,9 +502,8 @@ static std::vector<char> LoadBootableImage(const std::string& kernel, const std: 


static bool UnzipToMemory(ZipArchiveHandle zip, const std::string& entry_name,

                          std::vector<char>* out) {

    ZipString zip_entry_name(entry_name.c_str());

    ZipEntry zip_entry;

    if (FindEntry(zip, zip_entry_name, &zip_entry) != 0) {

    if (FindEntry(zip, entry_name, &zip_entry) != 0) {

        fprintf(stderr, "archive does not contain '%s'\n", entry_name.c_str());

        return false;

    }
@@ -614,9 +613,8 @@ static void delete_fbemarker_tmpdir(const std::string& dir) { 
static int unzip_to_file(ZipArchiveHandle zip, const char* entry_name) {

    unique_fd fd(make_temporary_fd(entry_name));



    ZipString zip_entry_name(entry_name);

    ZipEntry zip_entry;

    if (FindEntry(zip, zip_entry_name, &zip_entry) != 0) {

    if (FindEntry(zip, entry_name, &zip_entry) != 0) {

        fprintf(stderr, "archive does not contain '%s'\n", entry_name);

        errno = ENOENT;

        return -1;

libziparchive/include/ziparchive/zip_archive.h

+7 −6
Original line number Diff line number Diff line
@@ -25,6 +25,8 @@ 
#include <sys/cdefs.h>

#include <sys/types.h>



#include <string_view>



#include "android-base/off64_t.h"



/* Zip compression methods we support */
@@ -39,10 +41,7 @@ struct ZipString { 


  ZipString() {}



  /*

   * entry_name has to be an c-style string with only ASCII characters.

   */

  explicit ZipString(const char* entry_name);

  explicit ZipString(std::string_view entry_name);



  bool operator==(const ZipString& rhs) const {

    return name && (name_length == rhs.name_length) && (memcmp(name, rhs.name, name_length) == 0);
@@ -149,8 +148,7 @@ int32_t OpenArchiveFromMemory(void* address, size_t length, const char* debugFil 
void CloseArchive(ZipArchiveHandle archive);



/*

 * Find an entry in the Zip archive, by name. |entryName| must be a null

 * terminated string, and |data| must point to a writeable memory location.

 * Find an entry in the Zip archive, by name. |data| must be non-null.

 *

 * Returns 0 if an entry is found, and populates |data| with information

 * about this entry. Returns negative values otherwise.
@@ -164,6 +162,8 @@ void CloseArchive(ZipArchiveHandle archive); 
 * On non-Windows platforms this method does not modify internal state and

 * can be called concurrently.

 */

int32_t FindEntry(const ZipArchiveHandle archive, const std::string_view entryName, ZipEntry* data);

// TODO: remove this internally, where there is a new user.

int32_t FindEntry(const ZipArchiveHandle archive, const ZipString& entryName, ZipEntry* data);



/*
@@ -179,6 +179,7 @@ int32_t FindEntry(const ZipArchiveHandle archive, const ZipString& entryName, Zi 
 *

 * Returns 0 on success and negative values on failure.

 */

// TODO: switch these ZipStrings to std::string_view.

int32_t StartIteration(ZipArchiveHandle archive, void** cookie_ptr,

                       const ZipString* optional_prefix, const ZipString* optional_suffix);

libziparchive/zip_archive.cc

+22 −4
Original line number Diff line number Diff line
@@ -690,8 +690,7 @@ static int32_t FindEntry(const ZipArchive* archive, const int32_t ent, ZipEntry* 


struct IterationHandle {

  uint32_t position;

  // We're not using vector here because this code is used in the Windows SDK

  // where the STL is not available.

  // TODO: switch these to std::string now that Windows uses libc++ too.

  ZipString prefix;

  ZipString suffix;

  ZipArchive* archive;
@@ -742,6 +741,7 @@ void EndIteration(void* cookie) { 
  delete reinterpret_cast<IterationHandle*>(cookie);

}



// TODO: remove this internally.

int32_t FindEntry(const ZipArchiveHandle archive, const ZipString& entryName, ZipEntry* data) {

  if (entryName.name_length == 0) {

    ALOGW("Zip: Invalid filename %.*s", entryName.name_length, entryName.name);
@@ -758,6 +758,23 @@ int32_t FindEntry(const ZipArchiveHandle archive, const ZipString& entryName, Zi 
  return FindEntry(archive, static_cast<uint32_t>(ent), data);

}



int32_t FindEntry(const ZipArchiveHandle archive, const std::string_view entryName,

                  ZipEntry* data) {

  if (entryName.empty() || entryName.size() > static_cast<size_t>(UINT16_MAX)) {

    ALOGW("Zip: Invalid filename of length %zu", entryName.size());

    return kInvalidEntryName;

  }



  const int64_t ent = EntryToIndex(archive->hash_table, archive->hash_table_size,

                                   ZipString(entryName), archive->central_directory.GetBasePtr());

  if (ent < 0) {

    ALOGV("Zip: Could not find entry %.*s", static_cast<int>(entryName.size()), entryName.data());

    return static_cast<int32_t>(ent);  // kEntryNotFound is safe to truncate.

  }

  // We know there are at most hast_table_size entries, safe to truncate.

  return FindEntry(archive, static_cast<uint32_t>(ent), data);

}



int32_t Next(void* cookie, ZipEntry* data, ZipString* name) {

  IterationHandle* handle = reinterpret_cast<IterationHandle*>(cookie);

  if (handle == NULL) {
@@ -1152,8 +1169,9 @@ int GetFileDescriptor(const ZipArchiveHandle archive) { 
  return archive->mapped_zip.GetFileDescriptor();

}



ZipString::ZipString(const char* entry_name) : name(reinterpret_cast<const uint8_t*>(entry_name)) {

  size_t len = strlen(entry_name);

ZipString::ZipString(std::string_view entry_name)

    : name(reinterpret_cast<const uint8_t*>(entry_name.data())) {

  size_t len = entry_name.size();

  CHECK_LE(len, static_cast<size_t>(UINT16_MAX));

  name_length = static_cast<uint16_t>(len);

}

libziparchive/zip_archive_benchmark.cpp

+1 −1
Original line number Diff line number Diff line
@@ -55,7 +55,7 @@ static void FindEntry_no_match(benchmark::State& state) { 


  // In order to walk through all file names in the archive, look for a name

  // that does not exist in the archive.

  ZipString name("thisFileNameDoesNotExist");

  std::string_view name("thisFileNameDoesNotExist");



  // Start the benchmark.

  while (state.KeepRunning()) {

libziparchive/zip_archive_test.cc

+31 −38
Original line number Diff line number Diff line
@@ -64,12 +64,6 @@ static int32_t OpenArchiveWrapper(const std::string& name, ZipArchiveHandle* han 
  return OpenArchive(abs_path.c_str(), handle);

}



static void SetZipString(ZipString* zip_str, const std::string& str) {

  zip_str->name = reinterpret_cast<const uint8_t*>(str.c_str());

  CHECK_LE(str.size(), std::numeric_limits<uint16_t>::max());

  zip_str->name_length = static_cast<uint16_t>(str.size());

}



TEST(ziparchive, Open) {

  ZipArchiveHandle handle;

  ASSERT_EQ(0, OpenArchiveWrapper(kValidZip, &handle));
@@ -192,9 +186,7 @@ TEST(ziparchive, FindEntry) { 
  ASSERT_EQ(0, OpenArchiveWrapper(kValidZip, &handle));



  ZipEntry data;

  ZipString name;

  SetZipString(&name, kATxtName);

  ASSERT_EQ(0, FindEntry(handle, name, &data));

  ASSERT_EQ(0, FindEntry(handle, kATxtName, &data));



  // Known facts about a.txt, from zipinfo -v.

  ASSERT_EQ(63, data.offset);
@@ -205,9 +197,28 @@ TEST(ziparchive, FindEntry) { 
  ASSERT_EQ(static_cast<uint32_t>(0x438a8005), data.mod_time);



  // An entry that doesn't exist. Should be a negative return code.

  ZipString absent_name;

  SetZipString(&absent_name, kNonexistentTxtName);

  ASSERT_LT(FindEntry(handle, absent_name, &data), 0);

  ASSERT_LT(FindEntry(handle, kNonexistentTxtName, &data), 0);



  CloseArchive(handle);

}



TEST(ziparchive, FindEntry_empty) {

  ZipArchiveHandle handle;

  ASSERT_EQ(0, OpenArchiveWrapper(kValidZip, &handle));



  ZipEntry data;

  ASSERT_EQ(kInvalidEntryName, FindEntry(handle, "", &data));



  CloseArchive(handle);

}



TEST(ziparchive, FindEntry_too_long) {

  ZipArchiveHandle handle;

  ASSERT_EQ(0, OpenArchiveWrapper(kValidZip, &handle));



  std::string very_long_name(65536, 'x');

  ZipEntry data;

  ASSERT_EQ(kInvalidEntryName, FindEntry(handle, very_long_name, &data));



  CloseArchive(handle);

}
@@ -234,9 +245,7 @@ TEST(ziparchive, ExtractToMemory) { 


  // An entry that's deflated.

  ZipEntry data;

  ZipString a_name;

  SetZipString(&a_name, kATxtName);

  ASSERT_EQ(0, FindEntry(handle, a_name, &data));

  ASSERT_EQ(0, FindEntry(handle, kATxtName, &data));

  const uint32_t a_size = data.uncompressed_length;

  ASSERT_EQ(a_size, kATxtContents.size());

  uint8_t* buffer = new uint8_t[a_size];
@@ -245,9 +254,7 @@ TEST(ziparchive, ExtractToMemory) { 
  delete[] buffer;



  // An entry that's stored.

  ZipString b_name;

  SetZipString(&b_name, kBTxtName);

  ASSERT_EQ(0, FindEntry(handle, b_name, &data));

  ASSERT_EQ(0, FindEntry(handle, kBTxtName, &data));

  const uint32_t b_size = data.uncompressed_length;

  ASSERT_EQ(b_size, kBTxtContents.size());

  buffer = new uint8_t[b_size];
@@ -302,9 +309,7 @@ TEST(ziparchive, EmptyEntries) { 
  ASSERT_EQ(0, OpenArchiveFd(tmp_file.fd, "EmptyEntriesTest", &handle, false));



  ZipEntry entry;

  ZipString empty_name;

  SetZipString(&empty_name, kEmptyTxtName);

  ASSERT_EQ(0, FindEntry(handle, empty_name, &entry));

  ASSERT_EQ(0, FindEntry(handle, kEmptyTxtName, &entry));

  ASSERT_EQ(static_cast<uint32_t>(0), entry.uncompressed_length);

  uint8_t buffer[1];

  ASSERT_EQ(0, ExtractToMemory(handle, &entry, buffer, 1));
@@ -327,9 +332,7 @@ TEST(ziparchive, EntryLargerThan32K) { 
  ASSERT_EQ(0, OpenArchiveFd(tmp_file.fd, "EntryLargerThan32KTest", &handle, false));



  ZipEntry entry;

  ZipString ab_name;

  SetZipString(&ab_name, kAbTxtName);

  ASSERT_EQ(0, FindEntry(handle, ab_name, &entry));

  ASSERT_EQ(0, FindEntry(handle, kAbTxtName, &entry));

  ASSERT_EQ(kAbUncompressedSize, entry.uncompressed_length);



  // Extract the entry to memory.
@@ -386,9 +389,7 @@ TEST(ziparchive, ExtractToFile) { 
  ASSERT_EQ(0, OpenArchiveWrapper(kValidZip, &handle));



  ZipEntry entry;

  ZipString name;

  SetZipString(&name, kATxtName);

  ASSERT_EQ(0, FindEntry(handle, name, &entry));

  ASSERT_EQ(0, FindEntry(handle, kATxtName, &entry));

  ASSERT_EQ(0, ExtractEntryToFile(handle, &entry, tmp_file.fd));



  // Assert that the first 8 bytes of the file haven't been clobbered.
@@ -424,10 +425,8 @@ TEST(ziparchive, OpenFromMemory) { 
            OpenArchiveFromMemory(file_map->data(), file_map->size(), zip_path.c_str(), &handle));



  // Assert one entry can be found and extracted correctly.

  std::string BINARY_PATH("META-INF/com/google/android/update-binary");

  ZipString binary_path(BINARY_PATH.c_str());

  ZipEntry binary_entry;

  ASSERT_EQ(0, FindEntry(handle, binary_path, &binary_entry));

  ASSERT_EQ(0, FindEntry(handle, "META-INF/com/google/android/update-binary", &binary_entry));

  TemporaryFile tmp_binary;

  ASSERT_NE(-1, tmp_binary.fd);

  ASSERT_EQ(0, ExtractEntryToFile(handle, &binary_entry, tmp_binary.fd));
@@ -436,9 +435,7 @@ TEST(ziparchive, OpenFromMemory) { 


static void ZipArchiveStreamTest(ZipArchiveHandle& handle, const std::string& entry_name, bool raw,

                                 bool verified, ZipEntry* entry, std::vector<uint8_t>* read_data) {

  ZipString name;

  SetZipString(&name, entry_name);

  ASSERT_EQ(0, FindEntry(handle, name, entry));

  ASSERT_EQ(0, FindEntry(handle, entry_name, entry));

  std::unique_ptr<ZipArchiveStreamEntry> stream;

  if (raw) {

    stream.reset(ZipArchiveStreamEntry::CreateRaw(handle, *entry));
@@ -589,11 +586,7 @@ static void ExtractEntryToMemory(const std::vector<uint8_t>& zip_data, 
  // an entry whose name is "name" and whose size is 12 (contents =

  // "abdcdefghijk").

  ZipEntry entry;

  ZipString name;

  std::string name_str = "name";

  SetZipString(&name, name_str);



  ASSERT_EQ(0, FindEntry(handle, name, &entry));

  ASSERT_EQ(0, FindEntry(handle, "name", &entry));

  ASSERT_EQ(static_cast<uint32_t>(12), entry.uncompressed_length);



  entry_out->resize(12);