diff --git a/libziparchive/include/ziparchive/zip_archive.h b/libziparchive/include/ziparchive/zip_archive.h index 047af90f2..098a9cb5d 100644 --- a/libziparchive/include/ziparchive/zip_archive.h +++ b/libziparchive/include/ziparchive/zip_archive.h @@ -126,6 +126,9 @@ int32_t OpenArchive(const char* fileName, ZipArchiveHandle* handle); int32_t OpenArchiveFd(const int fd, const char* debugFileName, ZipArchiveHandle* handle, bool assume_ownership = true); +int32_t OpenArchiveFdRange(const int fd, const char* debugFileName, ZipArchiveHandle* handle, + off64_t length, off64_t offset, bool assume_ownership = true); + int32_t OpenArchiveFromMemory(const void* address, size_t length, const char* debugFileName, ZipArchiveHandle* handle); /* @@ -222,6 +225,12 @@ int32_t ExtractToMemory(ZipArchiveHandle archive, ZipEntry* entry, uint8_t* begi int GetFileDescriptor(const ZipArchiveHandle archive); +/** + * Returns the offset of the zip archive in the backing file descriptor, or 0 if the zip archive is + * not backed by a file descriptor. + */ +off64_t GetFileDescriptorOffset(const ZipArchiveHandle archive); + const char* ErrorCodeString(int32_t error_code); #if !defined(_WIN32) diff --git a/libziparchive/zip_archive.cc b/libziparchive/zip_archive.cc index 68837ccf6..958c34b61 100644 --- a/libziparchive/zip_archive.cc +++ b/libziparchive/zip_archive.cc @@ -162,8 +162,8 @@ uint64_t GetOwnerTag(const ZipArchive* archive) { } #endif -ZipArchive::ZipArchive(const int fd, bool assume_ownership) - : mapped_zip(fd), +ZipArchive::ZipArchive(MappedZipFile&& map, bool assume_ownership) + : mapped_zip(map), close_file(assume_ownership), directory_offset(0), central_directory(), @@ -173,7 +173,8 @@ ZipArchive::ZipArchive(const int fd, bool assume_ownership) hash_table(nullptr) { #if defined(__BIONIC__) if (assume_ownership) { - android_fdsan_exchange_owner_tag(fd, 0, GetOwnerTag(this)); + CHECK(mapped_zip.HasFd()); + android_fdsan_exchange_owner_tag(mapped_zip.GetFileDescriptor(), 0, GetOwnerTag(this)); } #endif } @@ -442,14 +443,32 @@ static int32_t OpenArchiveInternal(ZipArchive* archive, const char* debug_file_n int32_t OpenArchiveFd(int fd, const char* debug_file_name, ZipArchiveHandle* handle, bool assume_ownership) { - ZipArchive* archive = new ZipArchive(fd, assume_ownership); + ZipArchive* archive = new ZipArchive(MappedZipFile(fd), assume_ownership); *handle = archive; return OpenArchiveInternal(archive, debug_file_name); } +int32_t OpenArchiveFdRange(int fd, const char* debug_file_name, ZipArchiveHandle* handle, + off64_t length, off64_t offset, bool assume_ownership) { + ZipArchive* archive = new ZipArchive(MappedZipFile(fd, length, offset), assume_ownership); + *handle = archive; + + if (length < 0) { + ALOGW("Invalid zip length %" PRId64, length); + return kIoError; + } + + if (offset < 0) { + ALOGW("Invalid zip offset %" PRId64, offset); + return kIoError; + } + + return OpenArchiveInternal(archive, debug_file_name); +} + int32_t OpenArchive(const char* fileName, ZipArchiveHandle* handle) { const int fd = ::android::base::utf8::open(fileName, O_RDONLY | O_BINARY | O_CLOEXEC, 0); - ZipArchive* archive = new ZipArchive(fd, true); + ZipArchive* archive = new ZipArchive(MappedZipFile(fd), true); *handle = archive; if (fd < 0) { @@ -1126,6 +1145,10 @@ int GetFileDescriptor(const ZipArchiveHandle archive) { return archive->mapped_zip.GetFileDescriptor(); } +off64_t GetFileDescriptorOffset(const ZipArchiveHandle archive) { + return archive->mapped_zip.GetFileOffset(); +} + #if !defined(_WIN32) class ProcessWriter : public zip_archive::Writer { public: @@ -1165,31 +1188,65 @@ const void* MappedZipFile::GetBasePtr() const { return base_ptr_; } +off64_t MappedZipFile::GetFileOffset() const { + return fd_offset_; +} + off64_t MappedZipFile::GetFileLength() const { if (has_fd_) { - off64_t result = lseek64(fd_, 0, SEEK_END); - if (result == -1) { + if (data_length_ != -1) { + return data_length_; + } + data_length_ = lseek64(fd_, 0, SEEK_END); + if (data_length_ == -1) { ALOGE("Zip: lseek on fd %d failed: %s", fd_, strerror(errno)); } - return result; + return data_length_; } else { if (base_ptr_ == nullptr) { ALOGE("Zip: invalid file map"); return -1; } - return static_cast(data_length_); + return data_length_; } } // Attempts to read |len| bytes into |buf| at offset |off|. bool MappedZipFile::ReadAtOffset(uint8_t* buf, size_t len, off64_t off) const { if (has_fd_) { - if (!android::base::ReadFullyAtOffset(fd_, buf, len, off)) { + if (off < 0) { + ALOGE("Zip: invalid offset %" PRId64, off); + return false; + } + + off64_t read_offset; + if (__builtin_add_overflow(fd_offset_, off, &read_offset)) { + ALOGE("Zip: invalid read offset %" PRId64 " overflows, fd offset %" PRId64, off, fd_offset_); + return false; + } + + if (data_length_ != -1) { + off64_t read_end; + if (len > std::numeric_limits::max() || + __builtin_add_overflow(off, static_cast(len), &read_end)) { + ALOGE("Zip: invalid read length %" PRId64 " overflows, offset %" PRId64, + static_cast(len), off); + return false; + } + + if (read_end > data_length_) { + ALOGE("Zip: invalid read length %" PRId64 " exceeds data length %" PRId64 ", offset %" + PRId64, static_cast(len), data_length_, off); + return false; + } + } + + if (!android::base::ReadFullyAtOffset(fd_, buf, len, read_offset)) { ALOGE("Zip: failed to read at offset %" PRId64, off); return false; } } else { - if (off < 0 || off > static_cast(data_length_)) { + if (off < 0 || off > data_length_) { ALOGE("Zip: invalid offset: %" PRId64 ", data length: %" PRId64, off, data_length_); return false; } @@ -1207,7 +1264,8 @@ void CentralDirectory::Initialize(const void* map_base_ptr, off64_t cd_start_off bool ZipArchive::InitializeCentralDirectory(off64_t cd_start_offset, size_t cd_size) { if (mapped_zip.HasFd()) { directory_map = android::base::MappedFile::FromFd(mapped_zip.GetFileDescriptor(), - cd_start_offset, cd_size, PROT_READ); + mapped_zip.GetFileOffset() + cd_start_offset, + cd_size, PROT_READ); if (!directory_map) { ALOGE("Zip: failed to map central directory (offset %" PRId64 ", size %zu): %s", cd_start_offset, cd_size, strerror(errno)); diff --git a/libziparchive/zip_archive_private.h b/libziparchive/zip_archive_private.h index 1d05fc718..362503816 100644 --- a/libziparchive/zip_archive_private.h +++ b/libziparchive/zip_archive_private.h @@ -97,10 +97,14 @@ enum ErrorCodes : int32_t { class MappedZipFile { public: explicit MappedZipFile(const int fd) - : has_fd_(true), fd_(fd), base_ptr_(nullptr), data_length_(0) {} + : has_fd_(true), fd_(fd), fd_offset_(0), base_ptr_(nullptr), data_length_(-1) {} + + explicit MappedZipFile(const int fd, off64_t length, off64_t offset) + : has_fd_(true), fd_(fd), fd_offset_(offset), base_ptr_(nullptr), data_length_(length) {} explicit MappedZipFile(const void* address, size_t length) - : has_fd_(false), fd_(-1), base_ptr_(address), data_length_(static_cast(length)) {} + : has_fd_(false), fd_(-1), fd_offset_(0), base_ptr_(address), + data_length_(static_cast(length)) {} bool HasFd() const { return has_fd_; } @@ -108,6 +112,8 @@ class MappedZipFile { const void* GetBasePtr() const; + off64_t GetFileOffset() const; + off64_t GetFileLength() const; bool ReadAtOffset(uint8_t* buf, size_t len, off64_t off) const; @@ -120,9 +126,10 @@ class MappedZipFile { const bool has_fd_; const int fd_; + const off64_t fd_offset_; const void* const base_ptr_; - const off64_t data_length_; + mutable off64_t data_length_; }; class CentralDirectory { @@ -180,7 +187,7 @@ struct ZipArchive { uint32_t hash_table_size; ZipStringOffset* hash_table; - ZipArchive(const int fd, bool assume_ownership); + ZipArchive(MappedZipFile&& map, bool assume_ownership); ZipArchive(const void* address, size_t length); ~ZipArchive(); diff --git a/libziparchive/zip_archive_test.cc b/libziparchive/zip_archive_test.cc index 091630407..35fb3fe0f 100644 --- a/libziparchive/zip_archive_test.cc +++ b/libziparchive/zip_archive_test.cc @@ -108,6 +108,32 @@ TEST(ziparchive, OpenDoNotAssumeFdOwnership) { close(fd); } +TEST(ziparchive, OpenAssumeFdRangeOwnership) { + int fd = open((test_data_dir + "/" + kValidZip).c_str(), O_RDONLY | O_BINARY); + ASSERT_NE(-1, fd); + const off64_t length = lseek64(fd, 0, SEEK_END); + ASSERT_NE(-1, length); + ZipArchiveHandle handle; + ASSERT_EQ(0, OpenArchiveFdRange(fd, "OpenWithAssumeFdOwnership", &handle, + static_cast(length), 0)); + CloseArchive(handle); + ASSERT_EQ(-1, lseek(fd, 0, SEEK_SET)); + ASSERT_EQ(EBADF, errno); +} + +TEST(ziparchive, OpenDoNotAssumeFdRangeOwnership) { + int fd = open((test_data_dir + "/" + kValidZip).c_str(), O_RDONLY | O_BINARY); + ASSERT_NE(-1, fd); + const off64_t length = lseek(fd, 0, SEEK_END); + ASSERT_NE(-1, length); + ZipArchiveHandle handle; + ASSERT_EQ(0, OpenArchiveFdRange(fd, "OpenWithAssumeFdOwnership", &handle, + static_cast(length), 0, false)); + CloseArchive(handle); + ASSERT_EQ(0, lseek(fd, 0, SEEK_SET)); + close(fd); +} + TEST(ziparchive, Iteration_std_string_view) { ZipArchiveHandle handle; ASSERT_EQ(0, OpenArchiveWrapper(kValidZip, &handle)); @@ -254,6 +280,48 @@ TEST(ziparchive, TestInvalidDeclaredLength) { CloseArchive(handle); } +TEST(ziparchive, OpenArchiveFdRange) { + TemporaryFile tmp_file; + ASSERT_NE(-1, tmp_file.fd); + + const std::string leading_garbage(21, 'x'); + ASSERT_TRUE(android::base::WriteFully(tmp_file.fd, leading_garbage.c_str(), + leading_garbage.size())); + + std::string valid_content; + ASSERT_TRUE(android::base::ReadFileToString(test_data_dir + "/" + kValidZip, &valid_content)); + ASSERT_TRUE(android::base::WriteFully(tmp_file.fd, valid_content.c_str(), valid_content.size())); + + const std::string ending_garbage(42, 'x'); + ASSERT_TRUE(android::base::WriteFully(tmp_file.fd, ending_garbage.c_str(), + ending_garbage.size())); + + ZipArchiveHandle handle; + ASSERT_EQ(0, lseek(tmp_file.fd, 0, SEEK_SET)); + ASSERT_EQ(0, OpenArchiveFdRange(tmp_file.fd, "OpenArchiveFdRange", &handle, + valid_content.size(), + static_cast(leading_garbage.size()))); + + // An entry that's deflated. + ZipEntry data; + ASSERT_EQ(0, FindEntry(handle, "a.txt", &data)); + const uint32_t a_size = data.uncompressed_length; + ASSERT_EQ(a_size, kATxtContents.size()); + auto buffer = std::unique_ptr(new uint8_t[a_size]); + ASSERT_EQ(0, ExtractToMemory(handle, &data, buffer.get(), a_size)); + ASSERT_EQ(0, memcmp(buffer.get(), kATxtContents.data(), a_size)); + + // An entry that's stored. + ASSERT_EQ(0, FindEntry(handle, "b.txt", &data)); + const uint32_t b_size = data.uncompressed_length; + ASSERT_EQ(b_size, kBTxtContents.size()); + buffer = std::unique_ptr(new uint8_t[b_size]); + ASSERT_EQ(0, ExtractToMemory(handle, &data, buffer.get(), b_size)); + ASSERT_EQ(0, memcmp(buffer.get(), kBTxtContents.data(), b_size)); + + CloseArchive(handle); +} + TEST(ziparchive, ExtractToMemory) { ZipArchiveHandle handle; ASSERT_EQ(0, OpenArchiveWrapper(kValidZip, &handle));