This is an automated email from the ASF dual-hosted git repository.
ggregory pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/commons-compress.git
The following commit(s) were added to refs/heads/master by this push:
new 7d2fde710 Support preamble garbage in ZipArchiveInputStream (#471)
7d2fde710 is described below
commit 7d2fde71046c31508483ca80abbdfcdb4d5c78b9
Author: Zbynek Vyskovsky <kvr...@gmail.com>
AuthorDate: Sun Jan 28 14:19:45 2024 -0800
Support preamble garbage in ZipArchiveInputStream (#471)
---
.../archivers/zip/ZipArchiveInputStream.java | 76 ++++++++++++++++------
.../archivers/zip/ZipArchiveInputStreamTest.java | 50 +++++++++++++-
.../compress/archivers/zip/ZipFileTest.java | 26 ++++++++
3 files changed, 132 insertions(+), 20 deletions(-)
diff --git
a/src/main/java/org/apache/commons/compress/archivers/zip/ZipArchiveInputStream.java
b/src/main/java/org/apache/commons/compress/archivers/zip/ZipArchiveInputStream.java
index 078ad68cf..75d935fab 100644
---
a/src/main/java/org/apache/commons/compress/archivers/zip/ZipArchiveInputStream.java
+++
b/src/main/java/org/apache/commons/compress/archivers/zip/ZipArchiveInputStream.java
@@ -177,6 +177,8 @@ public class ZipArchiveInputStream extends
ArchiveInputStream<ZipArchiveEntry> i
}
}
+ public static final int PREAMBLE_GARBAGE_MAX_SIZE = 4096;
+
private static final int LFH_LEN = 30;
/*
@@ -649,7 +651,11 @@ public class ZipArchiveInputStream extends
ArchiveInputStream<ZipArchiveEntry> i
// first local file header - look for it and fail with
// the appropriate error message if this is a split
// archive.
- readFirstLocalFileHeader();
+ if (!readFirstLocalFileHeader()) {
+ hitCentralDirectory = true;
+ skipRemainderOfArchive(true);
+ return null;
+ }
} else {
readFully(lfhBuf);
}
@@ -661,7 +667,7 @@ public class ZipArchiveInputStream extends
ArchiveInputStream<ZipArchiveEntry> i
if (!sig.equals(ZipLong.LFH_SIG)) {
if (sig.equals(ZipLong.CFH_SIG) || sig.equals(ZipLong.AED_SIG) ||
isApkSigningBlock(lfhBuf)) {
hitCentralDirectory = true;
- skipRemainderOfArchive();
+ skipRemainderOfArchive(false);
return null;
}
throw new ZipException(String.format("Unexpected record signature:
0x%x", sig.getValue()));
@@ -999,8 +1005,41 @@ public class ZipArchiveInputStream extends
ArchiveInputStream<ZipArchiveEntry> i
/**
* Fills the given array with the first local file header and deals with
splitting/spanning markers that may prefix the first LFH.
*/
- private void readFirstLocalFileHeader() throws IOException {
- readFully(lfhBuf);
+ private boolean readFirstLocalFileHeader() throws IOException {
+ try {
+ // for empty archive, we may get only EOCD size:
+ byte[] header = new byte[Math.min(LFH_LEN, ZipFile.MIN_EOCD_SIZE)];
+ readFully(header);
+ READ_LOOP: for (int i = 0; ; ) {
+ for (int j = 0; i <= PREAMBLE_GARBAGE_MAX_SIZE - 4 && j <=
header.length - 4; ++j, ++i) {
+ ZipLong sig = new ZipLong(header, j);
+ if (
+ sig.equals(ZipLong.LFH_SIG) ||
+ sig.equals(ZipLong.SINGLE_SEGMENT_SPLIT_MARKER) ||
+ sig.equals(ZipLong.DD_SIG)) {
+ // regular archive containing at least one entry:
+ System.arraycopy(header, j, header, 0, header.length -
j);
+ readFully(header, header.length - j);
+ break READ_LOOP;
+ } else if (
+ sig.equals(new
ZipLong(ZipArchiveOutputStream.EOCD_SIG))
+ ) {
+ // empty archive:
+ pushback(header, j, header.length - j);
+ return false;
+ }
+ }
+ if (i >= PREAMBLE_GARBAGE_MAX_SIZE - 4) {
+ throw new ZipException("Cannot find zip signature within
the first " + PREAMBLE_GARBAGE_MAX_SIZE + " bytes");
+ }
+ System.arraycopy(header, header.length - 3, header, 0, 3);
+ readFully(header, 3);
+ }
+ System.arraycopy(header, 0, lfhBuf, 0, header.length);
+ readFully(lfhBuf, header.length);
+ } catch (EOFException ex) {
+ throw new ZipException("Cannot find zip signature within the
file");
+ }
final ZipLong sig = new ZipLong(lfhBuf);
if (!skipSplitSig && sig.equals(ZipLong.DD_SIG)) {
@@ -1010,11 +1049,10 @@ public class ZipArchiveInputStream extends
ArchiveInputStream<ZipArchiveEntry> i
// the split ZIP signature(08074B50) should only be skipped when the
skipSplitSig is set
if (sig.equals(ZipLong.SINGLE_SEGMENT_SPLIT_MARKER) ||
sig.equals(ZipLong.DD_SIG)) {
// Just skip over the marker.
- final byte[] missedLfhBytes = new byte[4];
- readFully(missedLfhBytes);
- System.arraycopy(lfhBuf, 4, lfhBuf, 0, LFH_LEN - 4);
- System.arraycopy(missedLfhBytes, 0, lfhBuf, LFH_LEN - 4, 4);
+ System.arraycopy(lfhBuf, 4, lfhBuf, 0, lfhBuf.length - 4);
+ readFully(lfhBuf, lfhBuf.length - 4);
}
+ return true;
}
/**
@@ -1240,22 +1278,22 @@ public class ZipArchiveInputStream extends
ArchiveInputStream<ZipArchiveEntry> i
/**
* Reads the stream until it find the "End of central directory record"
and consumes it as well.
*/
- private void skipRemainderOfArchive() throws IOException {
+ private void skipRemainderOfArchive(boolean read) throws IOException {
// skip over central directory. One LFH has been read too much
// already. The calculation discounts file names and extra
// data, so it will be too short.
if (entriesRead > 0) {
realSkip((long) entriesRead * CFH_LEN - LFH_LEN);
- final boolean foundEocd = findEocdRecord();
- if (foundEocd) {
- realSkip((long) ZipFile.MIN_EOCD_SIZE - WORD /* signature */ -
SHORT /* comment len */);
- readFully(shortBuf);
- // file comment
- final int commentLen = ZipShort.getValue(shortBuf);
- if (commentLen >= 0) {
- realSkip(commentLen);
- return;
- }
+ }
+ final boolean foundEocd = findEocdRecord();
+ if (foundEocd) {
+ realSkip((long) ZipFile.MIN_EOCD_SIZE - WORD /* signature */ -
SHORT /* comment len */);
+ readFully(shortBuf);
+ // file comment
+ final int commentLen = ZipShort.getValue(shortBuf);
+ if (commentLen >= 0) {
+ realSkip(commentLen);
+ return;
}
}
throw new IOException("Truncated ZIP file");
diff --git
a/src/test/java/org/apache/commons/compress/archivers/zip/ZipArchiveInputStreamTest.java
b/src/test/java/org/apache/commons/compress/archivers/zip/ZipArchiveInputStreamTest.java
index a40acba2d..5d64283c6 100644
---
a/src/test/java/org/apache/commons/compress/archivers/zip/ZipArchiveInputStreamTest.java
+++
b/src/test/java/org/apache/commons/compress/archivers/zip/ZipArchiveInputStreamTest.java
@@ -32,8 +32,10 @@ import java.io.EOFException;
import java.io.File;
import java.io.IOException;
import java.io.InputStream;
+import java.io.OutputStream;
import java.nio.channels.Channels;
import java.nio.channels.SeekableByteChannel;
+import java.nio.charset.StandardCharsets;
import java.nio.file.Files;
import java.nio.file.Path;
import java.nio.file.Paths;
@@ -570,7 +572,7 @@ public class ZipArchiveInputStreamTest extends AbstractTest
{
public void testThrowOnInvalidEntry() throws Exception {
try (ZipArchiveInputStream zip = new
ZipArchiveInputStream(ZipArchiveInputStreamTest.class.getResourceAsStream("/invalid-zip.zip")))
{
final ZipException expected = assertThrows(ZipException.class,
zip::getNextZipEntry, "IOException expected");
- assertTrue(expected.getMessage().contains("Unexpected record
signature"));
+ assertTrue(expected.getMessage().contains("Cannot find zip
signature"));
}
}
@@ -724,4 +726,50 @@ public class ZipArchiveInputStreamTest extends
AbstractTest {
// Ignore expected exception
}
}
+
+ @Test
+ public void testZipWithShortBeginningGarbage() throws IOException {
+ Path path = createTempPath("preamble", ".zip");
+
+ try (OutputStream fos = Files.newOutputStream(path)) {
+ fos.write("#!/usr/bin/unzip\n".getBytes(StandardCharsets.UTF_8));
+ try (ZipArchiveOutputStream zos = new ZipArchiveOutputStream(fos))
{
+ ZipArchiveEntry entry = new ZipArchiveEntry("file-1.txt");
+ entry.setMethod(ZipEntry.DEFLATED);
+ zos.putArchiveEntry(entry);
+ zos.write("entry-content\n".getBytes(StandardCharsets.UTF_8));
+ zos.closeArchiveEntry();
+ }
+ }
+
+ try (InputStream is = Files.newInputStream(path);
ZipArchiveInputStream zis = new ZipArchiveInputStream(is)) {
+ ZipArchiveEntry entry = zis.getNextEntry();
+ assertEquals("file-1.txt", entry.getName());
+ byte[] content = IOUtils.toByteArray(zis);
+
assertArrayEquals("entry-content\n".getBytes(StandardCharsets.UTF_8), content);
+ }
+ }
+
+ @Test
+ public void testZipWithLongerBeginningGarbage() throws IOException {
+ Path path = createTempPath("preamble", ".zip");
+
+ try (OutputStream fos = Files.newOutputStream(path)) {
+ fos.write("#!/usr/bin/env some-program with quite a few arguments
to make it longer than the local header\n".getBytes(StandardCharsets.UTF_8));
+ try (ZipArchiveOutputStream zos = new ZipArchiveOutputStream(fos))
{
+ ZipArchiveEntry entry = new ZipArchiveEntry("file-1.txt");
+ entry.setMethod(ZipEntry.DEFLATED);
+ zos.putArchiveEntry(entry);
+ zos.write("entry-content\n".getBytes(StandardCharsets.UTF_8));
+ zos.closeArchiveEntry();
+ }
+ }
+
+ try (InputStream is = Files.newInputStream(path);
ZipArchiveInputStream zis = new ZipArchiveInputStream(is)) {
+ ZipArchiveEntry entry = zis.getNextEntry();
+ assertEquals("file-1.txt", entry.getName());
+ byte[] content = IOUtils.toByteArray(zis);
+
assertArrayEquals("entry-content\n".getBytes(StandardCharsets.UTF_8), content);
+ }
+ }
}
diff --git
a/src/test/java/org/apache/commons/compress/archivers/zip/ZipFileTest.java
b/src/test/java/org/apache/commons/compress/archivers/zip/ZipFileTest.java
index 2b7d1fa20..72418bfa2 100644
--- a/src/test/java/org/apache/commons/compress/archivers/zip/ZipFileTest.java
+++ b/src/test/java/org/apache/commons/compress/archivers/zip/ZipFileTest.java
@@ -37,6 +37,7 @@ import java.io.OutputStream;
import java.nio.channels.SeekableByteChannel;
import java.nio.charset.StandardCharsets;
import java.nio.file.Files;
+import java.nio.file.Path;
import java.nio.file.Paths;
import java.util.ArrayList;
import java.util.Arrays;
@@ -956,4 +957,29 @@ public class ZipFileTest extends AbstractTest {
assertNull(zf.getEntry("\u00e4\\\u00fc.txt"));
assertNotNull(zf.getEntry("\u00e4/\u00fc.txt"));
}
+
+ @Test
+ public void testZipWithShortBeginningGarbage() throws IOException {
+ Path path = createTempPath("preamble", ".zip");
+
+ try (OutputStream fos = Files.newOutputStream(path)) {
+ fos.write("#!/usr/bin/unzip\n".getBytes(StandardCharsets.UTF_8));
+ try (ZipArchiveOutputStream zos = new ZipArchiveOutputStream(fos))
{
+ ZipArchiveEntry entry = new ZipArchiveEntry("file-1.txt");
+ entry.setMethod(ZipEntry.DEFLATED);
+ zos.putArchiveEntry(entry);
+ zos.write("entry-content\n".getBytes(StandardCharsets.UTF_8));
+ zos.closeArchiveEntry();
+ }
+ }
+
+ try (ZipFile zipFile = ZipFile.builder().setPath(path).get()) {
+ ZipArchiveEntry entry = zipFile.getEntry("file-1.txt");
+ assertEquals("file-1.txt", entry.getName());
+ byte[] content =
IOUtils.toByteArray(zipFile.getInputStream(entry));
+
assertArrayEquals("entry-content\n".getBytes(StandardCharsets.UTF_8), content);
+ }
+ }
+
+
}
