diff options
Diffstat (limited to 'libziparchive')
| -rw-r--r-- | libziparchive/Android.mk | 9 | ||||
| -rw-r--r-- | libziparchive/zip_archive.cc | 476 | ||||
| -rw-r--r-- | libziparchive/zip_archive_test.cc | 54 |
3 files changed, 339 insertions, 200 deletions
diff --git a/libziparchive/Android.mk b/libziparchive/Android.mk index e754c3b..705caa5 100644 --- a/libziparchive/Android.mk +++ b/libziparchive/Android.mk @@ -30,6 +30,7 @@ LOCAL_SHARED_LIBRARIES := libutils LOCAL_MODULE:= libziparchive LOCAL_C_INCLUDES += ${includes} +LOCAL_CFLAGS := -Werror include $(BUILD_STATIC_LIBRARY) include $(CLEAR_VARS) @@ -40,6 +41,8 @@ LOCAL_C_INCLUDES += ${includes} LOCAL_STATIC_LIBRARIES := libz libutils LOCAL_MODULE:= libziparchive-host +LOCAL_CFLAGS := -Werror +LOCAL_MULTILIB := both include $(BUILD_HOST_STATIC_LIBRARY) include $(CLEAR_VARS) @@ -47,7 +50,8 @@ LOCAL_MODULE := ziparchive-tests LOCAL_CPP_EXTENSION := .cc LOCAL_CFLAGS += \ -DGTEST_OS_LINUX_ANDROID \ - -DGTEST_HAS_STD_STRING + -DGTEST_HAS_STD_STRING \ + -Werror LOCAL_SRC_FILES := zip_archive_test.cc LOCAL_SHARED_LIBRARIES := liblog LOCAL_STATIC_LIBRARIES := libziparchive libz libgtest libgtest_main libutils @@ -58,7 +62,8 @@ LOCAL_MODULE := ziparchive-tests-host LOCAL_CPP_EXTENSION := .cc LOCAL_CFLAGS += \ -DGTEST_OS_LINUX \ - -DGTEST_HAS_STD_STRING + -DGTEST_HAS_STD_STRING \ + -Werror LOCAL_SRC_FILES := zip_archive_test.cc LOCAL_STATIC_LIBRARIES := libziparchive-host \ libz \ diff --git a/libziparchive/zip_archive.cc b/libziparchive/zip_archive.cc index a23d4ae..128bad4 100644 --- a/libziparchive/zip_archive.cc +++ b/libziparchive/zip_archive.cc @@ -17,73 +17,191 @@ /* * Read-only access to Zip archives, with minimal heap allocation. */ -#include "ziparchive/zip_archive.h" - -#include <zlib.h> #include <assert.h> #include <errno.h> +#include <fcntl.h> +#include <inttypes.h> #include <limits.h> #include <log/log.h> -#include <fcntl.h> #include <stdlib.h> #include <string.h> #include <unistd.h> +#include <utils/Compat.h> #include <utils/FileMap.h> +#include <zlib.h> #include <JNIHelp.h> // TEMP_FAILURE_RETRY may or may not be in unistd -// This is for windows. If we don't open a file in binary mode, weirds +#include "ziparchive/zip_archive.h" + +// This is for windows. If we don't open a file in binary mode, weird // things will happen. #ifndef O_BINARY #define O_BINARY 0 #endif -/* - * Zip file constants. - */ -static const uint32_t kEOCDSignature = 0x06054b50; -static const uint32_t kEOCDLen = 2; -static const uint32_t kEOCDNumEntries = 8; // offset to #of entries in file -static const uint32_t kEOCDSize = 12; // size of the central directory -static const uint32_t kEOCDFileOffset = 16; // offset to central directory - -static const uint32_t kMaxCommentLen = 65535; // longest possible in ushort -static const uint32_t kMaxEOCDSearch = (kMaxCommentLen + kEOCDLen); - -static const uint32_t kLFHSignature = 0x04034b50; -static const uint32_t kLFHLen = 30; // excluding variable-len fields -static const uint32_t kLFHGPBFlags = 6; // general purpose bit flags -static const uint32_t kLFHCRC = 14; // offset to CRC -static const uint32_t kLFHCompLen = 18; // offset to compressed length -static const uint32_t kLFHUncompLen = 22; // offset to uncompressed length -static const uint32_t kLFHNameLen = 26; // offset to filename length -static const uint32_t kLFHExtraLen = 28; // offset to extra length - -static const uint32_t kCDESignature = 0x02014b50; -static const uint32_t kCDELen = 46; // excluding variable-len fields -static const uint32_t kCDEMethod = 10; // offset to compression method -static const uint32_t kCDEModWhen = 12; // offset to modification timestamp -static const uint32_t kCDECRC = 16; // offset to entry CRC -static const uint32_t kCDECompLen = 20; // offset to compressed length -static const uint32_t kCDEUncompLen = 24; // offset to uncompressed length -static const uint32_t kCDENameLen = 28; // offset to filename length -static const uint32_t kCDEExtraLen = 30; // offset to extra length -static const uint32_t kCDECommentLen = 32; // offset to comment length -static const uint32_t kCDELocalOffset = 42; // offset to local hdr - -static const uint32_t kDDOptSignature = 0x08074b50; // *OPTIONAL* data descriptor signature -static const uint32_t kDDSignatureLen = 4; -static const uint32_t kDDLen = 12; -static const uint32_t kDDMaxLen = 16; // max of 16 bytes with a signature, 12 bytes without -static const uint32_t kDDCrc32 = 0; // offset to crc32 -static const uint32_t kDDCompLen = 4; // offset to compressed length -static const uint32_t kDDUncompLen = 8; // offset to uncompressed length - -static const uint32_t kGPBDDFlagMask = 0x0008; // mask value that signifies that the entry has a DD - +#define DISALLOW_IMPLICIT_CONSTRUCTORS(TypeName) \ + TypeName(); \ + TypeName(const TypeName&); \ + void operator=(const TypeName&) + +// The "end of central directory" (EOCD) record. Each archive +// contains exactly once such record which appears at the end of +// the archive. It contains archive wide information like the +// number of entries in the archive and the offset to the central +// directory of the offset. +struct EocdRecord { + static const uint32_t kSignature = 0x06054b50; + + // End of central directory signature, should always be + // |kSignature|. + uint32_t eocd_signature; + // The number of the current "disk", i.e, the "disk" that this + // central directory is on. + // + // This implementation assumes that each archive spans a single + // disk only. i.e, that disk_num == 1. + uint16_t disk_num; + // The disk where the central directory starts. + // + // This implementation assumes that each archive spans a single + // disk only. i.e, that cd_start_disk == 1. + uint16_t cd_start_disk; + // The number of central directory records on this disk. + // + // This implementation assumes that each archive spans a single + // disk only. i.e, that num_records_on_disk == num_records. + uint16_t num_records_on_disk; + // The total number of central directory records. + uint16_t num_records; + // The size of the central directory (in bytes). + uint32_t cd_size; + // The offset of the start of the central directory, relative + // to the start of the file. + uint32_t cd_start_offset; + // Length of the central directory comment. + uint16_t comment_length; + private: + DISALLOW_IMPLICIT_CONSTRUCTORS(EocdRecord); +} __attribute__((packed)); + +// A structure representing the fixed length fields for a single +// record in the central directory of the archive. In addition to +// the fixed length fields listed here, each central directory +// record contains a variable length "file_name" and "extra_field" +// whose lengths are given by |file_name_length| and |extra_field_length| +// respectively. +struct CentralDirectoryRecord { + static const uint32_t kSignature = 0x02014b50; + + // The start of record signature. Must be |kSignature|. + uint32_t record_signature; + // Tool version. Ignored by this implementation. + uint16_t version_made_by; + // Tool version. Ignored by this implementation. + uint16_t version_needed; + // The "general purpose bit flags" for this entry. The only + // flag value that we currently check for is the "data descriptor" + // flag. + uint16_t gpb_flags; + // The compression method for this entry, one of |kCompressStored| + // and |kCompressDeflated|. + uint16_t compression_method; + // The file modification time and date for this entry. + uint16_t last_mod_time; + uint16_t last_mod_date; + // The CRC-32 checksum for this entry. + uint32_t crc32; + // The compressed size (in bytes) of this entry. + uint32_t compressed_size; + // The uncompressed size (in bytes) of this entry. + uint32_t uncompressed_size; + // The length of the entry file name in bytes. The file name + // will appear immediately after this record. + uint16_t file_name_length; + // The length of the extra field info (in bytes). This data + // will appear immediately after the entry file name. + uint16_t extra_field_length; + // The length of the entry comment (in bytes). This data will + // appear immediately after the extra field. + uint16_t comment_length; + // The start disk for this entry. Ignored by this implementation). + uint16_t file_start_disk; + // File attributes. Ignored by this implementation. + uint16_t internal_file_attributes; + // File attributes. Ignored by this implementation. + uint32_t external_file_attributes; + // The offset to the local file header for this entry, from the + // beginning of this archive. + uint32_t local_file_header_offset; + private: + DISALLOW_IMPLICIT_CONSTRUCTORS(CentralDirectoryRecord); +} __attribute__((packed)); + +// The local file header for a given entry. This duplicates information +// present in the central directory of the archive. It is an error for +// the information here to be different from the central directory +// information for a given entry. +struct LocalFileHeader { + static const uint32_t kSignature = 0x04034b50; + + // The local file header signature, must be |kSignature|. + uint32_t lfh_signature; + // Tool version. Ignored by this implementation. + uint16_t version_needed; + // The "general purpose bit flags" for this entry. The only + // flag value that we currently check for is the "data descriptor" + // flag. + uint16_t gpb_flags; + // The compression method for this entry, one of |kCompressStored| + // and |kCompressDeflated|. + uint16_t compression_method; + // The file modification time and date for this entry. + uint16_t last_mod_time; + uint16_t last_mod_date; + // The CRC-32 checksum for this entry. + uint32_t crc32; + // The compressed size (in bytes) of this entry. + uint32_t compressed_size; + // The uncompressed size (in bytes) of this entry. + uint32_t uncompressed_size; + // The length of the entry file name in bytes. The file name + // will appear immediately after this record. + uint16_t file_name_length; + // The length of the extra field info (in bytes). This data + // will appear immediately after the entry file name. + uint16_t extra_field_length; + private: + DISALLOW_IMPLICIT_CONSTRUCTORS(LocalFileHeader); +} __attribute__((packed)); + +struct DataDescriptor { + // The *optional* data descriptor start signature. + static const uint32_t kOptSignature = 0x08074b50; + + // CRC-32 checksum of the entry. + uint32_t crc32; + // Compressed size of the entry. + uint32_t compressed_size; + // Uncompressed size of the entry. + uint32_t uncompressed_size; + private: + DISALLOW_IMPLICIT_CONSTRUCTORS(DataDescriptor); +} __attribute__((packed)); + +#undef DISALLOW_IMPLICIT_CONSTRUCTORS + +static const uint32_t kGPBDDFlagMask = 0x0008; // mask value that signifies that the entry has a DD static const uint32_t kMaxErrorLen = 1024; +// The maximum size of a central directory or a file +// comment in bytes. +static const uint32_t kMaxCommentLen = 65535; + +// The maximum number of bytes to scan backwards for the EOCD start. +static const uint32_t kMaxEOCDSearch = kMaxCommentLen + sizeof(EocdRecord); + static const char* kErrorMessages[] = { "Unknown return code.", "Iteration ended", @@ -217,8 +335,7 @@ static int32_t CopyFileToFile(int fd, uint8_t* begin, const uint32_t length, uin ssize_t actual = TEMP_FAILURE_RETRY(read(fd, buf, get_size)); if (actual != get_size) { - ALOGW("CopyFileToFile: copy read failed (%d vs %zd)", - (int) actual, get_size); + ALOGW("CopyFileToFile: copy read failed (" ZD " vs " ZD ")", actual, get_size); return kIoError; } @@ -279,7 +396,7 @@ static int64_t EntryToIndex(const ZipEntryName* hash_table, ent = (ent + 1) & (hash_table_size - 1); } - ALOGV("Zip: Unable to find entry %.*s", name_length, name); + ALOGV("Zip: Unable to find entry %.*s", length, name); return kEntryNotFound; } @@ -310,39 +427,21 @@ static int32_t AddToHash(ZipEntryName *hash_table, const uint64_t hash_table_siz return 0; } -/* - * Get 2 little-endian bytes. - */ -static uint16_t get2LE(const uint8_t* src) { - return src[0] | (src[1] << 8); -} - -/* - * Get 4 little-endian bytes. - */ -static uint32_t get4LE(const uint8_t* src) { - uint32_t result; - - result = src[0]; - result |= src[1] << 8; - result |= src[2] << 16; - result |= src[3] << 24; - - return result; -} - static int32_t MapCentralDirectory0(int fd, const char* debug_file_name, ZipArchive* archive, off64_t file_length, - uint32_t read_amount, uint8_t* scan_buffer) { + off64_t read_amount, uint8_t* scan_buffer) { const off64_t search_start = file_length - read_amount; if (lseek64(fd, search_start, SEEK_SET) != search_start) { - ALOGW("Zip: seek %lld failed: %s", search_start, strerror(errno)); + ALOGW("Zip: seek %" PRId64 " failed: %s", static_cast<int64_t>(search_start), + strerror(errno)); return kIoError; } - ssize_t actual = TEMP_FAILURE_RETRY(read(fd, scan_buffer, read_amount)); - if (actual != (ssize_t) read_amount) { - ALOGW("Zip: read %zd failed: %s", read_amount, strerror(errno)); + ssize_t actual = TEMP_FAILURE_RETRY( + read(fd, scan_buffer, static_cast<size_t>(read_amount))); + if (actual != static_cast<ssize_t>(read_amount)) { + ALOGW("Zip: read %" PRId64 " failed: %s", static_cast<int64_t>(read_amount), + strerror(errno)); return kIoError; } @@ -352,9 +451,10 @@ static int32_t MapCentralDirectory0(int fd, const char* debug_file_name, * doing an initial minimal read; if we don't find it, retry with a * second read as above.) */ - int i; - for (i = read_amount - kEOCDLen; i >= 0; i--) { - if (scan_buffer[i] == 0x50 && get4LE(&scan_buffer[i]) == kEOCDSignature) { + int i = read_amount - sizeof(EocdRecord); + for (; i >= 0; i--) { + if (scan_buffer[i] == 0x50 && + ((*reinterpret_cast<uint32_t*>(&scan_buffer[i])) == EocdRecord::kSignature)) { ALOGV("+++ Found EOCD at buf+%d", i); break; } @@ -365,46 +465,52 @@ static int32_t MapCentralDirectory0(int fd, const char* debug_file_name, } const off64_t eocd_offset = search_start + i; - const uint8_t* eocd_ptr = scan_buffer + i; - - assert(eocd_offset < file_length); + const EocdRecord* eocd = reinterpret_cast<const EocdRecord*>(scan_buffer + i); + /* + * Verify that there's no trailing space at the end of the central directory + * and its comment. + */ + const off64_t calculated_length = eocd_offset + sizeof(EocdRecord) + + eocd->comment_length; + if (calculated_length != file_length) { + ALOGW("Zip: %" PRId64 " extraneous bytes at the end of the central directory", + static_cast<int64_t>(file_length - calculated_length)); + return kInvalidFile; + } /* * Grab the CD offset and size, and the number of entries in the - * archive. Verify that they look reasonable. Widen dir_size and - * dir_offset to the file offset type. + * archive and verify that they look reasonable. */ - const uint16_t num_entries = get2LE(eocd_ptr + kEOCDNumEntries); - const off64_t dir_size = get4LE(eocd_ptr + kEOCDSize); - const off64_t dir_offset = get4LE(eocd_ptr + kEOCDFileOffset); - - if (dir_offset + dir_size > eocd_offset) { - ALOGW("Zip: bad offsets (dir %lld, size %lld, eocd %lld)", - dir_offset, dir_size, eocd_offset); + if (eocd->cd_start_offset + eocd->cd_size > eocd_offset) { + ALOGW("Zip: bad offsets (dir %" PRIu32 ", size %" PRIu32 ", eocd %" PRId64 ")", + eocd->cd_start_offset, eocd->cd_size, static_cast<int64_t>(eocd_offset)); return kInvalidOffset; } - if (num_entries == 0) { + if (eocd->num_records == 0) { ALOGW("Zip: empty archive?"); return kEmptyArchive; } - ALOGV("+++ num_entries=%d dir_size=%d dir_offset=%d", num_entries, dir_size, - dir_offset); + ALOGV("+++ num_entries=%" PRIu32 "dir_size=%" PRIu32 " dir_offset=%" PRIu32, + eocd->num_records, eocd->cd_size, eocd->cd_start_offset); /* * It all looks good. Create a mapping for the CD, and set the fields * in archive. */ - android::FileMap* map = MapFileSegment(fd, dir_offset, dir_size, - true /* read only */, debug_file_name); + android::FileMap* map = MapFileSegment(fd, + static_cast<off64_t>(eocd->cd_start_offset), + static_cast<size_t>(eocd->cd_size), + true /* read only */, debug_file_name); if (map == NULL) { archive->directory_map = NULL; return kMmapFailed; } archive->directory_map = map; - archive->num_entries = num_entries; - archive->directory_offset = dir_offset; + archive->num_entries = eocd->num_records; + archive->directory_offset = eocd->cd_start_offset; return 0; } @@ -430,12 +536,12 @@ static int32_t MapCentralDirectory(int fd, const char* debug_file_name, } if (file_length > (off64_t) 0xffffffff) { - ALOGV("Zip: zip file too long %d", file_length); + ALOGV("Zip: zip file too long %" PRId64, static_cast<int64_t>(file_length)); return kInvalidFile; } - if (file_length < (int64_t) kEOCDLen) { - ALOGV("Zip: length %ld is too small to be zip", file_length); + if (file_length < static_cast<off64_t>(sizeof(EocdRecord))) { + ALOGV("Zip: length %" PRId64 " is too small to be zip", static_cast<int64_t>(file_length)); return kInvalidFile; } @@ -451,12 +557,12 @@ static int32_t MapCentralDirectory(int fd, const char* debug_file_name, * * We start by pulling in the last part of the file. */ - uint32_t read_amount = kMaxEOCDSearch; - if (file_length < (off64_t) read_amount) { + off64_t read_amount = kMaxEOCDSearch; + if (file_length < read_amount) { read_amount = file_length; } - uint8_t* scan_buffer = (uint8_t*) malloc(read_amount); + uint8_t* scan_buffer = reinterpret_cast<uint8_t*>(malloc(read_amount)); int32_t result = MapCentralDirectory0(fd, debug_file_name, archive, file_length, read_amount, scan_buffer); @@ -472,9 +578,9 @@ static int32_t MapCentralDirectory(int fd, const char* debug_file_name, */ static int32_t ParseZipArchive(ZipArchive* archive) { int32_t result = -1; - const uint8_t* cd_ptr = (const uint8_t*) archive->directory_map->getDataPtr(); - size_t cd_length = archive->directory_map->getDataLength(); - uint16_t num_entries = archive->num_entries; + const uint8_t* const cd_ptr = (const uint8_t*) archive->directory_map->getDataPtr(); + const size_t cd_length = archive->directory_map->getDataLength(); + const uint16_t num_entries = archive->num_entries; /* * Create hash table. We have a minimum 75% load factor, possibly as @@ -489,45 +595,49 @@ static int32_t ParseZipArchive(ZipArchive* archive) { * Walk through the central directory, adding entries to the hash * table and verifying values. */ + 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 (get4LE(ptr) != kCDESignature) { - ALOGW("Zip: missed a central dir sig (at %d)", i); + const CentralDirectoryRecord* cdr = + reinterpret_cast<const CentralDirectoryRecord*>(ptr); + if (cdr->record_signature != CentralDirectoryRecord::kSignature) { + ALOGW("Zip: missed a central dir sig (at %" PRIu16 ")", i); goto bail; } - if (ptr + kCDELen > cd_ptr + cd_length) { - ALOGW("Zip: ran off the end (at %d)", i); + if (ptr + sizeof(CentralDirectoryRecord) > cd_end) { + ALOGW("Zip: ran off the end (at %" PRIu16 ")", i); goto bail; } - const off64_t local_header_offset = get4LE(ptr + kCDELocalOffset); + const off64_t local_header_offset = cdr->local_file_header_offset; if (local_header_offset >= archive->directory_offset) { - ALOGW("Zip: bad LFH offset %lld at entry %d", local_header_offset, i); + ALOGW("Zip: bad LFH offset %" PRId64 " at entry %" PRIu16, (int64_t)local_header_offset, i); goto bail; } - const uint16_t file_name_length = get2LE(ptr + kCDENameLen); - const uint16_t extra_length = get2LE(ptr + kCDEExtraLen); - const uint16_t comment_length = get2LE(ptr + kCDECommentLen); + const uint16_t file_name_length = cdr->file_name_length; + const uint16_t extra_length = cdr->extra_field_length; + const uint16_t comment_length = cdr->comment_length; /* add the CDE filename to the hash table */ + const char* file_name = reinterpret_cast<const char *>(ptr + sizeof(CentralDirectoryRecord)); const int add_result = AddToHash(archive->hash_table, - archive->hash_table_size, (const char*) ptr + kCDELen, file_name_length); + archive->hash_table_size, file_name, file_name_length); if (add_result) { ALOGW("Zip: Error adding entry to hash table %d", add_result); result = add_result; goto bail; } - ptr += kCDELen + file_name_length + extra_length + comment_length; - if ((size_t)(ptr - cd_ptr) > cd_length) { - ALOGW("Zip: bad CD advance (%d vs %zd) at entry %d", - (int) (ptr - cd_ptr), cd_length, i); + ptr += sizeof(CentralDirectoryRecord) + file_name_length + extra_length + comment_length; + if ((ptr - cd_ptr) > static_cast<int64_t>(cd_length)) { + ALOGW("Zip: bad CD advance (%tu vs %zu) at entry %" PRIu16, + ptr - cd_ptr, cd_length, i); goto bail; } } - ALOGV("+++ zip good scan %d entries", num_entries); + ALOGV("+++ zip good scan %" PRIu16 " entries", num_entries); result = 0; @@ -591,32 +701,24 @@ void CloseArchive(ZipArchiveHandle handle) { archive->directory_map->release(); } free(archive->hash_table); - - /* ensure nobody tries to use the ZipArchive after it's closed */ - archive->directory_offset = -1; - archive->fd = -1; - archive->num_entries = -1; - archive->hash_table_size = -1; - archive->hash_table = NULL; + free(archive); } static int32_t UpdateEntryFromDataDescriptor(int fd, ZipEntry *entry) { - uint8_t ddBuf[kDDMaxLen]; + uint8_t ddBuf[sizeof(DataDescriptor) + sizeof(DataDescriptor::kOptSignature)]; ssize_t actual = TEMP_FAILURE_RETRY(read(fd, ddBuf, sizeof(ddBuf))); if (actual != sizeof(ddBuf)) { return kIoError; } - const uint32_t ddSignature = get4LE(ddBuf); - uint16_t ddOffset = 0; - if (ddSignature == kDDOptSignature) { - ddOffset = 4; - } + const uint32_t ddSignature = *(reinterpret_cast<const uint32_t*>(ddBuf)); + const uint16_t offset = (ddSignature == DataDescriptor::kOptSignature) ? 4 : 0; + const DataDescriptor* descriptor = reinterpret_cast<const DataDescriptor*>(ddBuf + offset); - entry->crc32 = get4LE(ddBuf + ddOffset + kDDCrc32); - entry->compressed_length = get4LE(ddBuf + ddOffset + kDDCompLen); - entry->uncompressed_length = get4LE(ddBuf + ddOffset + kDDUncompLen); + entry->crc32 = descriptor->crc32; + entry->compressed_length = descriptor->compressed_size; + entry->uncompressed_length = descriptor->uncompressed_size; return 0; } @@ -636,7 +738,7 @@ static inline ssize_t ReadAtOffset(int fd, uint8_t* buf, size_t len, // is Windows. Only recent versions of windows support unix like forks, // and even there the semantics are quite different. if (lseek64(fd, off, SEEK_SET) != off) { - ALOGW("Zip: failed seek to offset %lld", off); + ALOGW("Zip: failed seek to offset %" PRId64, off); return kIoError; } @@ -652,19 +754,22 @@ static int32_t FindEntry(const ZipArchive* archive, const int ent, // Recover the start of the central directory entry from the filename // pointer. The filename is the first entry past the fixed-size data, // so we can just subtract back from that. - const unsigned char* ptr = (const unsigned char*) name; - ptr -= kCDELen; + const uint8_t* ptr = reinterpret_cast<const uint8_t*>(name); + ptr -= sizeof(CentralDirectoryRecord); // This is the base of our mmapped region, we have to sanity check that // the name that's in the hash table is a pointer to a location within // this mapped region. - const unsigned char* base_ptr = (const unsigned char*) - archive->directory_map->getDataPtr(); + const uint8_t* base_ptr = reinterpret_cast<const uint8_t*>( + archive->directory_map->getDataPtr()); if (ptr < base_ptr || ptr > base_ptr + archive->directory_map->getDataLength()) { ALOGW("Zip: Invalid entry pointer"); return kInvalidOffset; } + const CentralDirectoryRecord *cdr = + reinterpret_cast<const CentralDirectoryRecord*>(ptr); + // The offset of the start of the central directory in the zipfile. // We keep this lying around so that we can sanity check all our lengths // and our per-file structures. @@ -673,52 +778,48 @@ static int32_t FindEntry(const ZipArchive* archive, const int ent, // Fill out the compression method, modification time, crc32 // and other interesting attributes from the central directory. These // will later be compared against values from the local file header. - data->method = get2LE(ptr + kCDEMethod); - data->mod_time = get4LE(ptr + kCDEModWhen); - data->crc32 = get4LE(ptr + kCDECRC); - data->compressed_length = get4LE(ptr + kCDECompLen); - data->uncompressed_length = get4LE(ptr + kCDEUncompLen); + data->method = cdr->compression_method; + data->mod_time = cdr->last_mod_time; + data->crc32 = cdr->crc32; + data->compressed_length = cdr->compressed_size; + data->uncompressed_length = cdr->uncompressed_size; // Figure out the local header offset from the central directory. The // actual file data will begin after the local header and the name / // extra comments. - const off64_t local_header_offset = get4LE(ptr + kCDELocalOffset); - if (local_header_offset + (off64_t) kLFHLen >= cd_offset) { + const off64_t local_header_offset = cdr->local_file_header_offset; + if (local_header_offset + static_cast<off64_t>(sizeof(LocalFileHeader)) >= cd_offset) { ALOGW("Zip: bad local hdr offset in zip"); return kInvalidOffset; } - uint8_t lfh_buf[kLFHLen]; + uint8_t lfh_buf[sizeof(LocalFileHeader)]; ssize_t actual = ReadAtOffset(archive->fd, lfh_buf, sizeof(lfh_buf), local_header_offset); if (actual != sizeof(lfh_buf)) { - ALOGW("Zip: failed reading lfh name from offset %lld", local_header_offset); + ALOGW("Zip: failed reading lfh name from offset %" PRId64, (int64_t)local_header_offset); return kIoError; } - if (get4LE(lfh_buf) != kLFHSignature) { - ALOGW("Zip: didn't find signature at start of lfh, offset=%lld", - local_header_offset); + const LocalFileHeader *lfh = reinterpret_cast<const LocalFileHeader*>(lfh_buf); + + if (lfh->lfh_signature != LocalFileHeader::kSignature) { + ALOGW("Zip: didn't find signature at start of lfh, offset=%" PRId64, + static_cast<int64_t>(local_header_offset)); return kInvalidOffset; } // Paranoia: Match the values specified in the local file header // to those specified in the central directory. - const uint16_t lfhGpbFlags = get2LE(lfh_buf + kLFHGPBFlags); - const uint16_t lfhNameLen = get2LE(lfh_buf + kLFHNameLen); - const uint16_t lfhExtraLen = get2LE(lfh_buf + kLFHExtraLen); - - if ((lfhGpbFlags & kGPBDDFlagMask) == 0) { - const uint32_t lfhCrc = get4LE(lfh_buf + kLFHCRC); - const uint32_t lfhCompLen = get4LE(lfh_buf + kLFHCompLen); - const uint32_t lfhUncompLen = get4LE(lfh_buf + kLFHUncompLen); - + if ((lfh->gpb_flags & kGPBDDFlagMask) == 0) { data->has_data_descriptor = 0; - if (data->compressed_length != lfhCompLen || data->uncompressed_length != lfhUncompLen - || data->crc32 != lfhCrc) { - ALOGW("Zip: size/crc32 mismatch. expected {%d, %d, %x}, was {%d, %d, %x}", + if (data->compressed_length != lfh->compressed_size + || data->uncompressed_length != lfh->uncompressed_size + || data->crc32 != lfh->crc32) { + ALOGW("Zip: size/crc32 mismatch. expected {%" PRIu32 ", %" PRIu32 + ", %" PRIx32 "}, was {%" PRIu32 ", %" PRIu32 ", %" PRIx32 "}", data->compressed_length, data->uncompressed_length, data->crc32, - lfhCompLen, lfhUncompLen, lfhCrc); + lfh->compressed_size, lfh->uncompressed_size, lfh->crc32); return kInconsistentInformation; } } else { @@ -727,9 +828,9 @@ static int32_t FindEntry(const ZipArchive* archive, const int ent, // Check that the local file header name matches the declared // name in the central directory. - if (lfhNameLen == nameLen) { - const off64_t name_offset = local_header_offset + kLFHLen; - if (name_offset + lfhNameLen >= cd_offset) { + if (lfh->file_name_length == nameLen) { + const off64_t name_offset = local_header_offset + sizeof(LocalFileHeader); + if (name_offset + lfh->file_name_length >= cd_offset) { ALOGW("Zip: Invalid declared length"); return kInvalidOffset; } @@ -739,7 +840,7 @@ static int32_t FindEntry(const ZipArchive* archive, const int ent, name_offset); if (actual != nameLen) { - ALOGW("Zip: failed reading lfh name from offset %lld", name_offset); + ALOGW("Zip: failed reading lfh name from offset %" PRId64, (int64_t)name_offset); free(name_buf); return kIoError; } @@ -755,22 +856,23 @@ static int32_t FindEntry(const ZipArchive* archive, const int ent, return kInconsistentInformation; } - const off64_t data_offset = local_header_offset + kLFHLen + lfhNameLen + lfhExtraLen; + const off64_t data_offset = local_header_offset + sizeof(LocalFileHeader) + + lfh->file_name_length + lfh->extra_field_length; if (data_offset > cd_offset) { - ALOGW("Zip: bad data offset %lld in zip", (off64_t) data_offset); + ALOGW("Zip: bad data offset %" PRId64 " in zip", (int64_t)data_offset); return kInvalidOffset; } if ((off64_t)(data_offset + data->compressed_length) > cd_offset) { - ALOGW("Zip: bad compressed length in zip (%lld + %zd > %lld)", - data_offset, data->compressed_length, cd_offset); + ALOGW("Zip: bad compressed length in zip (%" PRId64 " + %" PRIu32 " > %" PRId64 ")", + (int64_t)data_offset, data->compressed_length, (int64_t)cd_offset); return kInvalidOffset; } if (data->method == kCompressStored && (off64_t)(data_offset + data->uncompressed_length) > cd_offset) { - ALOGW("Zip: bad uncompressed length in zip (%lld + %zd > %lld)", - data_offset, data->uncompressed_length, cd_offset); + ALOGW("Zip: bad uncompressed length in zip (%" PRId64 " + %" PRIu32 " > %" PRId64 ")", + (int64_t)data_offset, data->uncompressed_length, (int64_t)cd_offset); return kInvalidOffset; } @@ -906,10 +1008,10 @@ static int32_t InflateToFile(int fd, const ZipEntry* entry, do { /* read as much as we can */ if (zstream.avail_in == 0) { - const ssize_t getSize = (compressed_length > kBufSize) ? kBufSize : compressed_length; - const ssize_t actual = TEMP_FAILURE_RETRY(read(fd, read_buf, getSize)); + const ZD_TYPE getSize = (compressed_length > kBufSize) ? kBufSize : compressed_length; + const ZD_TYPE actual = TEMP_FAILURE_RETRY(read(fd, read_buf, getSize)); if (actual != getSize) { - ALOGW("Zip: inflate read failed (%d vs %zd)", actual, getSize); + ALOGW("Zip: inflate read failed (" ZD " vs " ZD ")", actual, getSize); result = kIoError; goto z_bail; } @@ -952,7 +1054,7 @@ static int32_t InflateToFile(int fd, const ZipEntry* entry, *crc_out = zstream.adler; if (zstream.total_out != uncompressed_length || compressed_length != 0) { - ALOGW("Zip: size mismatch on inflated file (%ld vs %zd)", + ALOGW("Zip: size mismatch on inflated file (%lu vs %" PRIu32 ")", zstream.total_out, uncompressed_length); result = kInconsistentInformation; goto z_bail; @@ -973,7 +1075,7 @@ int32_t ExtractToMemory(ZipArchiveHandle handle, off64_t data_offset = entry->offset; if (lseek64(archive->fd, data_offset, SEEK_SET) != data_offset) { - ALOGW("Zip: lseek to data at %lld failed", (off64_t) data_offset); + ALOGW("Zip: lseek to data at %" PRId64 " failed", (int64_t)data_offset); return kIoError; } @@ -996,7 +1098,7 @@ int32_t ExtractToMemory(ZipArchiveHandle handle, // TODO: Fix this check by passing the right flags to inflate2 so that // it calculates the CRC for us. if (entry->crc32 != crc && false) { - ALOGW("Zip: crc mismatch: expected %u, was %llu", entry->crc32, crc); + ALOGW("Zip: crc mismatch: expected %" PRIu32 ", was %" PRIu64, entry->crc32, crc); return kInconsistentInformation; } @@ -1016,8 +1118,8 @@ int32_t ExtractEntryToFile(ZipArchiveHandle handle, int result = TEMP_FAILURE_RETRY(ftruncate(fd, declared_length + current_offset)); if (result == -1) { - ALOGW("Zip: unable to truncate file to %lld: %s", declared_length + current_offset, - strerror(errno)); + ALOGW("Zip: unable to truncate file to %" PRId64 ": %s", + (int64_t)(declared_length + current_offset), strerror(errno)); return kIoError; } diff --git a/libziparchive/zip_archive_test.cc b/libziparchive/zip_archive_test.cc index 3082216..875b6de 100644 --- a/libziparchive/zip_archive_test.cc +++ b/libziparchive/zip_archive_test.cc @@ -140,11 +140,7 @@ TEST(ziparchive, ExtractToMemory) { CloseArchive(handle); } -TEST(ziparchive, EmptyEntries) { - char temp_file_pattern[] = "empty_entries_test_XXXXXX"; - int fd = mkstemp(temp_file_pattern); - ASSERT_NE(-1, fd); - const uint32_t data[] = { +static const uint32_t kEmptyEntriesZip[] = { 0x04034b50, 0x0000000a, 0x63600000, 0x00004438, 0x00000000, 0x00000000, 0x00090000, 0x6d65001c, 0x2e797470, 0x55747874, 0x03000954, 0x52e25c13, 0x52e25c24, 0x000b7875, 0x42890401, 0x88040000, 0x50000013, 0x1e02014b, @@ -152,8 +148,28 @@ TEST(ziparchive, EmptyEntries) { 0x00001800, 0x00000000, 0xa0000000, 0x00000081, 0x706d6500, 0x742e7974, 0x54557478, 0x13030005, 0x7552e25c, 0x01000b78, 0x00428904, 0x13880400, 0x4b500000, 0x00000605, 0x00010000, 0x004f0001, 0x00430000, 0x00000000 }; - const ssize_t file_size = 168; - ASSERT_EQ(file_size, TEMP_FAILURE_RETRY(write(fd, data, file_size))); + +static int make_temporary_file(const char* file_name_pattern) { + char full_path[1024]; + // Account for differences between the host and the target. + // + // TODO: Maybe reuse bionic/tests/TemporaryFile.h. + snprintf(full_path, sizeof(full_path), "/data/local/tmp/%s", file_name_pattern); + int fd = mkstemp(full_path); + if (fd == -1) { + snprintf(full_path, sizeof(full_path), "/tmp/%s", file_name_pattern); + fd = mkstemp(full_path); + } + + return fd; +} + +TEST(ziparchive, EmptyEntries) { + char temp_file_pattern[] = "empty_entries_test_XXXXXX"; + int fd = make_temporary_file(temp_file_pattern); + ASSERT_NE(-1, fd); + const ssize_t file_size = sizeof(kEmptyEntriesZip); + ASSERT_EQ(file_size, TEMP_FAILURE_RETRY(write(fd, kEmptyEntriesZip, file_size))); ZipArchiveHandle handle; ASSERT_EQ(0, OpenArchiveFd(fd, "EmptyEntriesTest", &handle)); @@ -165,7 +181,7 @@ TEST(ziparchive, EmptyEntries) { ASSERT_EQ(0, ExtractToMemory(handle, &entry, buffer, 1)); char output_file_pattern[] = "empty_entries_output_XXXXXX"; - int output_fd = mkstemp(output_file_pattern); + int output_fd = make_temporary_file(output_file_pattern); ASSERT_NE(-1, output_fd); ASSERT_EQ(0, ExtractEntryToFile(handle, &entry, output_fd)); @@ -177,9 +193,25 @@ TEST(ziparchive, EmptyEntries) { close(output_fd); } +TEST(ziparchive, TrailerAfterEOCD) { + char temp_file_pattern[] = "trailer_after_eocd_test_XXXXXX"; + int fd = make_temporary_file(temp_file_pattern); + ASSERT_NE(-1, fd); + + // Create a file with 8 bytes of random garbage. + static const uint8_t trailer[] = { 'A' ,'n', 'd', 'r', 'o', 'i', 'd', 'z' }; + const ssize_t file_size = sizeof(kEmptyEntriesZip); + const ssize_t trailer_size = sizeof(trailer); + ASSERT_EQ(file_size, TEMP_FAILURE_RETRY(write(fd, kEmptyEntriesZip, file_size))); + ASSERT_EQ(trailer_size, TEMP_FAILURE_RETRY(write(fd, trailer, trailer_size))); + + ZipArchiveHandle handle; + ASSERT_GT(0, OpenArchiveFd(fd, "EmptyEntriesTest", &handle)); +} + TEST(ziparchive, ExtractToFile) { char kTempFilePattern[] = "zip_archive_input_XXXXXX"; - int fd = mkstemp(kTempFilePattern); + int fd = make_temporary_file(kTempFilePattern); ASSERT_NE(-1, fd); const uint8_t data[8] = { '1', '2', '3', '4', '5', '6', '7', '8' }; const ssize_t data_size = sizeof(data); @@ -209,7 +241,8 @@ TEST(ziparchive, ExtractToFile) { sizeof(kATxtContents))); // Assert that the total length of the file is sane - ASSERT_EQ(data_size + sizeof(kATxtContents), lseek64(fd, 0, SEEK_END)); + ASSERT_EQ(data_size + static_cast<ssize_t>(sizeof(kATxtContents)), + lseek64(fd, 0, SEEK_END)); close(fd); } @@ -247,4 +280,3 @@ int main(int argc, char** argv) { return RUN_ALL_TESTS(); } - |