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 63c4c5e64 COMPRESS-632: Fixes and tests for ArInputStream (#440)
63c4c5e64 is described below
commit 63c4c5e6468c0faa0d9af05d563ba3fdccf32c21
Author: Yakov Shafranovich <yako...@users.noreply.github.com>
AuthorDate: Tue Nov 14 16:38:35 2023 -0500
COMPRESS-632: Fixes and tests for ArInputStream (#440)
* Fixes and tests for AR parsing issues
* added another test
* another test
---------
Co-authored-by: Yakov Shafranovich <yako...@amazon.com>
---
.../archivers/ar/ArArchiveInputStream.java | 60 ++++++++++++++++-----
.../archivers/ar/ArArchiveInputStreamTest.java | 29 ++++++++++
.../compress/ar/number_parsing/bad_group.ar | 8 +++
.../compress/ar/number_parsing/bad_length.ar | 8 +++
.../ar/number_parsing/bad_long_namelen_bsd.ar | 5 ++
.../ar/number_parsing/bad_long_namelen_gnu1.ar | 8 +++
.../ar/number_parsing/bad_long_namelen_gnu2.ar | 6 +++
.../ar/number_parsing/bad_long_namelen_gnu3.ar | Bin 0 -> 274 bytes
.../compress/ar/number_parsing/bad_modified.ar | 8 +++
.../ar/number_parsing/bad_table_length_gnu.ar | 8 +++
.../commons/compress/ar/number_parsing/bad_user.ar | 8 +++
11 files changed, 135 insertions(+), 13 deletions(-)
diff --git
a/src/main/java/org/apache/commons/compress/archivers/ar/ArArchiveInputStream.java
b/src/main/java/org/apache/commons/compress/archivers/ar/ArArchiveInputStream.java
index ddb9e6c6c..aba0f155a 100644
---
a/src/main/java/org/apache/commons/compress/archivers/ar/ArArchiveInputStream.java
+++
b/src/main/java/org/apache/commons/compress/archivers/ar/ArArchiveInputStream.java
@@ -205,8 +205,13 @@ public class ArArchiveInputStream extends
ArchiveInputStream<ArArchiveEntry> {
* @since 1.3
*/
private String getBSDLongName(final String bsdLongName) throws IOException
{
- final int nameLen =
- Integer.parseInt(bsdLongName.substring(BSD_LONGNAME_PREFIX_LEN));
+ final int nameLen;
+ try {
+ nameLen =
Integer.parseInt(bsdLongName.substring(BSD_LONGNAME_PREFIX_LEN));
+ } catch (NumberFormatException ex) {
+ throw new IOException("Broken archive, unable to parse BSD long
name length field as a number", ex);
+ }
+
final byte[] name = IOUtils.readRange(input, nameLen);
final int read = name.length;
trackReadBytes(read);
@@ -229,10 +234,19 @@ public class ArArchiveInputStream extends
ArchiveInputStream<ArArchiveEntry> {
}
for (int i = offset; i < namebuffer.length; i++) {
if (namebuffer[i] == '\012' || namebuffer[i] == 0) {
- if (namebuffer[i - 1] == '/') {
+ // Avoid array errors
+ if (i == 0) {
+ break;
+ } else if (namebuffer[i - 1] == '/') {
i--; // drop trailing /
}
- return ArchiveUtils.toAsciiString(namebuffer, offset, i -
offset);
+
+ // Check there is a something to return, otherwise break out
of the loop
+ if (i - offset > 0) {
+ return ArchiveUtils.toAsciiString(namebuffer, offset, i -
offset);
+ } else {
+ break;
+ }
}
}
throw new IOException("Failed to read entry: " + offset);
@@ -311,11 +325,21 @@ public class ArArchiveInputStream extends
ArchiveInputStream<ArArchiveEntry> {
return getNextArEntry();
}
- long len = asLong(metaData, LENGTH_OFFSET, LENGTH_LEN);
+ long len;
+ try {
+ len = asLong(metaData, LENGTH_OFFSET, LENGTH_LEN);
+ } catch (NumberFormatException ex) {
+ throw new IOException("Broken archive, unable to parse ar_size
field as a number", ex);
+ }
if (temp.endsWith("/")) { // GNU terminator
temp = temp.substring(0, temp.length() - 1);
} else if (isGNULongName(temp)) {
- final int off = Integer.parseInt(temp.substring(1));// get the
offset
+ int off;
+ try {
+ off = Integer.parseInt(temp.substring(1));// get the offset
+ } catch (NumberFormatException ex) {
+ throw new IOException("Broken archive, unable to parse GNU
long name offset field as a number", ex);
+ }
temp = getExtendedName(off); // convert to the long name
} else if (isBSDLongName(temp)) {
temp = getBSDLongName(temp);
@@ -331,12 +355,16 @@ public class ArArchiveInputStream extends
ArchiveInputStream<ArArchiveEntry> {
throw new IOException("broken archive, entry with negative size");
}
- currentEntry = new ArArchiveEntry(temp, len,
- asInt(metaData, USER_ID_OFFSET,
USER_ID_LEN, true),
- asInt(metaData, GROUP_ID_OFFSET,
GROUP_ID_LEN, true),
- asInt(metaData, FILE_MODE_OFFSET,
FILE_MODE_LEN, 8),
- asLong(metaData,
LAST_MODIFIED_OFFSET, LAST_MODIFIED_LEN));
- return currentEntry;
+ try {
+ currentEntry = new ArArchiveEntry(temp, len,
+ asInt(metaData, USER_ID_OFFSET, USER_ID_LEN, true),
+ asInt(metaData, GROUP_ID_OFFSET, GROUP_ID_LEN, true),
+ asInt(metaData, FILE_MODE_OFFSET, FILE_MODE_LEN, 8),
+ asLong(metaData, LAST_MODIFIED_OFFSET, LAST_MODIFIED_LEN));
+ return currentEntry;
+ } catch (final NumberFormatException ex) {
+ throw new IOException("Broken archive, unable to parse entry
metadata fields as numbers", ex);
+ }
}
/*
@@ -389,7 +417,13 @@ public class ArArchiveInputStream extends
ArchiveInputStream<ArArchiveEntry> {
* @see #isGNUStringTable
*/
private ArArchiveEntry readGNUStringTable(final byte[] length, final int
offset, final int len) throws IOException {
- final int bufflen = asInt(length, offset, len); // Assume length will
fit in an int
+ int bufflen;
+ try {
+ bufflen = asInt(length, offset, len); // Assume length will fit in
an int
+ } catch (NumberFormatException ex) {
+ throw new IOException("Broken archive, unable to parse GNU string
table length field as a number", ex);
+ }
+
namebuffer = IOUtils.readRange(input, bufflen);
final int read = namebuffer.length;
trackReadBytes(read);
diff --git
a/src/test/java/org/apache/commons/compress/archivers/ar/ArArchiveInputStreamTest.java
b/src/test/java/org/apache/commons/compress/archivers/ar/ArArchiveInputStreamTest.java
index ae86a45c1..2b2bd7d3e 100644
---
a/src/test/java/org/apache/commons/compress/archivers/ar/ArArchiveInputStreamTest.java
+++
b/src/test/java/org/apache/commons/compress/archivers/ar/ArArchiveInputStreamTest.java
@@ -34,6 +34,8 @@ import org.apache.commons.compress.archivers.ArchiveEntry;
import org.apache.commons.compress.utils.ArchiveUtils;
import org.apache.commons.compress.utils.IOUtils;
import org.junit.jupiter.api.Test;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.ValueSource;
public class ArArchiveInputStreamTest extends AbstractTest {
@@ -134,4 +136,31 @@ public class ArArchiveInputStreamTest extends AbstractTest
{
assertEquals(-1, archive.read());
}
}
+
+ @ParameterizedTest
+ @ValueSource(strings = {"bad_group.ar", "bad_length.ar",
"bad_modified.ar", "bad_user.ar"})
+ public void testInvalidNumericFields(final String testFileName) throws
Exception {
+ try (InputStream in =
newInputStream("org/apache/commons/compress/ar/number_parsing/" + testFileName);
+ ArArchiveInputStream archive = new ArArchiveInputStream(in)) {
+ assertThrows(IOException.class, archive::getNextEntry);
+ }
+ }
+
+ @ParameterizedTest
+ @ValueSource(strings = {"bad_long_namelen_bsd.ar",
"bad_long_namelen_gnu1.ar", "bad_long_namelen_gnu2.ar",
"bad_long_namelen_gnu3.ar", "bad_table_length_gnu.ar"})
+ public void testInvalidLongNameLength(final String testFileName) throws
Exception {
+ try (InputStream in =
newInputStream("org/apache/commons/compress/ar/number_parsing/" + testFileName);
+ ArArchiveInputStream archive = new ArArchiveInputStream(in)) {
+ assertThrows(IOException.class, archive::getNextEntry);
+ }
+ }
+
+ @Test
+ public void testInvalidBadTableLength() throws Exception {
+ try (InputStream in =
newInputStream("org/apache/commons/compress/ar/number_parsing/bad_table_length_gnu.ar");
+ ArArchiveInputStream archive = new ArArchiveInputStream(in)) {
+ assertThrows(IOException.class, archive::getNextEntry);
+ }
+ }
+
}
diff --git
a/src/test/resources/org/apache/commons/compress/ar/number_parsing/bad_group.ar
b/src/test/resources/org/apache/commons/compress/ar/number_parsing/bad_group.ar
new file mode 100644
index 000000000..1cffcf1b3
--- /dev/null
+++
b/src/test/resources/org/apache/commons/compress/ar/number_parsing/bad_group.ar
@@ -0,0 +1,8 @@
+!<arch>
+// 68 `
+this_is_a_long_file_name.txt/
+this_is_a_long_file_name_as_well.txt/
+/0 1454693980 1000 1.23 100664 14 `
+Hello, world!
+/30 1454694016 1000 1000 100664 4 `
+Bye
diff --git
a/src/test/resources/org/apache/commons/compress/ar/number_parsing/bad_length.ar
b/src/test/resources/org/apache/commons/compress/ar/number_parsing/bad_length.ar
new file mode 100644
index 000000000..259991876
--- /dev/null
+++
b/src/test/resources/org/apache/commons/compress/ar/number_parsing/bad_length.ar
@@ -0,0 +1,8 @@
+!<arch>
+// 68 `
+this_is_a_long_file_name.txt/
+this_is_a_long_file_name_as_well.txt/
+/0 1454693980 1000 1000 100664 1.23 `
+Hello, world!
+/30 1454694016 1000 1000 100664 4 `
+Bye
diff --git
a/src/test/resources/org/apache/commons/compress/ar/number_parsing/bad_long_namelen_bsd.ar
b/src/test/resources/org/apache/commons/compress/ar/number_parsing/bad_long_namelen_bsd.ar
new file mode 100644
index 000000000..cd0f3b334
--- /dev/null
+++
b/src/test/resources/org/apache/commons/compress/ar/number_parsing/bad_long_namelen_bsd.ar
@@ -0,0 +1,5 @@
+!<arch>
+#1/123456789012 1311256511 1000 1000 100644 42 `
+this_is_a_long_file_name.txtHello, world!
+#1/36 1454694016 1000 1000 100664 40 `
+this_is_a_long_file_name_as_well.txtBye
diff --git
a/src/test/resources/org/apache/commons/compress/ar/number_parsing/bad_long_namelen_gnu1.ar
b/src/test/resources/org/apache/commons/compress/ar/number_parsing/bad_long_namelen_gnu1.ar
new file mode 100644
index 000000000..5779e549e
--- /dev/null
+++
b/src/test/resources/org/apache/commons/compress/ar/number_parsing/bad_long_namelen_gnu1.ar
@@ -0,0 +1,8 @@
+!<arch>
+// 68 `
+this_is_a_long_file_name.txt/
+this_is_a_long_file_name_as_well.txt/
+/9999999999 1454693980 1000 1000 100664 14 `
+Hello, world!
+/30 1454694016 1000 1000 100664 4 `
+Bye
diff --git
a/src/test/resources/org/apache/commons/compress/ar/number_parsing/bad_long_namelen_gnu2.ar
b/src/test/resources/org/apache/commons/compress/ar/number_parsing/bad_long_namelen_gnu2.ar
new file mode 100644
index 000000000..51e998d1f
--- /dev/null
+++
b/src/test/resources/org/apache/commons/compress/ar/number_parsing/bad_long_namelen_gnu2.ar
@@ -0,0 +1,6 @@
+!<arch>
+// 68 `
+this_is_a_long_file_name.txt/
+this_is_a_long_file_name_as_well.txt/
+/29 1454694016 1000 1000 100664 4 `
+Bye
diff --git
a/src/test/resources/org/apache/commons/compress/ar/number_parsing/bad_long_namelen_gnu3.ar
b/src/test/resources/org/apache/commons/compress/ar/number_parsing/bad_long_namelen_gnu3.ar
new file mode 100644
index 000000000..81a31e6db
Binary files /dev/null and
b/src/test/resources/org/apache/commons/compress/ar/number_parsing/bad_long_namelen_gnu3.ar
differ
diff --git
a/src/test/resources/org/apache/commons/compress/ar/number_parsing/bad_modified.ar
b/src/test/resources/org/apache/commons/compress/ar/number_parsing/bad_modified.ar
new file mode 100644
index 000000000..f4e12bbcd
--- /dev/null
+++
b/src/test/resources/org/apache/commons/compress/ar/number_parsing/bad_modified.ar
@@ -0,0 +1,8 @@
+!<arch>
+// 68 `
+this_is_a_long_file_name.txt/
+this_is_a_long_file_name_as_well.txt/
+/0 9e99999999 1000 1000 100664 14 `
+Hello, world!
+/30 1454694016 1000 1000 100664 4 `
+Bye
diff --git
a/src/test/resources/org/apache/commons/compress/ar/number_parsing/bad_table_length_gnu.ar
b/src/test/resources/org/apache/commons/compress/ar/number_parsing/bad_table_length_gnu.ar
new file mode 100644
index 000000000..9fd61f458
--- /dev/null
+++
b/src/test/resources/org/apache/commons/compress/ar/number_parsing/bad_table_length_gnu.ar
@@ -0,0 +1,8 @@
+!<arch>
+// 1.23 `
+this_is_a_long_file_name.txt/
+this_is_a_long_file_name_as_well.txt/
+/0 1454693980 1000 1000 100664 14 `
+Hello, world!
+/30 1454694016 1000 1000 100664 4 `
+Bye
diff --git
a/src/test/resources/org/apache/commons/compress/ar/number_parsing/bad_user.ar
b/src/test/resources/org/apache/commons/compress/ar/number_parsing/bad_user.ar
new file mode 100644
index 000000000..fc46fed87
--- /dev/null
+++
b/src/test/resources/org/apache/commons/compress/ar/number_parsing/bad_user.ar
@@ -0,0 +1,8 @@
+!<arch>
+// 68 `
+this_is_a_long_file_name.txt/
+this_is_a_long_file_name_as_well.txt/
+/0 1454693980 9e99 1000 100664 14 `
+Hello, world!
+/30 1454694016 1000 1000 100664 4 `
+Bye
