[8.8.0] Fix Zip64 archive central directory extraction in ZipReader (https://github.com/bazelbuild/bazel/pull/30685) (#30764)
### Description
Fixes #30681.
In commit b12c06a2d54684cdf9a83d9ecc634850a7858db6 (#30530),
`ZipReader.readCentralDirectory()` was modified to compute the central
directory start as `actualCenStart = actualCenEnd -
zipData.getCentralDirectorySize()`, replacing the parsed
`zipData.getCentralDirectoryOffset()`.
This broke reading standard Zip64 archives where 32-bit EOCD fields do
not overflow (as the 76-byte Zip64 EOCD and locator sit between the
central directory and the 32-bit EOCD, shifting the computed offset 76
bytes into the central directory) or when `centralDirectorySize` is
`0xFFFFFFFF`.
This change:
1. Restores `ZipReader`'s standard central directory parsing to use
`zipData.getCentralDirectoryOffset()`.
2. Populates `centralDirectorySize` in
`Zip64EndOfCentralDirectory.read()`.
3. Restricts the unadjusted SFX central directory start computation
strictly to `AdjustSfx`.
4. Adds a unit test in `ZipReaderTest` covering Zip64 archives with
valid 32-bit EOCD offsets.
Closes #30685.
PiperOrigin-RevId: 966082613
Change-Id: I4c9583901610aeed4ad4bfce75915b32eb6b2c86
Commit
https://github.com/bazelbuild/bazel/commit/3dd0a0577005b741b83020ba669e8f9276969af3
Co-authored-by: Yun Peng <pcloudy@google.com>
Co-authored-by: Ian (Hee) Cha <heec@google.com>
diff --git a/src/java_tools/singlejar/java/com/google/devtools/build/zip/AdjustSfx.java b/src/java_tools/singlejar/java/com/google/devtools/build/zip/AdjustSfx.java
index d9b4b8b..4e3d158 100644
--- a/src/java_tools/singlejar/java/com/google/devtools/build/zip/AdjustSfx.java
+++ b/src/java_tools/singlejar/java/com/google/devtools/build/zip/AdjustSfx.java
@@ -41,7 +41,7 @@
File inputFile = new File(args[0]);
File outputFile = new File(args[1]);
- try (ZipReader reader = new ZipReader(inputFile, UTF_8, false);
+ try (ZipReader reader = ZipReader.createForAdjustSfx(inputFile, UTF_8);
InputStream in = new FileInputStream(inputFile);
OutputStream out = new BufferedOutputStream(new FileOutputStream(outputFile))) {
diff --git a/src/java_tools/singlejar/java/com/google/devtools/build/zip/Zip64EndOfCentralDirectory.java b/src/java_tools/singlejar/java/com/google/devtools/build/zip/Zip64EndOfCentralDirectory.java
index 58424b8..9432f27 100644
--- a/src/java_tools/singlejar/java/com/google/devtools/build/zip/Zip64EndOfCentralDirectory.java
+++ b/src/java_tools/singlejar/java/com/google/devtools/build/zip/Zip64EndOfCentralDirectory.java
@@ -51,6 +51,7 @@
"Malformed Zip64 End of Central Directory; does not start with %08x", SIGNATURE));
}
file.setZip64(true);
+ file.setCentralDirectorySize(ZipUtil.getUnsignedLong(fixedSizeData, CD_SIZE_OFFSET));
file.setCentralDirectoryOffset(ZipUtil.getUnsignedLong(fixedSizeData, CD_OFFSET_OFFSET));
file.setExpectedEntries(ZipUtil.getUnsignedLong(fixedSizeData, TOTAL_ENTRIES_OFFSET));
return file;
diff --git a/src/java_tools/singlejar/java/com/google/devtools/build/zip/ZipReader.java b/src/java_tools/singlejar/java/com/google/devtools/build/zip/ZipReader.java
index 0945431..2769c02 100644
--- a/src/java_tools/singlejar/java/com/google/devtools/build/zip/ZipReader.java
+++ b/src/java_tools/singlejar/java/com/google/devtools/build/zip/ZipReader.java
@@ -85,13 +85,22 @@
* @throws IOException if an I/O error has occurred
*/
public ZipReader(File file, Charset charset, boolean strictEntries) throws IOException {
+ this(file, charset, strictEntries, /* isUnadjustedSfx= */ false);
+ }
+
+ ZipReader(File file, Charset charset, boolean strictEntries, boolean isUnadjustedSfx)
+ throws IOException {
if (file == null || charset == null) {
throw new NullPointerException();
}
this.file = file;
this.in = new RandomAccessFile(file, "r");
this.zipData = new ZipFileData(charset);
- readCentralDirectory(strictEntries);
+ readCentralDirectory(strictEntries, isUnadjustedSfx);
+ }
+
+ static ZipReader createForAdjustSfx(File file, Charset charset) throws IOException {
+ return new ZipReader(file, charset, /* strictEntries= */ false, /* isUnadjustedSfx= */ true);
}
/**
@@ -189,12 +198,13 @@
/**
* Finds, reads and parses ZIP file entries from the central directory.
*
- * @param strictEntries force parsing to use the number of entries recorded in the end of
- * central directory as the correct value, not as an estimate
+ * @param strictEntries force parsing to use the number of entries recorded in the end of central
+ * directory as the correct value, not as an estimate
* @throws ZipException if a ZIP format error has occurred
* @throws IOException if an I/O error has occurred
*/
- private void readCentralDirectory(boolean strictEntries) throws IOException {
+ private void readCentralDirectory(boolean strictEntries, boolean isUnadjustedSfx)
+ throws IOException {
long eocdLocation = findEndOfCentralDirectoryRecord();
InputStream stream = getStreamAt(eocdLocation);
EndOfCentralDirectoryRecord.read(stream, zipData);
@@ -211,16 +221,21 @@
}
}
- long actualCenEnd =
- (zipData.isZip64() && zipData.getZip64EndOfCentralDirectoryOffset() > 0)
- ? zipData.getZip64EndOfCentralDirectoryOffset()
- : eocdLocation;
- long actualCenStart = actualCenEnd - zipData.getCentralDirectorySize();
+ if (isUnadjustedSfx) {
+ long actualCenEnd =
+ (zipData.isZip64() && zipData.getZip64EndOfCentralDirectoryOffset() > 0)
+ ? zipData.getZip64EndOfCentralDirectoryOffset()
+ : eocdLocation;
+ long actualCenStart = actualCenEnd - zipData.getCentralDirectorySize();
+ readCentralDirectoryFileHeaders(zipData.getExpectedEntries(), actualCenStart);
+ return;
+ }
if (zipData.isZip64() || strictEntries) {
// If in Zip64 format or using strict entry numbers, use the parsed information as is to read
// the central directory file headers.
- readCentralDirectoryFileHeaders(zipData.getExpectedEntries(), actualCenStart);
+ readCentralDirectoryFileHeaders(
+ zipData.getExpectedEntries(), zipData.getCentralDirectoryOffset());
} else {
// If not in Zip64 format, compute central directory offset by end of central directory record
// offset and central directory size to allow reading large non-compliant Zip32 directories.
@@ -230,7 +245,8 @@
if ((int) centralDirectoryOffset == (int) zipData.getCentralDirectoryOffset()) {
readCentralDirectoryFileHeaders(centralDirectoryOffset);
} else {
- readCentralDirectoryFileHeaders(zipData.getExpectedEntries(), actualCenStart);
+ readCentralDirectoryFileHeaders(
+ zipData.getExpectedEntries(), zipData.getCentralDirectoryOffset());
}
}
}
diff --git a/src/java_tools/singlejar/javatests/com/google/devtools/build/zip/ZipReaderTest.java b/src/java_tools/singlejar/javatests/com/google/devtools/build/zip/ZipReaderTest.java
index ae3ae00..4705df9 100644
--- a/src/java_tools/singlejar/javatests/com/google/devtools/build/zip/ZipReaderTest.java
+++ b/src/java_tools/singlejar/javatests/com/google/devtools/build/zip/ZipReaderTest.java
@@ -477,6 +477,45 @@
}
}
+ @Test
+ public void testZip64_WithZip32Offsets() throws IOException {
+ // A zip file with Zip64 EOCD and Locator, but with non-overflowing 32-bit values in EOCD (as
+ // generated by Python / standard tools for archives with Zip64 entries).
+ byte[] data =
+ new byte[] {
+ 0x50, 0x4b, 0x03, 0x04, 0x14, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+ 0x00,
+ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x04, 0x00, 0x00, 0x00, 0x74,
+ 0x65,
+ 0x73, 0x74, 0x50, 0x4b, 0x01, 0x02, 0x14, 0x00, 0x14, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+ 0x00,
+ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x04,
+ 0x00,
+ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+ 0x00,
+ 0x74, 0x65, 0x73, 0x74, 0x50, 0x4b, 0x06, 0x06, 0x2c, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+ 0x00,
+ 0x2d, 0x00, 0x2d, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x01, 0x00, 0x00,
+ 0x00,
+ 0x00, 0x00, 0x00, 0x00, 0x01, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x32, 0x00, 0x00,
+ 0x00,
+ 0x00, 0x00, 0x00, 0x00, 0x22, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x50, 0x4b, 0x06,
+ 0x07,
+ 0x00, 0x00, 0x00, 0x00, 0x54, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x01, 0x00, 0x00,
+ 0x00,
+ 0x50, 0x4b, 0x05, 0x06, 0x00, 0x00, 0x00, 0x00, 0x01, 0x00, 0x01, 0x00, 0x32, 0x00, 0x00,
+ 0x00,
+ 0x22, 0x00, 0x00, 0x00, 0x00, 0x00
+ };
+ try (FileOutputStream out = new FileOutputStream(test)) {
+ out.write(data);
+ }
+ try (ZipReader reader = new ZipReader(test, UTF_8)) {
+ assertThat(reader.entries()).hasSize(1);
+ assertThat(reader.getEntry("test")).isNotNull();
+ }
+ }
+
@Test public void testZip64_Potential() throws IOException {
try (ZipWriter writer = new ZipWriter(new FileOutputStream(test), UTF_8, true)) {
ZipFileEntry template = new ZipFileEntry("template");
@@ -612,7 +651,7 @@
// Initializing ZipReader parses the EOCD header. This verifies that signatureLocation + 20
// falling across the 64-byte buffer boundary is handled gracefully without throwing an
// ArrayIndexOutOfBoundsException.
- try (ZipReader reader = new ZipReader(sfxFile, UTF_8)) {
+ try (ZipReader reader = ZipReader.createForAdjustSfx(sfxFile, UTF_8)) {
assertThat(reader.entries()).hasSize(1);
ZipFileEntry entry = reader.getEntry("foo.txt");
assertThat(entry).isNotNull();
