You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@commons.apache.org by gg...@apache.org on 2025/08/05 21:20:12 UTC
(commons-compress) 02/02: org.apache.commons.compress.archivers.dump.DumpArchiveInputStream.getNextEntry() throws an ArchiveException instead of ArrayIndexOutOfBoundsException
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
commit 4b63450f292401f22f630cdf1f49e16224f2d69f
Author: Gary D. Gregory <ga...@gmail.com>
AuthorDate: Tue Aug 5 17:20:04 2025 -0400
org.apache.commons.compress.archivers.dump.DumpArchiveInputStream.getNextEntry()
throws an ArchiveException instead of ArrayIndexOutOfBoundsException
---
src/changes/changes.xml | 1 +
.../compress/archivers/dump/DumpArchiveEntry.java | 22 +--------------------
.../archivers/dump/DumpArchiveInputStream.java | 3 +++
.../archivers/dump/DumpArchiveInputStreamTest.java | 11 +++++++++++
.../apache/commons/compress/dump/getNextEntry.bin | Bin 0 -> 45610 bytes
5 files changed, 16 insertions(+), 21 deletions(-)
diff --git a/src/changes/changes.xml b/src/changes/changes.xml
index 6d915d6c7..8b270ebe7 100644
--- a/src/changes/changes.xml
+++ b/src/changes/changes.xml
@@ -55,6 +55,7 @@ The <action> type attribute can be add,update,fix,remove.
<action type="fix" dev="ggregory" due-to="Tyler Nighswander, Gary Gregory">org.apache.commons.compress.archivers.sevenz.SevenZFile.Builder.get() now throws a MemoryLimitException (an IOException subclass) instead of an OOME.</action>
<action type="fix" dev="ggregory" due-to="Tyler Nighswander, Gary Gregory">org.apache.commons.compress.compressors.bzip2.BZip2CompressorInputStream constructors now throws CompressorException instead of ArrayIndexOutOfBoundsException.</action>
<action type="fix" dev="ggregory" due-to="Tyler Nighswander, Gary Gregory">org.apache.commons.compress.archivers.dump.TapeInputStream.resetBlockSize(int, boolean) now throws a MemoryLimitException (an IOException subclass) instead of an OOME.</action>
+ <action type="fix" dev="ggregory" due-to="Tyler Nighswander, Gary Gregory">org.apache.commons.compress.archivers.dump.DumpArchiveInputStream.getNextEntry() throws an ArchiveException instead of ArrayIndexOutOfBoundsException.</action>
<action type="fix" dev="ggregory" due-to="Tyler Nighswander, Gary Gregory">org.apache.commons.compress.archivers.zip.ZipFile.createBoundedInputStream(long, long) now throws an ArchiveException instead of IllegalArgumentException.</action>
<action type="fix" dev="ggregory" due-to="Tyler Nighswander, Gary Gregory">org.apache.commons.compress.archivers.zip.ZipFile.getContentBeforeFirstLocalFileHeader() now throws an ArchiveException instead of IllegalArgumentException.</action>
<action type="fix" dev="ggregory" due-to="Tyler Nighswander, Gary Gregory">org.apache.commons.compress.archivers.tar.TarArchiveInputStream.paxHeaders() now throws an ArchiveException if there are too many PAX headers; override with -D'TarArchiveInputStream.MAX_PAX_HEADER_DEPTH=850'.</action>
diff --git a/src/main/java/org/apache/commons/compress/archivers/dump/DumpArchiveEntry.java b/src/main/java/org/apache/commons/compress/archivers/dump/DumpArchiveEntry.java
index 3b81dc001..1e54c02c6 100644
--- a/src/main/java/org/apache/commons/compress/archivers/dump/DumpArchiveEntry.java
+++ b/src/main/java/org/apache/commons/compress/archivers/dump/DumpArchiveEntry.java
@@ -273,7 +273,7 @@ public static Set<PERMISSION> find(final int code) {
* Archive entry as stored on tape. There is one TSH for (at most) every 512k in the file.
*/
static final class TapeSegmentHeader {
- private static final int CDATA_LEN = 512;
+ static final int CDATA_LEN = 512;
private DumpArchiveConstants.SEGMENT_TYPE type;
private int volume;
private int ino;
@@ -391,37 +391,29 @@ public static TYPE find(final int code) {
static DumpArchiveEntry parse(final byte[] buffer) {
final DumpArchiveEntry entry = new DumpArchiveEntry();
final TapeSegmentHeader header = entry.header;
-
header.type = DumpArchiveConstants.SEGMENT_TYPE.find(DumpArchiveUtil.convert32(buffer, 0));
-
// header.dumpDate = new Date(1000L * DumpArchiveUtil.convert32(buffer, 4));
// header.previousDumpDate = new Date(1000L * DumpArchiveUtil.convert32(
// buffer, 8));
header.volume = DumpArchiveUtil.convert32(buffer, 12);
// header.tapea = DumpArchiveUtil.convert32(buffer, 16);
entry.ino = header.ino = DumpArchiveUtil.convert32(buffer, 20);
-
// header.magic = DumpArchiveUtil.convert32(buffer, 24);
// header.checksum = DumpArchiveUtil.convert32(buffer, 28);
final int m = DumpArchiveUtil.convert16(buffer, 32);
-
// determine the type of the file.
entry.setType(TYPE.find(m >> 12 & 0x0F));
-
// determine the standard permissions
entry.setMode(m);
-
entry.nlink = DumpArchiveUtil.convert16(buffer, 34);
// inumber, oldids?
entry.setSize(DumpArchiveUtil.convert64(buffer, 40));
-
long t = 1000L * DumpArchiveUtil.convert32(buffer, 48) + DumpArchiveUtil.convert32(buffer, 52) / 1000;
entry.setAccessTime(new Date(t));
t = 1000L * DumpArchiveUtil.convert32(buffer, 56) + DumpArchiveUtil.convert32(buffer, 60) / 1000;
entry.setLastModifiedDate(new Date(t));
t = 1000L * DumpArchiveUtil.convert32(buffer, 64) + DumpArchiveUtil.convert32(buffer, 68) / 1000;
entry.ctime = t;
-
// db: 72-119 - direct blocks
// id: 120-131 - indirect blocks
// entry.flags = DumpArchiveUtil.convert32(buffer, 132);
@@ -431,19 +423,14 @@ static DumpArchiveEntry parse(final byte[] buffer) {
entry.setGroupId(DumpArchiveUtil.convert32(buffer, 148));
// two 32-bit spare values.
header.count = DumpArchiveUtil.convert32(buffer, 160);
-
header.holes = 0;
-
for (int i = 0; i < 512 && i < header.count; i++) {
if (buffer[164 + i] == 0) {
header.holes++;
}
}
-
System.arraycopy(buffer, 164, header.cdata, 0, 512);
-
entry.volume = header.getVolume();
-
// entry.isSummaryOnly = false;
return entry;
}
@@ -528,19 +515,15 @@ public boolean equals(final Object o) {
if (o == null || !o.getClass().equals(getClass())) {
return false;
}
-
final DumpArchiveEntry rhs = (DumpArchiveEntry) o;
-
if (ino != rhs.ino) {
return false;
}
-
// summary is always null right now, but this may change some day
if (summary == null && rhs.summary != null // NOSONAR
|| summary != null && !summary.equals(rhs.summary)) { // NOSONAR
return false;
}
-
return true;
}
@@ -974,15 +957,12 @@ public String toString() {
void update(final byte[] buffer) {
header.volume = DumpArchiveUtil.convert32(buffer, 16);
header.count = DumpArchiveUtil.convert32(buffer, 160);
-
header.holes = 0;
-
for (int i = 0; i < 512 && i < header.count; i++) {
if (buffer[164 + i] == 0) {
header.holes++;
}
}
-
System.arraycopy(buffer, 164, header.cdata, 0, 512);
}
}
diff --git a/src/main/java/org/apache/commons/compress/archivers/dump/DumpArchiveInputStream.java b/src/main/java/org/apache/commons/compress/archivers/dump/DumpArchiveInputStream.java
index aa3825f9a..bc6e9a8c5 100644
--- a/src/main/java/org/apache/commons/compress/archivers/dump/DumpArchiveInputStream.java
+++ b/src/main/java/org/apache/commons/compress/archivers/dump/DumpArchiveInputStream.java
@@ -220,6 +220,9 @@ public DumpArchiveEntry getNextEntry() throws IOException {
// we might still have holes... easiest to do it
// block by block. We may want to revisit this if
// the unnecessary decompression time adds up.
+ if (active.getHeaderCount() >= DumpArchiveEntry.TapeSegmentHeader.CDATA_LEN) {
+ throw new ArchiveException("Header count");
+ }
while (readIdx < active.getHeaderCount()) {
if (!active.isSparseRecord(readIdx++) && raw.skip(DumpArchiveConstants.TP_SIZE) == -1) {
throw new EOFException();
diff --git a/src/test/java/org/apache/commons/compress/archivers/dump/DumpArchiveInputStreamTest.java b/src/test/java/org/apache/commons/compress/archivers/dump/DumpArchiveInputStreamTest.java
index 46aa17ae0..6012dbb7c 100644
--- a/src/test/java/org/apache/commons/compress/archivers/dump/DumpArchiveInputStreamTest.java
+++ b/src/test/java/org/apache/commons/compress/archivers/dump/DumpArchiveInputStreamTest.java
@@ -26,7 +26,10 @@
import static org.junit.jupiter.api.Assertions.assertTimeoutPreemptively;
import static org.junit.jupiter.api.Assertions.assertTrue;
+import java.io.IOException;
import java.io.InputStream;
+import java.nio.file.Files;
+import java.nio.file.Paths;
import java.time.Duration;
import java.util.concurrent.TimeUnit;
@@ -62,6 +65,14 @@ void testDirectoryNullBytes() throws Exception {
}
}
+ @Test
+ void testGetNextEntry() throws IOException {
+ try (DumpArchiveInputStream inputStream = new DumpArchiveInputStream(
+ Files.newInputStream(Paths.get("src/test/resources/org/apache/commons/compress/dump/getNextEntry.bin")))) {
+ assertThrows(ArchiveException.class, inputStream::getNextEntry);
+ }
+ }
+
@Test
void testInvalidCompressType() throws Exception {
try (InputStream is = newInputStream("org/apache/commons/compress/dump/invalid_compression_type-fail.dump")) {
diff --git a/src/test/resources/org/apache/commons/compress/dump/getNextEntry.bin b/src/test/resources/org/apache/commons/compress/dump/getNextEntry.bin
new file mode 100644
index 000000000..9f0165168
Binary files /dev/null and b/src/test/resources/org/apache/commons/compress/dump/getNextEntry.bin differ