You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@commons.apache.org by bo...@apache.org on 2019/08/11 15:50:53 UTC
[commons-compress] 03/03: make varius places where bad data can
cause RuntimeExceptions throw checked exceptions
This is an automated email from the ASF dual-hosted git repository.
bodewig pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/commons-compress.git
commit 40b1d2fb8ff2eb4374eda8169524fade205a5e6c
Author: Stefan Bodewig <bo...@apache.org>
AuthorDate: Sun Aug 11 17:49:08 2019 +0200
make varius places where bad data can cause RuntimeExceptions throw checked exceptions
inspired by COMPRESS-490
---
.../archivers/cpio/CpioArchiveInputStream.java | 26 +++++++-
.../compress/archivers/dump/TapeInputStream.java | 6 ++
.../archivers/sevenz/AES256SHA256Decoder.java | 6 ++
.../BoundedSeekableByteChannelInputStream.java | 2 +-
.../compress/archivers/sevenz/LZMA2Decoder.java | 15 +++--
.../compress/archivers/sevenz/LZMADecoder.java | 12 ++++
.../compress/archivers/sevenz/SevenZFile.java | 43 +++++++++----
.../compress/archivers/zip/AsiExtraField.java | 3 +
.../commons/compress/archivers/zip/BinaryTree.java | 13 +++-
.../commons/compress/archivers/zip/BitStream.java | 3 +
.../archivers/zip/ExplodingInputStream.java | 12 +++-
.../compress/archivers/zip/PKWareExtraHeader.java | 15 ++++-
.../archivers/zip/ResourceAlignmentExtraField.java | 3 +
.../archivers/zip/X0015_CertificateIdForFile.java | 6 +-
.../X0016_CertificateIdForCentralDirectory.java | 7 ++-
.../zip/X0017_StrongEncryptionHeader.java | 70 ++++++++++++++++++----
.../archivers/zip/X5455_ExtendedTimestamp.java | 8 +--
.../compress/archivers/zip/X7875_NewUnix.java | 19 ++++--
.../compress/compressors/lzw/LZWInputStream.java | 10 ++++
.../archivers/zip/ExtraFieldUtilsTest.java | 63 +++++++++++++++----
20 files changed, 283 insertions(+), 59 deletions(-)
diff --git a/src/main/java/org/apache/commons/compress/archivers/cpio/CpioArchiveInputStream.java b/src/main/java/org/apache/commons/compress/archivers/cpio/CpioArchiveInputStream.java
index 68625dc..da20cf8 100644
--- a/src/main/java/org/apache/commons/compress/archivers/cpio/CpioArchiveInputStream.java
+++ b/src/main/java/org/apache/commons/compress/archivers/cpio/CpioArchiveInputStream.java
@@ -148,10 +148,14 @@ public class CpioArchiveInputStream extends ArchiveInputStream implements
* @param encoding
* The encoding of file names to expect - use null for
* the platform's default.
+ * @throws IllegalArgumentException if <code>blockSize</code> is not bigger than 0
* @since 1.6
*/
public CpioArchiveInputStream(final InputStream in, final int blockSize, final String encoding) {
this.in = in;
+ if (blockSize <= 0) {
+ throw new IllegalArgumentException("blockSize must be bigger than 0");
+ }
this.blockSize = blockSize;
this.encoding = encoding;
this.zipEncoding = ZipEncodingHelper.getZipEncoding(encoding);
@@ -382,11 +386,17 @@ public class CpioArchiveInputStream extends ArchiveInputStream implements
ret.setNumberOfLinks(readAsciiLong(8, 16));
ret.setTime(readAsciiLong(8, 16));
ret.setSize(readAsciiLong(8, 16));
+ if (ret.getSize() < 0) {
+ throw new IOException("Found illegal entry with negative length");
+ }
ret.setDeviceMaj(readAsciiLong(8, 16));
ret.setDeviceMin(readAsciiLong(8, 16));
ret.setRemoteDeviceMaj(readAsciiLong(8, 16));
ret.setRemoteDeviceMin(readAsciiLong(8, 16));
final long namesize = readAsciiLong(8, 16);
+ if (namesize < 0) {
+ throw new IOException("Found illegal entry with negative name length");
+ }
ret.setChksum(readAsciiLong(8, 16));
final String name = readCString((int) namesize);
ret.setName(name);
@@ -415,7 +425,13 @@ public class CpioArchiveInputStream extends ArchiveInputStream implements
ret.setRemoteDevice(readAsciiLong(6, 8));
ret.setTime(readAsciiLong(11, 8));
final long namesize = readAsciiLong(6, 8);
+ if (namesize < 0) {
+ throw new IOException("Found illegal entry with negative name length");
+ }
ret.setSize(readAsciiLong(11, 8));
+ if (ret.getSize() < 0) {
+ throw new IOException("Found illegal entry with negative length");
+ }
final String name = readCString((int) namesize);
ret.setName(name);
if (CpioUtil.fileType(mode) == 0 && !name.equals(CPIO_TRAILER)){
@@ -443,7 +459,13 @@ public class CpioArchiveInputStream extends ArchiveInputStream implements
ret.setRemoteDevice(readBinaryLong(2, swapHalfWord));
ret.setTime(readBinaryLong(4, swapHalfWord));
final long namesize = readBinaryLong(2, swapHalfWord);
+ if (namesize < 0) {
+ throw new IOException("Found illegal entry with negative name length");
+ }
ret.setSize(readBinaryLong(4, swapHalfWord));
+ if (ret.getSize() < 0) {
+ throw new IOException("Found illegal entry with negative length");
+ }
final String name = readCString((int) namesize);
ret.setName(name);
if (CpioUtil.fileType(mode) == 0 && !name.equals(CPIO_TRAILER)){
@@ -460,7 +482,9 @@ public class CpioArchiveInputStream extends ArchiveInputStream implements
// don't include trailing NUL in file name to decode
final byte tmpBuffer[] = new byte[length - 1];
readFully(tmpBuffer, 0, tmpBuffer.length);
- this.in.read();
+ if (this.in.read() == -1) {
+ throw new EOFException();
+ }
return zipEncoding.decode(tmpBuffer);
}
diff --git a/src/main/java/org/apache/commons/compress/archivers/dump/TapeInputStream.java b/src/main/java/org/apache/commons/compress/archivers/dump/TapeInputStream.java
index 3049e26..471c53e 100644
--- a/src/main/java/org/apache/commons/compress/archivers/dump/TapeInputStream.java
+++ b/src/main/java/org/apache/commons/compress/archivers/dump/TapeInputStream.java
@@ -63,11 +63,17 @@ class TapeInputStream extends FilterInputStream {
* more than one block has been read
* @throws IOException
* there was an error reading additional blocks.
+ * @throws IOException
+ * recsPerBlock is smaller than 1
*/
public void resetBlockSize(final int recsPerBlock, final boolean isCompressed)
throws IOException {
this.isCompressed = isCompressed;
+ if (recsPerBlock < 1) {
+ throw new IOException("Block with " + recsPerBlock
+ + " records found, must be at least 1");
+ }
blockSize = RECORD_SIZE * recsPerBlock;
// save first block in case we need it again
diff --git a/src/main/java/org/apache/commons/compress/archivers/sevenz/AES256SHA256Decoder.java b/src/main/java/org/apache/commons/compress/archivers/sevenz/AES256SHA256Decoder.java
index c752a1b..caa9217 100644
--- a/src/main/java/org/apache/commons/compress/archivers/sevenz/AES256SHA256Decoder.java
+++ b/src/main/java/org/apache/commons/compress/archivers/sevenz/AES256SHA256Decoder.java
@@ -41,6 +41,12 @@ class AES256SHA256Decoder extends CoderBase {
if (isInitialized) {
return cipherInputStream;
}
+ if (coder.properties == null) {
+ throw new IOException("Missing AES256 properties in " + archiveName);
+ }
+ if (coder.properties.length < 2) {
+ throw new IOException("AES256 properties too short in " + archiveName);
+ }
final int byte0 = 0xff & coder.properties[0];
final int numCyclesPower = byte0 & 0x3f;
final int byte1 = 0xff & coder.properties[1];
diff --git a/src/main/java/org/apache/commons/compress/archivers/sevenz/BoundedSeekableByteChannelInputStream.java b/src/main/java/org/apache/commons/compress/archivers/sevenz/BoundedSeekableByteChannelInputStream.java
index 32b3bda..a51afb1 100644
--- a/src/main/java/org/apache/commons/compress/archivers/sevenz/BoundedSeekableByteChannelInputStream.java
+++ b/src/main/java/org/apache/commons/compress/archivers/sevenz/BoundedSeekableByteChannelInputStream.java
@@ -54,7 +54,7 @@ class BoundedSeekableByteChannelInputStream extends InputStream {
@Override
public int read(final byte[] b, final int off, final int len) throws IOException {
- if (bytesRemaining == 0) {
+ if (bytesRemaining <= 0) {
return -1;
}
int bytesToRead = len;
diff --git a/src/main/java/org/apache/commons/compress/archivers/sevenz/LZMA2Decoder.java b/src/main/java/org/apache/commons/compress/archivers/sevenz/LZMA2Decoder.java
index e76f678..f20c861 100644
--- a/src/main/java/org/apache/commons/compress/archivers/sevenz/LZMA2Decoder.java
+++ b/src/main/java/org/apache/commons/compress/archivers/sevenz/LZMA2Decoder.java
@@ -66,7 +66,8 @@ class LZMA2Decoder extends CoderBase {
}
@Override
- Object getOptionsFromCoder(final Coder coder, final InputStream in) {
+ Object getOptionsFromCoder(final Coder coder, final InputStream in)
+ throws IOException {
return getDictionarySize(coder);
}
@@ -77,13 +78,19 @@ class LZMA2Decoder extends CoderBase {
return numberOptionOrDefault(opts);
}
- private int getDictionarySize(final Coder coder) throws IllegalArgumentException {
+ private int getDictionarySize(final Coder coder) throws IOException {
+ if (coder.properties == null) {
+ throw new IOException("Missing LZMA2 properties");
+ }
+ if (coder.properties.length < 1) {
+ throw new IOException("LZMA2 properties too short");
+ }
final int dictionarySizeBits = 0xff & coder.properties[0];
if ((dictionarySizeBits & (~0x3f)) != 0) {
- throw new IllegalArgumentException("Unsupported LZMA2 property bits");
+ throw new IOException("Unsupported LZMA2 property bits");
}
if (dictionarySizeBits > 40) {
- throw new IllegalArgumentException("Dictionary larger than 4GiB maximum size");
+ throw new IOException("Dictionary larger than 4GiB maximum size");
}
if (dictionarySizeBits == 40) {
return 0xFFFFffff;
diff --git a/src/main/java/org/apache/commons/compress/archivers/sevenz/LZMADecoder.java b/src/main/java/org/apache/commons/compress/archivers/sevenz/LZMADecoder.java
index 659a97c..45080b9 100644
--- a/src/main/java/org/apache/commons/compress/archivers/sevenz/LZMADecoder.java
+++ b/src/main/java/org/apache/commons/compress/archivers/sevenz/LZMADecoder.java
@@ -36,6 +36,12 @@ class LZMADecoder extends CoderBase {
@Override
InputStream decode(final String archiveName, final InputStream in, final long uncompressedLength,
final Coder coder, final byte[] password, int maxMemoryLimitInKb) throws IOException {
+ if (coder.properties == null) {
+ throw new IOException("Missing LZMA properties");
+ }
+ if (coder.properties.length < 1) {
+ throw new IOException("LZMA properties too short");
+ }
final byte propsByte = coder.properties[0];
final int dictSize = getDictionarySize(coder);
if (dictSize > LZMAInputStream.DICT_SIZE_MAX) {
@@ -69,6 +75,12 @@ class LZMADecoder extends CoderBase {
@Override
Object getOptionsFromCoder(final Coder coder, final InputStream in) throws IOException {
+ if (coder.properties == null) {
+ throw new IOException("Missing LZMA properties");
+ }
+ if (coder.properties.length < 1) {
+ throw new IOException("LZMA properties too short");
+ }
final byte propsByte = coder.properties[0];
int props = propsByte & 0xFF;
int pb = props / (9 * 5);
diff --git a/src/main/java/org/apache/commons/compress/archivers/sevenz/SevenZFile.java b/src/main/java/org/apache/commons/compress/archivers/sevenz/SevenZFile.java
index 8b56a1f..eba32e3 100644
--- a/src/main/java/org/apache/commons/compress/archivers/sevenz/SevenZFile.java
+++ b/src/main/java/org/apache/commons/compress/archivers/sevenz/SevenZFile.java
@@ -501,11 +501,8 @@ public class SevenZFile implements Closeable {
final long startHeaderCrc = 0xffffFFFFL & buf.getInt();
final StartHeader startHeader = readStartHeader(startHeaderCrc);
-
+ assertFitsIntoInt("nextHeaderSize", startHeader.nextHeaderSize);
final int nextHeaderSizeInt = (int) startHeader.nextHeaderSize;
- if (nextHeaderSizeInt != startHeader.nextHeaderSize) {
- throw new IOException("Cannot handle nextHeaderSize " + startHeader.nextHeaderSize);
- }
channel.position(SIGNATURE_HEADER_SIZE + startHeader.nextHeaderOffset);
buf = ByteBuffer.allocate(nextHeaderSizeInt).order(ByteOrder.LITTLE_ENDIAN);
readFully(buf);
@@ -577,6 +574,7 @@ public class SevenZFile implements Closeable {
int nid = getUnsignedByte(input);
while (nid != NID.kEnd) {
final long propertySize = readUint64(input);
+ assertFitsIntoInt("propertySize", propertySize);
final byte[] property = new byte[(int)propertySize];
input.get(property);
nid = getUnsignedByte(input);
@@ -607,6 +605,7 @@ public class SevenZFile implements Closeable {
inputStreamStack = new CRC32VerifyingInputStream(inputStreamStack,
folder.getUnpackSize(), folder.crc);
}
+ assertFitsIntoInt("unpackSize", folder.getUnpackSize());
final byte[] nextHeader = new byte[(int)folder.getUnpackSize()];
try (DataInputStream nextHeaderInputStream = new DataInputStream(inputStreamStack)) {
nextHeaderInputStream.readFully(nextHeader);
@@ -643,9 +642,11 @@ public class SevenZFile implements Closeable {
private void readPackInfo(final ByteBuffer header, final Archive archive) throws IOException {
archive.packPos = readUint64(header);
final long numPackStreams = readUint64(header);
+ assertFitsIntoInt("numPackStreams", numPackStreams);
+ final int numPackStreamsInt = (int) numPackStreams;
int nid = getUnsignedByte(header);
if (nid == NID.kSize) {
- archive.packSizes = new long[(int)numPackStreams];
+ archive.packSizes = new long[numPackStreamsInt];
for (int i = 0; i < archive.packSizes.length; i++) {
archive.packSizes[i] = readUint64(header);
}
@@ -653,9 +654,9 @@ public class SevenZFile implements Closeable {
}
if (nid == NID.kCRC) {
- archive.packCrcsDefined = readAllOrBits(header, (int)numPackStreams);
- archive.packCrcs = new long[(int)numPackStreams];
- for (int i = 0; i < (int)numPackStreams; i++) {
+ archive.packCrcsDefined = readAllOrBits(header, numPackStreamsInt);
+ archive.packCrcs = new long[numPackStreamsInt];
+ for (int i = 0; i < numPackStreamsInt; i++) {
if (archive.packCrcsDefined.get(i)) {
archive.packCrcs[i] = 0xffffFFFFL & header.getInt();
}
@@ -675,13 +676,15 @@ public class SevenZFile implements Closeable {
throw new IOException("Expected kFolder, got " + nid);
}
final long numFolders = readUint64(header);
- final Folder[] folders = new Folder[(int)numFolders];
+ assertFitsIntoInt("numFolders", numFolders);
+ final int numFoldersInt = (int) numFolders;
+ final Folder[] folders = new Folder[numFoldersInt];
archive.folders = folders;
final int external = getUnsignedByte(header);
if (external != 0) {
throw new IOException("External unsupported");
}
- for (int i = 0; i < (int)numFolders; i++) {
+ for (int i = 0; i < numFoldersInt; i++) {
folders[i] = readFolder(header);
}
@@ -690,6 +693,7 @@ public class SevenZFile implements Closeable {
throw new IOException("Expected kCodersUnpackSize, got " + nid);
}
for (final Folder folder : folders) {
+ assertFitsIntoInt("totalOutputStreams", folder.totalOutputStreams);
folder.unpackSizes = new long[(int)folder.totalOutputStreams];
for (int i = 0; i < folder.totalOutputStreams; i++) {
folder.unpackSizes[i] = readUint64(header);
@@ -698,8 +702,8 @@ public class SevenZFile implements Closeable {
nid = getUnsignedByte(header);
if (nid == NID.kCRC) {
- final BitSet crcsDefined = readAllOrBits(header, (int)numFolders);
- for (int i = 0; i < (int)numFolders; i++) {
+ final BitSet crcsDefined = readAllOrBits(header, numFoldersInt);
+ for (int i = 0; i < numFoldersInt; i++) {
if (crcsDefined.get(i)) {
folders[i].hasCrc = true;
folders[i].crc = 0xffffFFFFL & header.getInt();
@@ -727,6 +731,7 @@ public class SevenZFile implements Closeable {
totalUnpackStreams = 0;
for (final Folder folder : archive.folders) {
final long numStreams = readUint64(header);
+ assertFitsIntoInt("numStreams", numStreams);
folder.numUnpackSubStreams = (int)numStreams;
totalUnpackStreams += numStreams;
}
@@ -803,6 +808,7 @@ public class SevenZFile implements Closeable {
final Folder folder = new Folder();
final long numCoders = readUint64(header);
+ assertFitsIntoInt("numCoders", numCoders);
final Coder[] coders = new Coder[(int)numCoders];
long totalInStreams = 0;
long totalOutStreams = 0;
@@ -827,6 +833,7 @@ public class SevenZFile implements Closeable {
totalOutStreams += coders[i].numOutStreams;
if (hasAttributes) {
final long propertiesSize = readUint64(header);
+ assertFitsIntoInt("propertiesSize", propertiesSize);
coders[i].properties = new byte[(int)propertiesSize];
header.get(coders[i].properties);
}
@@ -837,13 +844,16 @@ public class SevenZFile implements Closeable {
}
}
folder.coders = coders;
+ assertFitsIntoInt("totalInStreams", totalInStreams);
folder.totalInputStreams = totalInStreams;
+ assertFitsIntoInt("totalOutStreams", totalOutStreams);
folder.totalOutputStreams = totalOutStreams;
if (totalOutStreams == 0) {
throw new IOException("Total output streams can't be 0");
}
final long numBindPairs = totalOutStreams - 1;
+ assertFitsIntoInt("numBindPairs", numBindPairs);
final BindPair[] bindPairs = new BindPair[(int)numBindPairs];
for (int i = 0; i < bindPairs.length; i++) {
bindPairs[i] = new BindPair();
@@ -856,6 +866,7 @@ public class SevenZFile implements Closeable {
throw new IOException("Total input streams can't be less than the number of bind pairs");
}
final long numPackedStreams = totalInStreams - numBindPairs;
+ assertFitsIntoInt("numPackedStreams", numPackedStreams);
final long packedStreams[] = new long[(int)numPackedStreams];
if (numPackedStreams == 1) {
int i;
@@ -909,6 +920,7 @@ public class SevenZFile implements Closeable {
private void readFilesInfo(final ByteBuffer header, final Archive archive) throws IOException {
final long numFiles = readUint64(header);
+ assertFitsIntoInt("numFiles", numFiles);
final SevenZArchiveEntry[] files = new SevenZArchiveEntry[(int)numFiles];
for (int i = 0; i < files.length; i++) {
files[i] = new SevenZArchiveEntry();
@@ -949,6 +961,7 @@ public class SevenZFile implements Closeable {
if (((size - 1) & 1) != 0) {
throw new IOException("File names length invalid");
}
+ assertFitsIntoInt("file names length", size - 1);
final byte[] names = new byte[(int)(size - 1)];
header.get(names);
int nextFile = 0;
@@ -1374,4 +1387,10 @@ public class SevenZFile implements Closeable {
encoded.get(e);
return e;
}
+
+ private static void assertFitsIntoInt(String what, long value) throws IOException {
+ if (value > Integer.MAX_VALUE) {
+ throw new IOException("Cannot handle " + what + value);
+ }
+ }
}
diff --git a/src/main/java/org/apache/commons/compress/archivers/zip/AsiExtraField.java b/src/main/java/org/apache/commons/compress/archivers/zip/AsiExtraField.java
index d5dac8e..d2ed167 100644
--- a/src/main/java/org/apache/commons/compress/archivers/zip/AsiExtraField.java
+++ b/src/main/java/org/apache/commons/compress/archivers/zip/AsiExtraField.java
@@ -288,6 +288,9 @@ public class AsiExtraField implements ZipExtraField, UnixStat, Cloneable {
if (linkArray.length == 0) {
link = "";
+ } else if (linkArray.length > tmp.length - 10) {
+ throw new ZipException("Bad symbolic link name length " + linkArray.length
+ + " in ASI extra field");
} else {
System.arraycopy(tmp, 10, linkArray, 0, linkArray.length);
link = new String(linkArray); // Uses default charset - see class Javadoc
diff --git a/src/main/java/org/apache/commons/compress/archivers/zip/BinaryTree.java b/src/main/java/org/apache/commons/compress/archivers/zip/BinaryTree.java
index 3ff1428..e742175 100644
--- a/src/main/java/org/apache/commons/compress/archivers/zip/BinaryTree.java
+++ b/src/main/java/org/apache/commons/compress/archivers/zip/BinaryTree.java
@@ -47,7 +47,11 @@ class BinaryTree {
private final int[] tree;
public BinaryTree(final int depth) {
- tree = new int[(1 << (depth + 1)) - 1];
+ if (depth < 0 || depth > 30) {
+ throw new IllegalArgumentException("depth must be bigger than 0 and not bigger than 30"
+ + " but is " + depth);
+ }
+ tree = new int[(int) ((1l << (depth + 1)) - 1)];
Arrays.fill(tree, UNDEFINED);
}
@@ -110,6 +114,10 @@ class BinaryTree {
* Decodes the packed binary tree from the specified stream.
*/
static BinaryTree decode(final InputStream in, final int totalNumberOfValues) throws IOException {
+ if (totalNumberOfValues < 0) {
+ throw new IllegalArgumentException("totalNumberOfValues must be bigger than 0, is "
+ + totalNumberOfValues);
+ }
// the first byte contains the size of the structure minus one
final int size = in.read() + 1;
if (size == 0) {
@@ -130,6 +138,9 @@ class BinaryTree {
for (final byte b : encodedTree) {
// each byte encodes the number of values (upper 4 bits) for a bit length (lower 4 bits)
final int numberOfValues = ((b & 0xF0) >> 4) + 1;
+ if (pos + numberOfValues > totalNumberOfValues) {
+ throw new IOException("Number of values exceeds given total number of values");
+ }
final int bitLength = (b & 0x0F) + 1;
for (int j = 0; j < numberOfValues; j++) {
diff --git a/src/main/java/org/apache/commons/compress/archivers/zip/BitStream.java b/src/main/java/org/apache/commons/compress/archivers/zip/BitStream.java
index fb737b7..904f153 100644
--- a/src/main/java/org/apache/commons/compress/archivers/zip/BitStream.java
+++ b/src/main/java/org/apache/commons/compress/archivers/zip/BitStream.java
@@ -52,6 +52,9 @@ class BitStream extends BitInputStream {
* @return The value formed by the n bits, or -1 if the end of the stream has been reached
*/
long nextBits(final int n) throws IOException {
+ if (n < 0 || n > 8) {
+ throw new IOException("Trying to read " + n + " bits, at most 8 are allowed");
+ }
return readBits(n);
}
diff --git a/src/main/java/org/apache/commons/compress/archivers/zip/ExplodingInputStream.java b/src/main/java/org/apache/commons/compress/archivers/zip/ExplodingInputStream.java
index b3e1ca6..0c899f9 100644
--- a/src/main/java/org/apache/commons/compress/archivers/zip/ExplodingInputStream.java
+++ b/src/main/java/org/apache/commons/compress/archivers/zip/ExplodingInputStream.java
@@ -161,7 +161,10 @@ class ExplodingInputStream extends InputStream implements InputStreamStatistics
init();
final int bit = bits.nextBit();
- if (bit == 1) {
+ if (bit == -1) {
+ // EOF
+ return;
+ } else if (bit == 1) {
// literal value
int literal;
if (literalTree != null) {
@@ -190,7 +193,12 @@ class ExplodingInputStream extends InputStream implements InputStreamStatistics
int length = lengthTree.read(bits);
if (length == 63) {
- length += bits.nextBits(8);
+ final long nextByte = bits.nextBits(8);
+ if (nextByte == -1) {
+ // EOF
+ return;
+ }
+ length += nextByte;
}
length += minimumMatchLength;
diff --git a/src/main/java/org/apache/commons/compress/archivers/zip/PKWareExtraHeader.java b/src/main/java/org/apache/commons/compress/archivers/zip/PKWareExtraHeader.java
index a523ad2..6828fff 100644
--- a/src/main/java/org/apache/commons/compress/archivers/zip/PKWareExtraHeader.java
+++ b/src/main/java/org/apache/commons/compress/archivers/zip/PKWareExtraHeader.java
@@ -22,6 +22,7 @@ import java.util.Arrays;
import java.util.Collections;
import java.util.HashMap;
import java.util.Map;
+import java.util.zip.ZipException;
/**
* Base class for all PKWare strong crypto extra headers.
@@ -170,7 +171,8 @@ public abstract class PKWareExtraHeader implements ZipExtraField {
* @see ZipExtraField#parseFromLocalFileData(byte[], int, int)
*/
@Override
- public void parseFromLocalFileData(final byte[] data, final int offset, final int length) {
+ public void parseFromLocalFileData(final byte[] data, final int offset, final int length)
+ throws ZipException {
setLocalFileDataData(Arrays.copyOfRange(data, offset, offset + length));
}
@@ -184,7 +186,8 @@ public abstract class PKWareExtraHeader implements ZipExtraField {
* @see ZipExtraField#parseFromCentralDirectoryData(byte[], int, int)
*/
@Override
- public void parseFromCentralDirectoryData(final byte[] data, final int offset, final int length) {
+ public void parseFromCentralDirectoryData(final byte[] data, final int offset, final int length)
+ throws ZipException {
final byte[] tmp = Arrays.copyOfRange(data, offset, offset + length);
setCentralDirectoryData(tmp);
if (localData == null) {
@@ -192,6 +195,14 @@ public abstract class PKWareExtraHeader implements ZipExtraField {
}
}
+ protected final void assertMinimalLength(final int minimum, final int length)
+ throws ZipException {
+ if (length < minimum) {
+ throw new ZipException(getClass().getName() + " is too short, only "
+ + length + " bytes, expected at least " + minimum);
+ }
+ }
+
/**
* Encryption algorithm.
*
diff --git a/src/main/java/org/apache/commons/compress/archivers/zip/ResourceAlignmentExtraField.java b/src/main/java/org/apache/commons/compress/archivers/zip/ResourceAlignmentExtraField.java
index 3d0741c..bc31147 100644
--- a/src/main/java/org/apache/commons/compress/archivers/zip/ResourceAlignmentExtraField.java
+++ b/src/main/java/org/apache/commons/compress/archivers/zip/ResourceAlignmentExtraField.java
@@ -67,6 +67,9 @@ public class ResourceAlignmentExtraField implements ZipExtraField {
if (alignment < 0 || alignment > 0x7fff) {
throw new IllegalArgumentException("Alignment must be between 0 and 0x7fff, was: " + alignment);
}
+ if (padding < 0) {
+ throw new IllegalArgumentException("Padding must not be negative, was: " + padding);
+ }
this.alignment = (short) alignment;
this.allowMethodChange = allowMethodChange;
this.padding = padding;
diff --git a/src/main/java/org/apache/commons/compress/archivers/zip/X0015_CertificateIdForFile.java b/src/main/java/org/apache/commons/compress/archivers/zip/X0015_CertificateIdForFile.java
index 89b327b..d3dd30b 100644
--- a/src/main/java/org/apache/commons/compress/archivers/zip/X0015_CertificateIdForFile.java
+++ b/src/main/java/org/apache/commons/compress/archivers/zip/X0015_CertificateIdForFile.java
@@ -18,6 +18,8 @@
*/
package org.apache.commons.compress.archivers.zip;
+import java.util.zip.ZipException;
+
/**
* X.509 Certificate ID and Signature for individual file (0x0015).
*
@@ -67,7 +69,9 @@ public class X0015_CertificateIdForFile extends PKWareExtraHeader {
}
@Override
- public void parseFromCentralDirectoryData(final byte[] data, final int offset, final int length) {
+ public void parseFromCentralDirectoryData(final byte[] data, final int offset, final int length)
+ throws ZipException {
+ assertMinimalLength(4, length);
super.parseFromCentralDirectoryData(data, offset, length);
this.rcount = ZipShort.getValue(data, offset);
this.hashAlg = HashAlgorithm.getAlgorithmByCode(ZipShort.getValue(data, offset + 2));
diff --git a/src/main/java/org/apache/commons/compress/archivers/zip/X0016_CertificateIdForCentralDirectory.java b/src/main/java/org/apache/commons/compress/archivers/zip/X0016_CertificateIdForCentralDirectory.java
index bab1e61..b674053 100644
--- a/src/main/java/org/apache/commons/compress/archivers/zip/X0016_CertificateIdForCentralDirectory.java
+++ b/src/main/java/org/apache/commons/compress/archivers/zip/X0016_CertificateIdForCentralDirectory.java
@@ -18,6 +18,8 @@
*/
package org.apache.commons.compress.archivers.zip;
+import java.util.zip.ZipException;
+
/**
* X.509 Certificate ID and Signature for central directory (0x0016).
*
@@ -68,7 +70,10 @@ public class X0016_CertificateIdForCentralDirectory extends PKWareExtraHeader {
}
@Override
- public void parseFromCentralDirectoryData(final byte[] data, final int offset, final int length) {
+ public void parseFromCentralDirectoryData(final byte[] data, final int offset, final int length)
+ throws ZipException {
+ assertMinimalLength(4, length);
+ // TODO: double check we really do not want to call super here
this.rcount = ZipShort.getValue(data, offset);
this.hashAlg = HashAlgorithm.getAlgorithmByCode(ZipShort.getValue(data, offset + 2));
}
diff --git a/src/main/java/org/apache/commons/compress/archivers/zip/X0017_StrongEncryptionHeader.java b/src/main/java/org/apache/commons/compress/archivers/zip/X0017_StrongEncryptionHeader.java
index acc3b22..d3669e9 100644
--- a/src/main/java/org/apache/commons/compress/archivers/zip/X0017_StrongEncryptionHeader.java
+++ b/src/main/java/org/apache/commons/compress/archivers/zip/X0017_StrongEncryptionHeader.java
@@ -18,6 +18,9 @@
*/
package org.apache.commons.compress.archivers.zip;
+import java.util.Arrays;
+import java.util.zip.ZipException;
+
/**
* Strong Encryption Header (0x0017).
*
@@ -299,7 +302,10 @@ public class X0017_StrongEncryptionHeader extends PKWareExtraHeader {
* @param offset offset into buffer to read data
* @param length the length of data
*/
- public void parseCentralDirectoryFormat(final byte[] data, final int offset, final int length) {
+ public void parseCentralDirectoryFormat(final byte[] data, final int offset, final int length)
+ throws ZipException {
+ assertMinimalLength(12, length);
+ // TODO: double check we really do not want to call super here
this.format = ZipShort.getValue(data, offset);
this.algId = EncryptionAlgorithm.getAlgorithmByCode(ZipShort.getValue(data, offset + 2));
this.bitlen = ZipShort.getValue(data, offset + 4);
@@ -307,6 +313,7 @@ public class X0017_StrongEncryptionHeader extends PKWareExtraHeader {
this.rcount = ZipLong.getValue(data, offset + 8);
if (rcount > 0) {
+ assertMinimalLength(16, length);
this.hashAlg = HashAlgorithm.getAlgorithmByCode(ZipShort.getValue(data, offset + 12));
this.hashSize = ZipShort.getValue(data, offset + 14);
// srlist... hashed public keys
@@ -327,38 +334,64 @@ public class X0017_StrongEncryptionHeader extends PKWareExtraHeader {
* @param offset offset into buffer to read data
* @param length the length of data
*/
- public void parseFileFormat(final byte[] data, final int offset, final int length) {
+ public void parseFileFormat(final byte[] data, final int offset, final int length)
+ throws ZipException {
+ assertMinimalLength(4, length);
final int ivSize = ZipShort.getValue(data, offset);
- this.ivData = new byte[ivSize];
- System.arraycopy(data, offset + 4, this.ivData, 0, ivSize);
+ assertDynamicLengthFits("ivSize", ivSize, 4, length);
+ // TODO: what is at offset + 2?
+ this.ivData = Arrays.copyOfRange(data, offset + 4, ivSize);
+ assertMinimalLength(16 + ivSize, length); // up to and including erdSize
+ // TODO: what is at offset + 4 + ivSize?
this.format = ZipShort.getValue(data, offset + ivSize + 6);
this.algId = EncryptionAlgorithm.getAlgorithmByCode(ZipShort.getValue(data, offset + ivSize + 8));
this.bitlen = ZipShort.getValue(data, offset + ivSize + 10);
this.flags = ZipShort.getValue(data, offset + ivSize + 12);
final int erdSize = ZipShort.getValue(data, offset + ivSize + 14);
- this.erdData = new byte[erdSize];
- System.arraycopy(data, offset + ivSize + 16, this.erdData, 0, erdSize);
+ assertDynamicLengthFits("erdSize", erdSize, ivSize + 16, length);
+ this.erdData = Arrays.copyOfRange(data, offset + ivSize + 16, erdSize);
+ assertMinimalLength(16 + 4 + ivSize + erdSize, length);
this.rcount = ZipLong.getValue(data, offset + ivSize + 16 + erdSize);
- System.out.println("rcount: " + rcount);
if (rcount == 0) {
+ assertMinimalLength(ivSize + 20 + erdSize + 2, length);
final int vSize = ZipShort.getValue(data, offset + ivSize + 20 + erdSize);
- this.vData = new byte[vSize - 4];
- this.vCRC32 = new byte[4];
- System.arraycopy(data, offset + ivSize + 22 + erdSize , this.vData, 0, vSize - 4);
- System.arraycopy(data, offset + ivSize + 22 + erdSize + vSize - 4, vCRC32, 0, 4);
+ assertDynamicLengthFits("vSize", vSize, ivSize + 22 + erdSize, length);
+ if (vSize < 4) {
+ throw new ZipException("Invalid X0017_StrongEncryptionHeader: vSize " + vSize
+ + " is too small to hold CRC");
+ }
+ this.vData = Arrays.copyOfRange(data, offset + ivSize + 22 + erdSize, vSize - 4);
+ this.vCRC32 = Arrays.copyOfRange(data, offset + ivSize + 22 + erdSize + vSize - 4, 4);
} else {
+ assertMinimalLength(ivSize + 20 + erdSize + 6, length); // up to and including resize
this.hashAlg = HashAlgorithm.getAlgorithmByCode(ZipShort.getValue(data, offset + ivSize + 20 + erdSize));
this.hashSize = ZipShort.getValue(data, offset + ivSize + 22 + erdSize);
final int resize = ZipShort.getValue(data, offset + ivSize + 24 + erdSize);
+
this.recipientKeyHash = new byte[this.hashSize];
+ if (resize < this.hashSize) {
+ throw new ZipException("Invalid X0017_StrongEncryptionHeader: resize " + resize
+ + " is too small to hold hashSize" + this.hashSize);
+ }
this.keyBlob = new byte[resize - this.hashSize];
+ // TODO: this looks suspicious, 26 rather than 24 would be "after" resize
+ assertDynamicLengthFits("resize", resize, ivSize + 24 + erdSize, length);
+ // TODO use Arrays.copyOfRange
System.arraycopy(data, offset + ivSize + 24 + erdSize, this.recipientKeyHash, 0, this.hashSize);
System.arraycopy(data, offset + ivSize + 24 + erdSize + this.hashSize, this.keyBlob, 0, resize - this.hashSize);
+ assertMinimalLength(ivSize + 26 + erdSize + resize + 2, length);
final int vSize = ZipShort.getValue(data, offset + ivSize + 26 + erdSize + resize);
+ if (vSize < 4) {
+ throw new ZipException("Invalid X0017_StrongEncryptionHeader: vSize " + vSize
+ + " is too small to hold CRC");
+ }
+ // TODO: these offsets look even more suspicious, the constant should likely be 28 rather than 22
+ assertDynamicLengthFits("vSize", vSize, ivSize + 22 + erdSize + resize, length);
+ // TODO: use Arrays.copyOfRange
this.vData = new byte[vSize - 4];
this.vCRC32 = new byte[4];
System.arraycopy(data, offset + ivSize + 22 + erdSize + resize, this.vData, 0, vSize - 4);
@@ -369,14 +402,25 @@ public class X0017_StrongEncryptionHeader extends PKWareExtraHeader {
}
@Override
- public void parseFromLocalFileData(final byte[] data, final int offset, final int length) {
+ public void parseFromLocalFileData(final byte[] data, final int offset, final int length)
+ throws ZipException {
super.parseFromLocalFileData(data, offset, length);
parseFileFormat(data, offset, length);
}
@Override
- public void parseFromCentralDirectoryData(final byte[] data, final int offset, final int length) {
+ public void parseFromCentralDirectoryData(final byte[] data, final int offset, final int length)
+ throws ZipException {
super.parseFromCentralDirectoryData(data, offset, length);
parseCentralDirectoryFormat(data, offset, length);
}
+
+ private void assertDynamicLengthFits(final String what, final int dynamicLength, final int prefixLength,
+ final int length) throws ZipException {
+ if (prefixLength + dynamicLength > length) {
+ throw new ZipException("Invalid X0017_StrongEncryptionHeader: " + what + " "
+ + dynamicLength + " doesn't fit into " + length + " bytes of data at position "
+ + prefixLength);
+ }
+ }
}
diff --git a/src/main/java/org/apache/commons/compress/archivers/zip/X5455_ExtendedTimestamp.java b/src/main/java/org/apache/commons/compress/archivers/zip/X5455_ExtendedTimestamp.java
index ac3fada..acec5b0 100644
--- a/src/main/java/org/apache/commons/compress/archivers/zip/X5455_ExtendedTimestamp.java
+++ b/src/main/java/org/apache/commons/compress/archivers/zip/X5455_ExtendedTimestamp.java
@@ -219,15 +219,15 @@ public class X5455_ExtendedTimestamp implements ZipExtraField, Cloneable, Serial
final byte[] data, int offset, final int length
) throws ZipException {
reset();
+ if (length < 1) {
+ throw new ZipException("X5455_ExtendedTimestamp too short, only " + length + " bytes");
+ }
final int len = offset + length;
setFlags(data[offset++]);
- if (bit0_modifyTimePresent) {
+ if (bit0_modifyTimePresent && offset + 4 <= len) {
modifyTime = new ZipLong(data, offset);
offset += 4;
}
-
- // Notice the extra length check in case we are parsing the shorter
- // central data field (for both access and create timestamps).
if (bit1_accessTimePresent && offset + 4 <= len) {
accessTime = new ZipLong(data, offset);
offset += 4;
diff --git a/src/main/java/org/apache/commons/compress/archivers/zip/X7875_NewUnix.java b/src/main/java/org/apache/commons/compress/archivers/zip/X7875_NewUnix.java
index a540dba..3af80a9 100644
--- a/src/main/java/org/apache/commons/compress/archivers/zip/X7875_NewUnix.java
+++ b/src/main/java/org/apache/commons/compress/archivers/zip/X7875_NewUnix.java
@@ -20,6 +20,7 @@ package org.apache.commons.compress.archivers.zip;
import java.io.Serializable;
import java.math.BigInteger;
+import java.util.Arrays;
import java.util.zip.ZipException;
import static org.apache.commons.compress.archivers.zip.ZipUtil.reverse;
@@ -224,16 +225,26 @@ public class X7875_NewUnix implements ZipExtraField, Cloneable, Serializable {
final byte[] data, int offset, final int length
) throws ZipException {
reset();
+ if (length < 3) {
+ throw new ZipException("X7875_NewUnix length is too short, only "
+ + length + " bytes");
+ }
this.version = signedByteToUnsignedInt(data[offset++]);
final int uidSize = signedByteToUnsignedInt(data[offset++]);
- final byte[] uidBytes = new byte[uidSize];
- System.arraycopy(data, offset, uidBytes, 0, uidSize);
+ if (uidSize + 3 > length) {
+ throw new ZipException("X7875_NewUnix invalid: uidSize " + uidSize
+ + " doesn't fit into " + length + " bytes");
+ }
+ final byte[] uidBytes = Arrays.copyOfRange(data, offset, offset + uidSize);
offset += uidSize;
this.uid = new BigInteger(1, reverse(uidBytes)); // sign-bit forced positive
final int gidSize = signedByteToUnsignedInt(data[offset++]);
- final byte[] gidBytes = new byte[gidSize];
- System.arraycopy(data, offset, gidBytes, 0, gidSize);
+ if (uidSize + 3 + gidSize > length) {
+ throw new ZipException("X7875_NewUnix invalid: gidSize " + gidSize
+ + " doesn't fit into " + length + " bytes");
+ }
+ final byte[] gidBytes = Arrays.copyOfRange(data, offset, offset + gidSize);
this.gid = new BigInteger(1, reverse(gidBytes)); // sign-bit forced positive
}
diff --git a/src/main/java/org/apache/commons/compress/compressors/lzw/LZWInputStream.java b/src/main/java/org/apache/commons/compress/compressors/lzw/LZWInputStream.java
index 18b8a48..7c6ab1f 100644
--- a/src/main/java/org/apache/commons/compress/compressors/lzw/LZWInputStream.java
+++ b/src/main/java/org/apache/commons/compress/compressors/lzw/LZWInputStream.java
@@ -128,9 +128,14 @@ public abstract class LZWInputStream extends CompressorInputStream implements In
* @param maxCodeSize maximum code size
* @param memoryLimitInKb maximum allowed estimated memory usage in Kb
* @throws MemoryLimitException if estimated memory usage is greater than memoryLimitInKb
+ * @throws IllegalArgumentException if <code>maxCodeSize</code> is not bigger than 0
*/
protected void initializeTables(final int maxCodeSize, final int memoryLimitInKb)
throws MemoryLimitException {
+ if (maxCodeSize <= 0) {
+ throw new IllegalArgumentException("maxCodeSize is " + maxCodeSize
+ + ", must be bigger than 0");
+ }
if (memoryLimitInKb > -1) {
final int maxTableSize = 1 << maxCodeSize;
@@ -148,8 +153,13 @@ public abstract class LZWInputStream extends CompressorInputStream implements In
/**
* Initializes the arrays based on the maximum code size.
* @param maxCodeSize maximum code size
+ * @throws IllegalArgumentException if <code>maxCodeSize</code> is not bigger than 0
*/
protected void initializeTables(final int maxCodeSize) {
+ if (maxCodeSize <= 0) {
+ throw new IllegalArgumentException("maxCodeSize is " + maxCodeSize
+ + ", must be bigger than 0");
+ }
final int maxTableSize = 1 << maxCodeSize;
prefixes = new int[maxTableSize];
characters = new byte[maxTableSize];
diff --git a/src/test/java/org/apache/commons/compress/archivers/zip/ExtraFieldUtilsTest.java b/src/test/java/org/apache/commons/compress/archivers/zip/ExtraFieldUtilsTest.java
index 7f7c30c..190eccb 100644
--- a/src/test/java/org/apache/commons/compress/archivers/zip/ExtraFieldUtilsTest.java
+++ b/src/test/java/org/apache/commons/compress/archivers/zip/ExtraFieldUtilsTest.java
@@ -39,6 +39,12 @@ public class ExtraFieldUtilsTest implements UnixStat {
*/
static final ZipShort UNRECOGNIZED_HEADER = new ZipShort(0x5555);
+ /**
+ * Header-ID of a ZipExtraField not supported by Commons Compress
+ * used for the ArrayIndexOutOfBoundsTest.
+ */
+ static final ZipShort AIOB_HEADER = new ZipShort(0x1000);
+
private AsiExtraField a;
private UnrecognizedExtraField dummy;
private byte[] data;
@@ -98,26 +104,18 @@ public class ExtraFieldUtilsTest implements UnixStat {
@Test
public void parseTurnsArrayIndexOutOfBoundsIntoZipException() throws Exception {
- AsiExtraField f = new AsiExtraField();
- f.setLinkedFile("foo");
- byte[] l = f.getLocalFileDataData();
- // manipulate size of path name to read 4 rather than 3
- l[9] = 4;
- // and fake CRC so we actually reach the AIOBE
- l[0] = (byte) 0x52;
- l[1] = (byte) 0x26;
- l[2] = (byte) 0x18;
- l[3] = (byte) 0x19;
- byte[] d = new byte[4 + l.length];
+ ExtraFieldUtils.register(AiobThrowingExtraField.class);
+ AiobThrowingExtraField f = new AiobThrowingExtraField();
+ byte[] d = new byte[4 + AiobThrowingExtraField.LENGTH];
System.arraycopy(f.getHeaderId().getBytes(), 0, d, 0, 2);
System.arraycopy(f.getLocalFileDataLength().getBytes(), 0, d, 2, 2);
- System.arraycopy(l, 0, d, 4, l.length);
+ System.arraycopy(f.getLocalFileDataData(), 0, d, 4, AiobThrowingExtraField.LENGTH);
try {
ExtraFieldUtils.parse(d);
fail("data should be invalid");
} catch (final ZipException e) {
assertEquals("message",
- "Failed to parse corrupt ZIP extra field of type 756e",
+ "Failed to parse corrupt ZIP extra field of type 1000",
e.getMessage());
}
}
@@ -246,4 +244,43 @@ public class ExtraFieldUtilsTest implements UnixStat {
}
}
+
+ public static class AiobThrowingExtraField implements ZipExtraField {
+ static final int LENGTH = 4;
+ @Override
+ public ZipShort getHeaderId() {
+ return AIOB_HEADER;
+ }
+ @Override
+ public ZipShort getLocalFileDataLength() {
+ return new ZipShort(LENGTH);
+ }
+
+ @Override
+ public ZipShort getCentralDirectoryLength() {
+ return getLocalFileDataLength();
+ }
+
+ @Override
+ public byte[] getLocalFileDataData() {
+ return new byte[LENGTH];
+ }
+
+ @Override
+ public byte[] getCentralDirectoryData() {
+ return getLocalFileDataData();
+ }
+
+ @Override
+ public void parseFromLocalFileData(byte[] buffer, int offset, int length)
+ throws ZipException {
+ throw new ArrayIndexOutOfBoundsException();
+ }
+
+ @Override
+ public void parseFromCentralDirectoryData(byte[] buffer, int offset, int length)
+ throws ZipException {
+ parseFromLocalFileData(buffer, offset, length);
+ }
+ }
}