Merge "libziparchive: report errors on over-long names."

am: d62b9ca271

Change-Id: I7e492b3fa8b74ff9c41363d4b1ffc9b401eff660
This commit is contained in:
Elliott Hughes 2019-05-06 17:21:12 -07:00 committed by android-build-merger
commit d43d5baed5
6 changed files with 76 additions and 66 deletions

View file

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

View file

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

View file

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

View file

@ -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()) {

View file

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

View file

@ -62,7 +62,7 @@ TEST_F(zipwriter, WriteUncompressedZipWithOneFile) {
ASSERT_EQ(0, OpenArchiveFd(fd_, "temp", &handle, false));
ZipEntry data;
ASSERT_EQ(0, FindEntry(handle, ZipString("file.txt"), &data));
ASSERT_EQ(0, FindEntry(handle, "file.txt", &data));
EXPECT_EQ(kCompressStored, data.method);
EXPECT_EQ(0u, data.has_data_descriptor);
EXPECT_EQ(strlen(expected), data.compressed_length);
@ -95,19 +95,19 @@ TEST_F(zipwriter, WriteUncompressedZipWithMultipleFiles) {
ZipEntry data;
ASSERT_EQ(0, FindEntry(handle, ZipString("file.txt"), &data));
ASSERT_EQ(0, FindEntry(handle, "file.txt", &data));
EXPECT_EQ(kCompressStored, data.method);
EXPECT_EQ(2u, data.compressed_length);
ASSERT_EQ(2u, data.uncompressed_length);
ASSERT_TRUE(AssertFileEntryContentsEq("he", handle, &data));
ASSERT_EQ(0, FindEntry(handle, ZipString("file/file.txt"), &data));
ASSERT_EQ(0, FindEntry(handle, "file/file.txt", &data));
EXPECT_EQ(kCompressStored, data.method);
EXPECT_EQ(3u, data.compressed_length);
ASSERT_EQ(3u, data.uncompressed_length);
ASSERT_TRUE(AssertFileEntryContentsEq("llo", handle, &data));
ASSERT_EQ(0, FindEntry(handle, ZipString("file/file2.txt"), &data));
ASSERT_EQ(0, FindEntry(handle, "file/file2.txt", &data));
EXPECT_EQ(kCompressStored, data.method);
EXPECT_EQ(0u, data.compressed_length);
EXPECT_EQ(0u, data.uncompressed_length);
@ -129,7 +129,7 @@ TEST_F(zipwriter, WriteUncompressedZipFileWithAlignedFlag) {
ASSERT_EQ(0, OpenArchiveFd(fd_, "temp", &handle, false));
ZipEntry data;
ASSERT_EQ(0, FindEntry(handle, ZipString("align.txt"), &data));
ASSERT_EQ(0, FindEntry(handle, "align.txt", &data));
EXPECT_EQ(0, data.offset & 0x03);
CloseArchive(handle);
@ -163,7 +163,7 @@ TEST_F(zipwriter, WriteUncompressedZipFileWithAlignedFlagAndTime) {
ASSERT_EQ(0, OpenArchiveFd(fd_, "temp", &handle, false));
ZipEntry data;
ASSERT_EQ(0, FindEntry(handle, ZipString("align.txt"), &data));
ASSERT_EQ(0, FindEntry(handle, "align.txt", &data));
EXPECT_EQ(0, data.offset & 0x03);
struct tm mod = data.GetModificationTime();
@ -191,7 +191,7 @@ TEST_F(zipwriter, WriteUncompressedZipFileWithAlignedValue) {
ASSERT_EQ(0, OpenArchiveFd(fd_, "temp", &handle, false));
ZipEntry data;
ASSERT_EQ(0, FindEntry(handle, ZipString("align.txt"), &data));
ASSERT_EQ(0, FindEntry(handle, "align.txt", &data));
EXPECT_EQ(0, data.offset & 0xfff);
CloseArchive(handle);
@ -213,7 +213,7 @@ TEST_F(zipwriter, WriteUncompressedZipFileWithAlignedValueAndTime) {
ASSERT_EQ(0, OpenArchiveFd(fd_, "temp", &handle, false));
ZipEntry data;
ASSERT_EQ(0, FindEntry(handle, ZipString("align.txt"), &data));
ASSERT_EQ(0, FindEntry(handle, "align.txt", &data));
EXPECT_EQ(0, data.offset & 0xfff);
struct tm mod = data.GetModificationTime();
@ -241,7 +241,7 @@ TEST_F(zipwriter, WriteCompressedZipWithOneFile) {
ASSERT_EQ(0, OpenArchiveFd(fd_, "temp", &handle, false));
ZipEntry data;
ASSERT_EQ(0, FindEntry(handle, ZipString("file.txt"), &data));
ASSERT_EQ(0, FindEntry(handle, "file.txt", &data));
EXPECT_EQ(kCompressDeflated, data.method);
ASSERT_EQ(4u, data.uncompressed_length);
ASSERT_TRUE(AssertFileEntryContentsEq("helo", handle, &data));
@ -273,7 +273,7 @@ TEST_F(zipwriter, WriteCompressedZipFlushFull) {
ASSERT_EQ(0, OpenArchiveFd(fd_, "temp", &handle, false));
ZipEntry data;
ASSERT_EQ(0, FindEntry(handle, ZipString("file.txt"), &data));
ASSERT_EQ(0, FindEntry(handle, "file.txt", &data));
EXPECT_EQ(kCompressDeflated, data.method);
EXPECT_EQ(kBufSize, data.uncompressed_length);
@ -340,12 +340,12 @@ TEST_F(zipwriter, BackupRemovesTheLastFile) {
ASSERT_EQ(0, OpenArchiveFd(fd_, "temp", &handle, false));
ZipEntry data;
ASSERT_EQ(0, FindEntry(handle, ZipString("keep.txt"), &data));
ASSERT_EQ(0, FindEntry(handle, "keep.txt", &data));
ASSERT_TRUE(AssertFileEntryContentsEq(kKeepThis, handle, &data));
ASSERT_NE(0, FindEntry(handle, ZipString("drop.txt"), &data));
ASSERT_NE(0, FindEntry(handle, "drop.txt", &data));
ASSERT_EQ(0, FindEntry(handle, ZipString("replace.txt"), &data));
ASSERT_EQ(0, FindEntry(handle, "replace.txt", &data));
ASSERT_TRUE(AssertFileEntryContentsEq(kReplaceWithThis, handle, &data));
CloseArchive(handle);