Merge "libziparchive: report errors on over-long names." (d62b9ca2) · Commits · e / os / android_system_core

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);