The month field is one based in the zipfile modification time. And
it causes an overflow converting it to struct tm. Switch to type to
signed integer to suppress the sub-overflow.
Bug: 153882979
Test: parse the problematic zipfile
Change-Id: Iaf47bcc7f83d61b18c9e7a98bb6ab3936c9257e3
We should check if the data to read resides within the boundary of
the extended field. Also check OOB when reading bytes from the
zipfile.
Bug: 153828925
Test: parse the poc with hwasan build
Change-Id: I54b58a287b9ae4ca0e5cc563086c1ed8051fb72a
Crc calculation shows up in the profiler in 2-5% range, and is
never currently validated. Let's disable it for good.
For a well-compressible test data the difference is even nicer:
Benchmark Time CPU Iteration
------------------------------------------------------------------
ziparchive-benchmarks:
before:
#ExtractEntry/2 1943244 ns 1926758 ns 375
#ExtractEntry/16 1877295 ns 1867049 ns 375
#ExtractEntry/1024 1888772 ns 1879976 ns 373
after:
#ExtractEntry/2 817003 ns 812870 ns 874
#ExtractEntry/16 814029 ns 809813 ns 875
#ExtractEntry/1024 804904 ns 800972 ns 879
Bug: 153392568
Test: atest, manual
Change-Id: I917abecab01301f1d09a5bf3b542d24b3875e359
We don't actually need to extract the empty entries. Since the old
code support extracting the empty entry to a empty buffer, add the
support back in ExtractToMemory.
Bug: 153393683
Test: unittests pass
Change-Id: Idb9f0f4e6e4ffd4b44b80ddd3f54069bb7cedd7b
This cl supports the parsing and extraction of the zip entry who
has a large size than UINT32_MAX. Also add a few checks in the
entry writers to make sure callers have enough space for extraction.
As many users of the library assume the entry size to be 32 bits long,
we keep the 32 bit ZipEntry. We also keep the functions that expect
the 32 bit ZipEntry in the public header file. These 32 bit wrappers
could be removed later once all users recognize the 64 bit ZipEntry.
Bug: 150900468
Test: unit tests pass
Change-Id: Ia6760638ccf51e97dbef6bd55dff352f1e7ce816
The size fields in the data descriptor can be either 4 bytes or 8 bytes.
This depends on if the size are read from the zip64 extended field in
the local file header. This cl adds support to parse these cases.
Also fix a misconception in that the uncompressed and compressed size
doesn't need to exist together in the zip64 fields of the central
directory. But they still need to co-exist in the fields of the local
file header.
Bug: 150900468
Test: unit tests pass, python tests pass
Change-Id: Ia54f9bf56c85ff456ead90a136f7fddc5be5220c
The old check has some missing cases and may lead to read OOB.
Bug: 152433916
Test: unit tests pass
Change-Id: I3e8705b9c2db081228c5f9bd191c133668376ff2
Implement the logic to parse zip64 eocd and zip64 extended info
in the extra field. Also add unit tests and python tests which
create packages larger than 4GiB.
The extraction of zip entry size > 4GiB will be supported in the follow
ups.
Bug: 150900468
Test: unit tests pass
Change-Id: I4cd9ebbd9709b3d2f9cd293625d2c79024bb45a5
To reduce the seeks for local file headers in large APK files, we can
specify entry prefix/suffix when we call StartIteration(). However,
some use cases need additional name matches that is outside the
prefix/suffix matches.
Adding a new option to StartIteration, which allows additional functor
that restricts the iteration to customized name matching schemes.
Test: atest ziparchive-tests
BUG: 151676293
Change-Id: Iff45e083b334602f183c05cb39ba521e7070252c
Add the definition of zip64 related structs. Also add place holders in
the zip parsing code. In addition, this cl changes the variable type of
num of entries to uint64_t. The number was capped at UINT16_MAX in zip32
format.
Bug: 150900468
Test: unit tests pass
Change-Id: I51a39e7b993fa376e0d050a04b8d39abae8a9e15
To allow the ResourcesLoader API to load part of a file as an APK
that contains resources, an additional override of OpenArchiveFd
that contains read offset and length as parameters must be created.
This functionality allows for an APK stored in a zip file to be read
without having to write the APK to disk.
Bug: 142716192
Test: atest FrameworksResourceLoaderTests
Change-Id: I772fc8b462d71de0529717c420ced552103a6e3f
Merged-In: I772fc8b462d71de0529717c420ced552103a6e3f
Add the std::map implementation to be used later in zip64 format.
Also move the entry map classes to a separate file to make the hierarchy
clear.
Test: unittests pass
Change-Id: I74d95f53207cc8ca871b955e2a15c184d5497833
The current implementation of the hashtable uses less memory than
a std::map. As most of the zip files we encountered don't use the zip64
extension, we should keep the current implementation. And the interface
adds the flexibility for us to switch to std::map for zip64 format.
Bug: 150900468
Test: unit tests pass
Change-Id: Ifd008785c9ff416a27049f9e0c54d9eef985bd85
d77c99ebc3 changed MappedFile to return a
bogus zero-length mapping on failure rather than nullptr. None of the
calling code was changed, though, and it seems like doing so would be a
bad idea. Revert that part of the change.
Add missing tests, and tidy up some of the logging. Also remove
single-use or obfuscatory constants from the tests.
The new "empty.zip" was created by using zip(1) to create a zip file
with one entry, then using `zip -d` to remove it.
The new "zero-size-cd.zip" was created by using zip(1) to create a zip
file containing a single empty file, and then hex editing the two byte
"size of the central directory" field in the "end of central directory
record" structure at the end of the file. (This is equivalent to, but
much smaller than, the example zip file provided by the bug reporter.)
Bug: http://b/145925341
Test: treehugger
Change-Id: Iff64673bce7dae886ccbc9dd6c2bbe18de19f9d2
golang doesn't include Unix mode by default.
Also show all the deflate variants ("defN" versus "defX").
Cope better with being called directly rather than via symlink.
Test: manual
Change-Id: I23b441c847ce9a557ea866b3c43bdf0542b26f10
Useful for debugging and hermetic builds. (Various places in the build
check to see that a file was stored uncompressed.)
Test: manual
Change-Id: I127e5689cd493ab06739b765beed50912dc9cc1d
Didn't find anything when I ran it, but it did get me to fix the
const/non-const void* in the API.
Test: treehugger
Change-Id: If3849d974965e3e5ffcbdaf5e47921316d717410
All but one existing caller actually wants a std::string.
Bug: http://b/129068177
Test: treehugger
Change-Id: I428c4453edaae74451db56e9542e4e462f08d43a
Same issue as with FindEntry: using ZipString in the API forces all
callers to make sure they don't hit the ZipString length limits. Switch
to std::string_view and uniformly use the empty string as a way to
signal no prefix/suffix rather than nullptr.
Also use default arguments to make the common case of no prefix and no
suffix more convenient.
Also just use std::string to increase the lifetime of the provided
prefix/suffix rather than manual memory management.
Bug: http://b/129068177
Test: treehugger
Change-Id: I6675e39ce62fadd766386d77d27423013c17d6f7
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
Enable -Wconversion (but not -Wsign-conversion). Fix up code. Handle
some actual error cases:
* too many files
* files too large
Bug: 130039052
Test: atest ziparchive-tests
Change-Id: I632af317b9e65dbc728851efefd0d11a2b5c29b9
Although ubsan's implicit-unsigned-integer-truncation sanitizer may be
happy, this code still performs an implicit conversion from a wider
width data structure to a narrower width data structure. Rather than
masking the bits, make the conversion explicit. This keeps ubsan happy
as well as addressing a -Wconversion warning.
This change addresses comments from the post-submit review
of a4e5433660.
Test: compiles and boots.
Bug: 122975762
Change-Id: I1fa6d6f8a6fcfb93ba9916b7d2b3564ca1d8caf3
std::hash returns a 64 bit value, which is truncated to a 32 bit value
in ComputeHash. ubsan's implicit-unsigned-integer-truncation doesn't
like this implicit truncation and crashes the program. Explicitly strip
off the top order bits after computing the hash.
Remove the windows specific version of the hash computation. The windows
compile now uses clang, so this code is obsolete. This also avoids us
having to add __attribute__((no_sanitize("integer"))) to the windows
code.
This is needed to support Android booting with ubsan's
implicit-unsigned-integer-truncation option enabled.
Test: compiles and boots
Bug: 122975762
Change-Id: I2f05fbf5ffee8e90a66a6fda32e80de9cca246c0
The process of determining whether or not to emit a safetynet error
entry while processing a malformed file relies on addition overflow.
Since this is only logging, and logging which isn't used, delete the
code instead of trying to fix the logic which is causing the integer
overflow.
This change is necessary to enable integer sanitization on this code.
Somewhat related to Bug: 122975762
Test: atest ziparchive-tests
Change-Id: I6b41ccf7881348cb4e5236324eaa44a05662a725
Prevent file descriptors from leaking across an exec() boundary.
Bug: 120983106
Test: compiles and boots
Change-Id: I392b0767674b557b1e4404a2ba63bc48e3e37b24
system/core/libziparchive/zip_archive.cc:847:36: error: use of undeclared identifier 'lseek64'; did you mean 'lseek'?
const off64_t current_offset = lseek64(fd, 0, SEEK_CUR);
Bug: N/A
Test: builds
Change-Id: If762011722d53376bb6dab35c6ee8031762e5a95
This allows us to remove libziparchive's dependency on libutils.
Bug: http://b/79112958
Test: ran libbase and libziparchive tests, ran fastboot manually
Change-Id: I95c651976dad222863e5b8c37d4514b778f5dce7
A typedef to void* allows an implicit conversion from ZipArchiveHandle*
(or any other pointer type) to ZipArchiveHandle.
See I95d79809b6e118fb3c39c7b98b8055c8e324db1a in platform/bionic.
Bug: none
Test: m checkbuild
Change-Id: I3dd426cb64c46ef81e1dd81b4a2e4f40ac2701df
Debugging memory allocations on the nexus launcher unveiled significant memory allocations for the hashatable used in libziparchive, ~1MB.
This is partly because of the ZipString struct storing an entry in the table. The struct stored a pointer to a string (on 64 bit, 8 bytes) and the length to read from that pointer, 2 bytes. Because of alignment, the structure consumed 16 bytes, wasting 6 bytes.
Now, we store entries in the hashtable as a ZipStringOffset. This new structure stores a 4 byte offset from a fixed location in the memory mapped file instead of the entire address, consuming 8 bytes with alignment.
Bug: 79416399
Test: Builds successfully and manual testing by opening launcher on Pixel 2 shows precisely 50% decrease in memory allocated for the hashtable. From 909312 bytes to 454656.
Change-Id: I28b43699233fbee7f63fccae2d4fe96fcc07e5c4
Allows for opening zip files usng paths longer than 260 characters and
with unicode characters on Windows.
Bug: 113110184
Test: manual and libziparchive_tests
Change-Id: I9ce96ac2f1b1e448ae2a2f69c1d4cb3395ea79ee
Both Extract...() functions don't need dynamic allocation
for the writers, as those are strictly scoped. This CL
changes heap allocation to stack allocation.
Test: zip_archive_test
Change-Id: Id727e4b9848235cd063cc67ecbe052d21ca21326
Only compute the crc32 if required. In addition :
- Add unit tests for Inflate that cover this addition.
- Fix an inconsistency in return codes that was revealed
by this new test.
Bug: 35246701
Test: zip_archive_tests
Test: make; zipalign.
Merged-In: I31d7554378f94fc8995f707471d57cb98311e2c2
Change-Id: I05111bfa665c610f93d1c1dee987a509bf87aa65
Previously fastboot would carry on regardless if decompression failed:
fastboot: archive does not contain 'vbmeta.img'
fastboot: extracting vendor.img (260 MB)...
fastboot: W/ziparchive(56777): Zip: unable to allocate 272781472 bytes at offset 0 : No space left on device
fastboot: failed to extract 'vendor.img': I/O error
fastboot: archive does not contain 'vendor_other.img'
fastboot: wiping userdata...
This is because all but "boot" and "system" are considered "optional",
and the implementation of "optional" was "ignore any failures". What it
_should_ have meant was "it's okay if these don't exist, but if they do,
failures matter".
Fix this logic, use die() more aggressively, and remove spurious "\n"s
from die() format strings.
Also fix spurious whitespace in the libziparchive format string. Before:
Zip: unable to allocate 272781472 bytes at offset 0 : No space left on device
After:
Zip: unable to allocate 272781472 bytes at offset 0: No space left on device
Bug: http://b/68383022
Test: `fastboot update` on marlin
Change-Id: I3cbf55f1a33ca125f293f873eafbcfb86c880ba8
Tianjie