For backward compatibility this method just delegates to {@link #decodeChunk(byte[], int, byte[], int, int)}, * ignoring the {@code inEnd} parameter. Subclasses should override it and consider the {@code inEnd} parameter. * + * @throws LZFException If content is not valid LZF: this includes truncated content, as well as + * back references that would point before start of the chunk's output, or produce more output + * than {@code outEnd - outPos} bytes. Note that invalid arguments (as opposed to invalid + * content) are instead reported as {@link ArrayIndexOutOfBoundsException}. + * * @since 1.2 */ public void decodeChunk(byte[] in, int inPos, int inEnd, byte[] out, int outPos, int outEnd) @@ -301,4 +321,19 @@ protected void _reportArrayOverflow(byte[] targetBuffer, int outPtr, int dataLen throw new LZFException("Target buffer too small ("+targetBuffer.length+"): can not copy/uncompress " +dataLen+" bytes to offset "+outPtr); } + + /** + * Helper method called when the header of a block extends past the end of available input + */ + private void _reportTruncatedHeader(int blockNr, int offset) throws LZFException { + throw new LZFException("Corrupt input data, block #"+blockNr+" (at offset "+offset + +"): truncated block header"); + } + + /** + * Helper method called when the content of a block extends past the end of available input + */ + private void _reportIncompleteBlock(int blockNr) throws LZFException { + throw new LZFException("Corrupt input data, block #"+blockNr+" is incomplete"); + } } diff --git a/src/main/java/com/ning/compress/lzf/impl/UnsafeChunkDecoder.java b/src/main/java/com/ning/compress/lzf/impl/UnsafeChunkDecoder.java index 8472cf2..0188d90 100644 --- a/src/main/java/com/ning/compress/lzf/impl/UnsafeChunkDecoder.java +++ b/src/main/java/com/ning/compress/lzf/impl/UnsafeChunkDecoder.java @@ -33,6 +33,17 @@ public class UnsafeChunkDecoder extends ChunkDecoder } } + /** + * Maximum offset of a back-reference: 5 bits from the control byte, a full byte, + * and the implicit 1 -- so at most 8192 bytes back + */ + private final static int MAX_BACK_REF_OFFSET = 8192; + + /** + * Maximum run length of a back-reference: 255 + 9, for the long form + */ + private final static int MAX_BACK_REF_LENGTH = 264; + private static final long BYTE_ARRAY_OFFSET = unsafe.arrayBaseOffset(byte[].class); // private static final long SHORT_ARRAY_OFFSET = unsafe.arrayBaseOffset(short[].class); // private static final long SHORT_ARRAY_STRIDE = unsafe.arrayIndexScale(short[].class); @@ -82,6 +93,14 @@ public final void decodeChunk(byte[] in, int inPos, int inEnd, byte[] out, int o final int outPosStart = outPos; + // Back-reference offsets and run lengths are bounded, so checks against the start and the + // end of this chunk's output can only fail near them: in between they are provably + // redundant, and skipping them matters because this is the hot path. + // Note: distance from the start is compared as a difference, not against a precomputed + // `outPos + MAX_BACK_REF_OFFSET`, since that sum can overflow for a very large output + // buffer -- which would silently disable the check for the whole chunk + final int lengthChecksAfter = outEnd - MAX_BACK_REF_LENGTH; + // We need to take care of end condition, leave last 32 bytes out final int inputEnd32 = inEnd - 32; final int outputEnd8 = outEnd - 8; @@ -89,35 +108,53 @@ public final void decodeChunk(byte[] in, int inPos, int inEnd, byte[] out, int o main_loop: do { + if (inPos >= inEnd) { + throw new LZFException("Corrupt data: truncated block"); + } int ctrl = in[inPos++] & 255; while (ctrl < LZFChunk.MAX_LITERAL) { // literal run(s) + final int literalLength = ctrl + 1; if (outPos > outputEnd32 || inPos > inputEnd32) { - System.arraycopy(in, inPos, out, outPos, ctrl+1); + // Near the end of input or output: a literal run is at most 32 bytes long, so + // before these bounds it provably fits and does not need to be checked + if (inPos > inEnd - literalLength || outPos > outEnd - literalLength) { + throw new LZFException("Corrupt data: truncated block"); + } + System.arraycopy(in, inPos, out, outPos, literalLength); } else { copyUpTo32(in, inPos, out, outPos, ctrl); } - ++ctrl; - inPos += ctrl; - outPos += ctrl; + inPos += literalLength; + outPos += literalLength; if (outPos >= outEnd) { break main_loop; } + // The literal run may end exactly at inEnd, but another byte is required for the next control token. + if (inPos >= inEnd) { + throw new LZFException("Corrupt data: truncated block"); + } ctrl = in[inPos++] & 255; } // back reference int len = ctrl >> 5; ctrl = -((ctrl & 0x1f) << 8) - 1; - // short back reference? 2 bytes; run lengths of 2 - 8 bytes + // short back reference? 2 bytes; run lengths of 3 - 8 bytes if (len < 7) { + if (inPos >= inEnd) { + throw new LZFException("Corrupt data: truncated block"); + } ctrl -= in[inPos++] & 255; - if (ctrl < -7 && outPos < outputEnd8) { // non-overlapping? can use efficient bulk copy - if (outPos + ctrl < outPosStart) { + final int copyLength = len + 2; + if ((outPos - outPosStart) < MAX_BACK_REF_OFFSET || outPos > lengthChecksAfter) { + if (outPos > outEnd - copyLength || outPos + ctrl < outPosStart) { throw new LZFException("Invalid back reference"); } + } + if (ctrl < -7 && outPos < outputEnd8) { // non-overlapping? can use efficient bulk copy final long rawOffset = BYTE_ARRAY_OFFSET + outPos; unsafe.putLong(out, rawOffset, unsafe.getLong(out, rawOffset + ctrl)); // moveLong(out, outPos, outEnd, ctrl); - outPos += len+2; + outPos += copyLength; continue; } // otherwise, byte-by-byte @@ -125,17 +162,22 @@ public final void decodeChunk(byte[] in, int inPos, int inEnd, byte[] out, int o continue; } // long back reference: 3 bytes, length of up to 264 bytes + if (inPos > inEnd - 2) { + throw new LZFException("Corrupt data: truncated block"); + } len = (in[inPos++] & 255) + 9; ctrl -= in[inPos++] & 255; - // First: ovelapping case can't use default handling, off line. + if ((outPos - outPosStart) < MAX_BACK_REF_OFFSET || outPos > lengthChecksAfter) { + if (outPos > outEnd - len || outPos + ctrl < outPosStart) { + throw new LZFException("Invalid back reference"); + } + } + // First: overlapping case can't use default handling, handled off-line. if ((ctrl > -9) || (outPos > outputEnd32)) { outPos = copyOverlappingLong(out, outPos, ctrl, len-9); continue; } // but non-overlapping is simple - if (outPos + ctrl < outPosStart) { - throw new LZFException("Invalid back reference"); - } if (len <= 32) { copyUpTo32(out, outPos+ctrl, outPos, len-1); outPos += len; @@ -310,9 +352,9 @@ private final static void copyUpTo32(byte[] in, int inputIndex, byte[] out, int } } private final static void copyLong(byte[] buffer, int inputIndex, int outputIndex, int length, - int outputEnd8) + int outputEnd32) { - if ((outputIndex + length) > outputEnd8) { + if ((outputIndex + length) > outputEnd32) { copyLongTail(buffer, inputIndex,outputIndex, length); return; } diff --git a/src/main/java/com/ning/compress/lzf/impl/VanillaChunkDecoder.java b/src/main/java/com/ning/compress/lzf/impl/VanillaChunkDecoder.java index 7f99f3c..bce2b8e 100644 --- a/src/main/java/com/ning/compress/lzf/impl/VanillaChunkDecoder.java +++ b/src/main/java/com/ning/compress/lzf/impl/VanillaChunkDecoder.java @@ -50,82 +50,37 @@ public void decodeChunk(byte[] in, int inPos, byte[] out, int outPos, int outEnd public final void decodeChunk(byte[] in, int inPos, int inEnd, byte[] out, int outPos, int outEnd) throws LZFException { + final int outStart = outPos; + // NOTE: loop must be entered at least once, same as UnsafeChunkDecoder: a compressed + // chunk always has at least one control byte, so a chunk with no content at all + // (declared lengths of 0) is corrupt and must be reported as such by both decoders do { + if (inPos >= inEnd) { + throw new LZFException("Corrupt data: truncated block"); + } int ctrl = in[inPos++] & 255; if (ctrl < LZFChunk.MAX_LITERAL) { // literal run - switch (ctrl) { - case 31: - out[outPos++] = in[inPos++]; - case 30: - out[outPos++] = in[inPos++]; - case 29: - out[outPos++] = in[inPos++]; - case 28: - out[outPos++] = in[inPos++]; - case 27: - out[outPos++] = in[inPos++]; - case 26: - out[outPos++] = in[inPos++]; - case 25: - out[outPos++] = in[inPos++]; - case 24: - out[outPos++] = in[inPos++]; - case 23: - out[outPos++] = in[inPos++]; - case 22: - out[outPos++] = in[inPos++]; - case 21: - out[outPos++] = in[inPos++]; - case 20: - out[outPos++] = in[inPos++]; - case 19: - out[outPos++] = in[inPos++]; - case 18: - out[outPos++] = in[inPos++]; - case 17: - out[outPos++] = in[inPos++]; - case 16: - out[outPos++] = in[inPos++]; - case 15: - out[outPos++] = in[inPos++]; - case 14: - out[outPos++] = in[inPos++]; - case 13: - out[outPos++] = in[inPos++]; - case 12: - out[outPos++] = in[inPos++]; - case 11: - out[outPos++] = in[inPos++]; - case 10: - out[outPos++] = in[inPos++]; - case 9: - out[outPos++] = in[inPos++]; - case 8: - out[outPos++] = in[inPos++]; - case 7: - out[outPos++] = in[inPos++]; - case 6: - out[outPos++] = in[inPos++]; - case 5: - out[outPos++] = in[inPos++]; - case 4: - out[outPos++] = in[inPos++]; - case 3: - out[outPos++] = in[inPos++]; - case 2: - out[outPos++] = in[inPos++]; - case 1: - out[outPos++] = in[inPos++]; - case 0: - out[outPos++] = in[inPos++]; + int literalLen = ctrl + 1; + if (inPos > inEnd - literalLen || outPos > outEnd - literalLen) { + throw new LZFException("Corrupt data: truncated block"); } + System.arraycopy(in, inPos, out, outPos, literalLen); + inPos += literalLen; + outPos += literalLen; continue; } // back reference int len = ctrl >> 5; ctrl = -((ctrl & 0x1f) << 8) - 1; if (len < 7) { // 2 bytes; length of 3 - 8 bytes + if (inPos >= inEnd) { + throw new LZFException("Corrupt data: truncated block"); + } ctrl -= in[inPos++] & 255; + final int copyLength = len + 2; + if (outPos > outEnd - copyLength || outPos + ctrl < outStart) { + throw new LZFException("Invalid back reference"); + } out[outPos] = out[outPos++ + ctrl]; out[outPos] = out[outPos++ + ctrl]; switch (len) { @@ -146,10 +101,17 @@ public final void decodeChunk(byte[] in, int inPos, int inEnd, byte[] out, int o } // long version (3 bytes, length of up to 264 bytes) + if (inPos > inEnd - 2) { + throw new LZFException("Corrupt data: truncated block"); + } len = in[inPos++] & 255; ctrl -= in[inPos++] & 255; - - // First: if there is no overlap, can just use arraycopy: + final int copyLength = len + 9; + if (outPos > outEnd - copyLength || outPos + ctrl < outStart) { + throw new LZFException("Invalid back reference"); + } + + // First: if there is no overlap, can just use bulk copy: if ((ctrl + len) < -9) { len += 9; if (len <= 32) { diff --git a/src/test/java/com/ning/compress/lzf/TestFuzzUnsafeLZF.java b/src/test/java/com/ning/compress/lzf/TestFuzzUnsafeLZF.java index 91b2453..d71d721 100644 --- a/src/test/java/com/ning/compress/lzf/TestFuzzUnsafeLZF.java +++ b/src/test/java/com/ning/compress/lzf/TestFuzzUnsafeLZF.java @@ -8,6 +8,7 @@ import com.ning.compress.lzf.impl.*; import java.io.ByteArrayInputStream; +import java.io.ByteArrayOutputStream; import java.io.IOException; import java.io.OutputStream; import java.lang.annotation.Retention; @@ -55,33 +56,43 @@ void decode(byte @NotNull @WithLength(min = 0, max = 32767) [] input, byte @NotN System.arraycopy(suffix, 0, input2, input.length, suffix.length); byte[] decoded1 = null; + Class failure1 = null; try { int decodedLen = decoder.decode(input1, 0, input.length, output); decoded1 = Arrays.copyOf(output, decodedLen); - } catch (LZFException | ArrayIndexOutOfBoundsException ignored) { + } catch (LZFException | RuntimeException e) { + failure1 = e.getClass(); } // Repeat decoding, this time with (ignored) suffix and prefilled output // Should lead to same decoded result Arrays.fill(output, (byte) 0xFF); byte[] decoded2 = null; + Class failure2 = null; try { int decodedLen = decoder.decode(input2, 0, input.length, output); decoded2 = Arrays.copyOf(output, decodedLen); - } catch (LZFException | ArrayIndexOutOfBoundsException ignored) { + } catch (LZFException | RuntimeException e) { + failure2 = e.getClass(); } assertArrayEquals(decoded1, decoded2); + assertEquals(failure1, failure2); // Compare with result of vanilla decoder byte[] decodedVanilla = null; + Class failureVanilla = null; try { int decodedLen = new VanillaChunkDecoder().decode(input, output); decodedVanilla = Arrays.copyOf(output, decodedLen); - } catch (Exception ignored) { + } catch (LZFException | RuntimeException e) { + failureVanilla = e.getClass(); } assertArrayEquals(decodedVanilla, decoded1); - + // Note: comparing the type of failure as well, not just the decoded content: malformed + // content has to be reported as `LZFException` by both implementations, and comparing + // content alone would not catch one of them throwing an unchecked exception instead + assertEquals(failureVanilla, failure1); } @LZFFuzzTest @@ -162,34 +173,76 @@ void encodeAppend(byte @NotNull @WithLength(min = 1, max = 32767) [] input, @InR @LZFFuzzTest void inputStreamRead(byte @NotNull @WithLength(min = 0, max = 32767) [] input, @InRange(min = 1, max = 32767) int readBufferSize) throws IOException { - UnsafeChunkDecoder decoder = new UnsafeChunkDecoder(); + // Compare the two decoder implementations: they have to produce the same content, and + // fail the same way. Note that the stream path reaches `decodeChunk(InputStream, ...)` + // and `skipOrDecodeChunk(...)`, which are implemented separately by each decoder. + Outcome vanilla = readOutcome(new VanillaChunkDecoder(), input, readBufferSize); + Outcome unsafe = readOutcome(new UnsafeChunkDecoder(), input, readBufferSize); + assertEquals(vanilla.failure, unsafe.failure); + assertArrayEquals(vanilla.content, unsafe.content); + } + + private static Outcome readOutcome(ChunkDecoder decoder, byte[] input, int readBufferSize) throws IOException { + ByteArrayOutputStream consumed = new ByteArrayOutputStream(); try (LZFInputStream inputStream = new LZFInputStream(decoder, new ByteArrayInputStream(input), new BufferRecycler(), false)) { byte[] readBuffer = new byte[readBufferSize]; - while (inputStream.read(readBuffer) != -1) { - // Do nothing, just consume the data + int count; + while ((count = inputStream.read(readBuffer)) != -1) { + consumed.write(readBuffer, 0, count); } - } catch (LZFException | ArrayIndexOutOfBoundsException ignored) { } // TODO: This IndexOutOfBoundsException occurs because LZFInputStream makes an invalid call to ByteArrayInputStream // The reason seems to be that `_inputBuffer` is only MAX_CHUNK_LEN large, but should be `2 + MAX_CHUNK_LEN` to // account for first two bytes encoding the length? (might affect more places in code) - catch (IndexOutOfBoundsException ignored) { + // Tolerated here as long as both decoder implementations behave the same way. + catch (LZFException | RuntimeException e) { + return new Outcome(e.getClass(), consumed.toByteArray()); } + return new Outcome(null, consumed.toByteArray()); } @LZFFuzzTest void inputStreamSkip(byte @NotNull @WithLength(min = 0, max = 32767) [] input, @InRange(min = 1, max = 32767) int skipCount) throws IOException { - UnsafeChunkDecoder decoder = new UnsafeChunkDecoder(); + Outcome vanilla = skipOutcome(new VanillaChunkDecoder(), input, skipCount); + Outcome unsafe = skipOutcome(new UnsafeChunkDecoder(), input, skipCount); + assertEquals(vanilla.failure, unsafe.failure); + assertEquals(vanilla.skipped, unsafe.skipped); + } + + private static Outcome skipOutcome(ChunkDecoder decoder, byte[] input, int skipCount) throws IOException { + long skipped = 0L; try (LZFInputStream inputStream = new LZFInputStream(decoder, new ByteArrayInputStream(input), new BufferRecycler(), false)) { - while (inputStream.skip(skipCount) > 0) { - // Do nothing, just consume the data + long count; + while ((count = inputStream.skip(skipCount)) > 0) { + skipped += count; } - } catch (LZFException ignored) { } - // TODO: This IndexOutOfBoundsException occurs because LZFInputStream makes an invalid call to ByteArrayInputStream - // The reason seems to be that `_inputBuffer` is only MAX_CHUNK_LEN large, but should be `2 + MAX_CHUNK_LEN` to - // account for first two bytes encoding the length? (might affect more places in code) - catch (IndexOutOfBoundsException ignored) { + // TODO: see `readOutcome` above for the IndexOutOfBoundsException case + catch (LZFException | RuntimeException e) { + return new Outcome(e.getClass(), skipped); + } + return new Outcome(null, skipped); + } + + /** + * Outcome of consuming content with one specific decoder implementation: what was produced, + * and how (or whether) it failed. Used to compare the implementations against each other. + */ + private static class Outcome { + final Class failure; + final byte[] content; + final long skipped; + + Outcome(Class failure, byte[] content) { + this.failure = failure; + this.content = content; + this.skipped = 0L; + } + + Outcome(Class failure, long skipped) { + this.failure = failure; + this.content = null; + this.skipped = skipped; } } diff --git a/src/test/java/com/ning/compress/lzf/TestLZFDecoder.java b/src/test/java/com/ning/compress/lzf/TestLZFDecoder.java index fb7dd0e..bbf549e 100644 --- a/src/test/java/com/ning/compress/lzf/TestLZFDecoder.java +++ b/src/test/java/com/ning/compress/lzf/TestLZFDecoder.java @@ -2,6 +2,7 @@ import java.io.*; import java.nio.charset.StandardCharsets; +import java.util.Arrays; import com.ning.compress.BaseForTests; import com.ning.compress.lzf.impl.UnsafeChunkDecoder; @@ -9,6 +10,7 @@ import org.junit.jupiter.api.Test; import static org.junit.jupiter.api.Assertions.assertArrayEquals; +import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertThrows; public class TestLZFDecoder extends BaseForTests @@ -50,12 +52,172 @@ public void testUnsafeValidation() { assertThrows(ArrayIndexOutOfBoundsException.class, () -> decoder.decodeChunk(array, goodStart, goodEnd, array, goodStart, array.length + 1)); } + @Test + public void testTruncatedShortBackReference() { + // Payload (after the 7 byte header) is: `0x00` = literal run of 1 byte (`0x04`), + // followed by `0x50` = short back-reference, which needs one more byte for its + // offset -- but the chunk ends. First case has a trailing byte that is outside of + // the given input end (and must not be read), second case has no trailing byte at all. + byte[] inputWithTrailingByte = new byte[] { + LZFChunk.BYTE_Z, LZFChunk.BYTE_V, LZFChunk.BLOCK_TYPE_COMPRESSED, + 0, 3, 0, (byte) 0x8f, 0, 4, 0x50, 0x53 + }; + byte[] truncatedInput = new byte[] { + LZFChunk.BYTE_Z, LZFChunk.BYTE_V, LZFChunk.BLOCK_TYPE_COMPRESSED, + 0, 3, 0, (byte) 0x8f, 0, 4, 0x50 + }; + + assertThrows(LZFException.class, () -> + ChunkDecoderFactory.safeInstance().decodeChunk(inputWithTrailingByte, 7, 10, new byte[143], 0, 143)); + assertThrows(LZFException.class, () -> + ChunkDecoderFactory.optimalInstance().decodeChunk(inputWithTrailingByte, 7, 10, new byte[143], 0, 143)); + assertThrows(LZFException.class, () -> + ChunkDecoderFactory.safeInstance().decodeChunk(truncatedInput, 7, 10, new byte[143], 0, 143)); + assertThrows(LZFException.class, () -> + ChunkDecoderFactory.optimalInstance().decodeChunk(truncatedInput, 7, 10, new byte[143], 0, 143)); + } + + @Test + public void testTruncatedBlocks() { + // Note: `decode(byte[])` validates framing up front, via `calculateUncompressedSize()`. + // These go through the overload that takes a target buffer, which does not -- so the + // framing loop itself has to check that header and content fit within the input. + byte[][] truncatedInputs = new byte[][] { + // signature only: no type, no length + { LZFChunk.BYTE_Z, LZFChunk.BYTE_V }, + // ... type but no length + { LZFChunk.BYTE_Z, LZFChunk.BYTE_V, LZFChunk.BLOCK_TYPE_COMPRESSED }, + // ... only one of the two length bytes + { LZFChunk.BYTE_Z, LZFChunk.BYTE_V, LZFChunk.BLOCK_TYPE_COMPRESSED, 0 }, + // compressed block without the 2 bytes of uncompressed length + { LZFChunk.BYTE_Z, LZFChunk.BYTE_V, LZFChunk.BLOCK_TYPE_COMPRESSED, 0, 2, 0 }, + // compressed block declaring 2 bytes of content, but only 1 included + { LZFChunk.BYTE_Z, LZFChunk.BYTE_V, LZFChunk.BLOCK_TYPE_COMPRESSED, 0, 2, 0, 0, 0 }, + // non-compressed block declaring 10 bytes of content, but only 2 included + { LZFChunk.BYTE_Z, LZFChunk.BYTE_V, LZFChunk.BLOCK_TYPE_NON_COMPRESSED, 0, 10, 0x41, 0x42 }, + }; + + for (int i = 0; i < truncatedInputs.length; ++i) { + final byte[] input = truncatedInputs[i]; + String desc = "truncated input #"+i+" (length "+input.length+")"; + assertThrows(LZFException.class, () -> + ChunkDecoderFactory.safeInstance().decode(input, 0, input.length, new byte[64]), desc); + assertThrows(LZFException.class, () -> + ChunkDecoderFactory.optimalInstance().decode(input, 0, input.length, new byte[64]), desc); + } + } + + @Test + public void testBackRefBeforeChunkStart() { + _testBackRefBeforeChunkStart(ChunkDecoderFactory.safeInstance()); + _testBackRefBeforeChunkStart(ChunkDecoderFactory.optimalInstance()); + } + + @Test + public void testBackRefPastChunkEnd() { + _testBackRefPastChunkEnd(ChunkDecoderFactory.safeInstance()); + _testBackRefPastChunkEnd(ChunkDecoderFactory.optimalInstance()); + } + + @Test + public void testBackRefIntoPreviousChunk() throws IOException { + // First chunk decodes normally; second one consists of nothing but a back-reference + // with offset -1, which would have to reach into output of the first chunk + byte[] firstChunk = compress("SECRETSECRETSECRETSECRET".getBytes(StandardCharsets.UTF_8)); + byte[] secondChunk = new byte[] { + LZFChunk.BYTE_Z, LZFChunk.BYTE_V, LZFChunk.BLOCK_TYPE_COMPRESSED, + 0, 2, // compressed length + 0, 3, // uncompressed length + SHORT_BACK_REF_MINUS_ONE[0], SHORT_BACK_REF_MINUS_ONE[1] + }; + byte[] input = new byte[firstChunk.length + secondChunk.length]; + System.arraycopy(firstChunk, 0, input, 0, firstChunk.length); + System.arraycopy(secondChunk, 0, input, firstChunk.length, secondChunk.length); + + assertThrows(LZFException.class, () -> ChunkDecoderFactory.safeInstance().decode(input)); + assertThrows(LZFException.class, () -> ChunkDecoderFactory.optimalInstance().decode(input)); + } + + @Test + public void testZeroLengthCompressedChunk() { + // Compressed chunk that declares both lengths as 0, so it does not even contain a + // control byte: malformed, and both decoders have to agree that it is + byte[] chunk = new byte[] { + LZFChunk.BYTE_Z, LZFChunk.BYTE_V, LZFChunk.BLOCK_TYPE_COMPRESSED, + 0, 0, // compressed length + 0, 0 // uncompressed length + }; + assertThrows(LZFException.class, () -> + ChunkDecoderFactory.safeInstance().decodeChunk(chunk, 7, 7, new byte[0], 0, 0)); + assertThrows(LZFException.class, () -> + ChunkDecoderFactory.optimalInstance().decodeChunk(chunk, 7, 7, new byte[0], 0, 0)); + + assertThrows(LZFException.class, () -> ChunkDecoderFactory.safeInstance().decode(chunk)); + assertThrows(LZFException.class, () -> ChunkDecoderFactory.optimalInstance().decode(chunk)); + } + /* /////////////////////////////////////////////////////////////////////// // Second-level test methods /////////////////////////////////////////////////////////////////////// */ + /** + * `0x20` = short back-reference, run length 3, high bits of offset 0; + * following `0x00` gives low bits, for an offset of -1 + */ + private final static byte[] SHORT_BACK_REF_MINUS_ONE = new byte[] { 0x20, 0x00 }; + + /** + * `0xE0` = long back-reference, high bits of offset 0; following `0x00` gives + * run length of 9, and last `0x00` low bits of offset, for an offset of -1 + */ + private final static byte[] LONG_BACK_REF_MINUS_ONE = new byte[] { (byte) 0xE0, 0x00, 0x00 }; + + /** + * Verifies that back-references pointing before the beginning of the chunk's own output + * are rejected. Offsets of -1 to -8 are handled by the "overlapping" copy paths, which + * are separate from the bulk copy ones. + */ + private void _testBackRefBeforeChunkStart(ChunkDecoder decoder) + { + // First: back-reference at the very beginning of the output buffer + assertThrows(LZFException.class, () -> + decoder.decodeChunk(SHORT_BACK_REF_MINUS_ONE, 0, 2, new byte[3], 0, 3)); + assertThrows(LZFException.class, () -> + decoder.decodeChunk(LONG_BACK_REF_MINUS_ONE, 0, 3, new byte[9], 0, 9)); + + // And then at beginning of the chunk, but not of the buffer: must not reach back + // into output of an earlier chunk (or into whatever the buffer contained before) + byte[] output = new byte[32]; + Arrays.fill(output, (byte) 'x'); + assertThrows(LZFException.class, () -> + decoder.decodeChunk(SHORT_BACK_REF_MINUS_ONE, 0, 2, output, 8, 11)); + assertThrows(LZFException.class, () -> + decoder.decodeChunk(LONG_BACK_REF_MINUS_ONE, 0, 3, output, 8, 17)); + } + + /** + * Verifies that a back-reference whose run length would write past the declared + * uncompressed length of the chunk is rejected (and does not write past it either). + */ + private void _testBackRefPastChunkEnd(ChunkDecoder decoder) + { + // `0x01` = literal run of 2 bytes ('A', 'B'), then a back-reference with a valid + // offset of -2, but a run length of 3 (short) / 9 (long) although only 2 bytes remain + byte[] shortBackRef = new byte[] { 0x01, 'A', 'B', 0x20, 0x01 }; + byte[] longBackRef = new byte[] { 0x01, 'A', 'B', (byte) 0xE0, 0x00, 0x01 }; + + byte[] output = new byte[16]; + assertThrows(LZFException.class, () -> decoder.decodeChunk(shortBackRef, 0, 5, output, 0, 4)); + assertThrows(LZFException.class, () -> decoder.decodeChunk(longBackRef, 0, 6, output, 0, 4)); + + // ... and nothing was written past the declared end of the chunk + for (int i = 4; i < output.length; ++i) { + assertEquals(0, output[i], "Wrote past declared uncompressed length, at offset "+i); + } + } + private void _testSimple(ChunkDecoder decoder) throws IOException { byte[] orig = "Another trivial test".getBytes(StandardCharsets.UTF_8); diff --git a/src/test/java/com/ning/compress/lzf/TestLZFDecoderParity.java b/src/test/java/com/ning/compress/lzf/TestLZFDecoderParity.java new file mode 100644 index 0000000..7e5af88 --- /dev/null +++ b/src/test/java/com/ning/compress/lzf/TestLZFDecoderParity.java @@ -0,0 +1,302 @@ +package com.ning.compress.lzf; + +import java.io.ByteArrayOutputStream; +import java.io.IOException; +import java.nio.charset.StandardCharsets; +import java.util.Arrays; +import java.util.Random; + +import com.ning.compress.BaseForTests; +import com.ning.compress.lzf.util.ChunkDecoderFactory; +import org.junit.jupiter.api.Test; + +import static org.junit.jupiter.api.Assertions.assertArrayEquals; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertNull; +import static org.junit.jupiter.api.Assertions.fail; + +/** + * Tests that verify that the "safe" and "optimal" {@link ChunkDecoder} implementations + * agree on malformed input. They are separate hand-optimized implementations of the same + * format, with validation duplicated in both, so they can drift apart -- and have: see + * "#85: Handle malformed LZF back references". + *
+ * For every input both decoders have to either fail with {@link LZFException}, or succeed + * with identical output. Anything else -- one succeeding where the other fails, differing + * output, or an exception other than {@code LZFException} for content (as opposed to + * argument) problems -- is a failure. + */ +public class TestLZFDecoderParity extends BaseForTests +{ + /** + * Declared uncompressed lengths to try; includes 0, the boundaries of the + * short (3 - 8 bytes) and long (9 - 264 bytes) back-reference run lengths + */ + private final static int[] DECLARED_LENGTHS = new int[] { 0, 1, 3, 8, 9, 16, 40, 264, 300 }; + + /** + * Offsets of the chunk within the output buffer: 0, and a non-zero one so that + * back-references pointing before the chunk (but still within the buffer) are covered + */ + private final static int[] CHUNK_OFFSETS = new int[] { 0, 5 }; + + @Test + public void testEveryControlByte() { + byte[][] tails = new byte[][] { + {}, { 0x00 }, { 0x41 }, { (byte) 0xFF }, { 0x00, 0x00 }, { 0x01, 0x02, 0x03 } + }; + for (int ctrl = 0; ctrl < 256; ++ctrl) { + for (byte[] tail : tails) { + byte[] payload = new byte[1 + tail.length]; + payload[0] = (byte) ctrl; + System.arraycopy(tail, 0, payload, 1, tail.length); + for (int declaredLen : DECLARED_LENGTHS) { + for (int chunkOffset : CHUNK_OFFSETS) { + _assertSameOutcome(payload, chunkOffset, declaredLen); + } + } + } + } + } + + @Test + public void testRandomPayloads() { + Random rnd = new Random(1234); // fixed seed: has to stay reproducible + for (int i = 0; i < 3000; ++i) { + byte[] payload = new byte[rnd.nextInt(24)]; + rnd.nextBytes(payload); + _assertSameOutcome(payload, CHUNK_OFFSETS[i & 1], DECLARED_LENGTHS[i % DECLARED_LENGTHS.length]); + } + } + + @Test + public void testTruncatedAndMutatedValidData() throws IOException { + byte[] orig = "the quick brown fox jumps over the lazy dog, the quick brown fox" + .getBytes(StandardCharsets.UTF_8); + // Compressed payload of a single chunk, without the 7 byte header + byte[] payload = Arrays.copyOfRange(compress(orig), 7, compress(orig).length); + + // Truncated at every possible point ... + for (int cut = 0; cut <= payload.length; ++cut) { + byte[] truncated = Arrays.copyOf(payload, cut); + for (int chunkOffset : CHUNK_OFFSETS) { + _assertSameOutcome(truncated, chunkOffset, orig.length); + } + } + // ... and with every single byte mutated, to hit control bytes, lengths and offsets + for (int i = 0; i < payload.length; ++i) { + for (int mask : new int[] { 0x01, 0x20, 0x80, 0xE0 }) { + byte[] mutated = payload.clone(); + mutated[i] = (byte) (mutated[i] ^ mask); + _assertSameOutcome(mutated, 0, orig.length); + } + } + } + + /** + * Back-reference offsets reach at most 8192 bytes back, so past that point a back-reference + * provably can not point before the start of the chunk -- which decoders may rely on to skip + * the check. Verifies the boundary of that reasoning: getting it wrong would let a decoder + * copy content from before the chunk (or from stale buffer content) without any error. + *
+ * Note that these use chunks larger than 8192 bytes on purpose: the cases above are all far + * too small to reach the point where such a check can be skipped. + */ + @Test + public void testMaxOffsetBackReferenceBoundary() { + // Short form: control byte with run length 1 (so 3 bytes) and all 5 offset bits set, + // followed by all 8 bits of the low offset byte -- an offset of -8192 + _testMaxOffsetBackReferences(new byte[] { (byte) 0x3F, (byte) 0xFF }, 3); + // Long form: control byte marking a long run with all 5 offset bits set, a run length byte + // of 0 (so 9 bytes), then the low offset byte -- also an offset of -8192. Covered + // separately because it is checked at a different place in the decoders. + _testMaxOffsetBackReferences(new byte[] { (byte) 0xFF, 0x00, (byte) 0xFF }, 9); + } + + private void _testMaxOffsetBackReferences(byte[] backRef, int runLength) { + // one byte short of the maximum offset being reachable: has to be rejected + _testMaxOffsetBackReference(backRef, runLength, MAX_BACK_REF_OFFSET - 1, false); + // exactly at the start of the chunk, and past it: valid + _testMaxOffsetBackReference(backRef, runLength, MAX_BACK_REF_OFFSET, true); + _testMaxOffsetBackReference(backRef, runLength, MAX_BACK_REF_OFFSET + 1, true); + _testMaxOffsetBackReference(backRef, runLength, MAX_BACK_REF_OFFSET + 4096, true); + } + + /** + * Verifies that a run overrunning the declared uncompressed length is rejected even far into + * a large chunk, where checks against the start of the chunk are no longer needed. + */ + @Test + public void testLargeChunkOverrun() { + final int prefixLen = 20000; + byte[] literals = _literalRuns(prefixLen); + byte[] payload = Arrays.copyOf(literals, literals.length + 3); + payload[literals.length] = (byte) 0xE0; // long back-reference, high bits of offset 0 + payload[literals.length + 1] = (byte) 0xFF; // run length 255 + 9 = 264 + payload[literals.length + 2] = (byte) 0x01; // low bits of offset, for -2 + + // Declared length one byte short of what the run produces: rejected + _assertBothFail(payload, 0, prefixLen + 263, "264 byte run at offset "+prefixLen+", 263 declared"); + // ... and the same content with a matching declared length is valid + _assertBothSucceed(payload, 0, prefixLen + 264, "264 byte run at offset "+prefixLen); + } + + /* + /////////////////////////////////////////////////////////////////////// + // Second-level test methods + /////////////////////////////////////////////////////////////////////// + */ + + /** + * Maximum offset of a back-reference: 5 bits from the control byte, a full byte, and the + * implicit 1. Kept here independently of the decoders on purpose, so that changing it there + * does not silently change what this test verifies. + */ + private final static int MAX_BACK_REF_OFFSET = 8192; + + private void _testMaxOffsetBackReference(byte[] backRef, int runLength, int outputBeforeBackRef, + boolean valid) + { + // Note: content is added after the back-reference so that it is not close to the end of + // the chunk. Otherwise a decoder would have to check the run length there anyway, which + // would catch a bad offset as a side effect -- and this would verify nothing about offsets. + final int tailLen = 1024; + byte[] head = _literalRuns(outputBeforeBackRef); + byte[] tail = _literalRuns(tailLen); + byte[] payload = new byte[head.length + backRef.length + tail.length]; + System.arraycopy(head, 0, payload, 0, head.length); + System.arraycopy(backRef, 0, payload, head.length, backRef.length); + System.arraycopy(tail, 0, payload, head.length + backRef.length, tail.length); + + final int declaredLen = outputBeforeBackRef + runLength + tailLen; + String desc = runLength+" byte back-reference with offset -"+MAX_BACK_REF_OFFSET + +" at output offset "+outputBeforeBackRef; + + for (int chunkOffset : CHUNK_OFFSETS) { + if (valid) { + byte[] output = _assertBothSucceed(payload, chunkOffset, declaredLen, desc); + // Copied bytes have to come from exactly 8192 bytes back, which is at or after the + // start of the chunk; had they come from before it, they would be the prefill + final int copiedFrom = outputBeforeBackRef - MAX_BACK_REF_OFFSET; + for (int i = 0; i < runLength; ++i) { + assertEquals(output[chunkOffset + copiedFrom + i], + output[chunkOffset + outputBeforeBackRef + i], + desc+": byte "+i+" of the run was not copied from output offset "+copiedFrom); + } + } else { + _assertBothFail(payload, chunkOffset, declaredLen, desc); + } + } + } + + /** + * Builds a payload of literal runs producing exactly given number of output bytes + */ + private byte[] _literalRuns(int length) + { + ByteArrayOutputStream payload = new ByteArrayOutputStream(); + int written = 0; + while (written < length) { + int run = Math.min(LZFChunk.MAX_LITERAL, length - written); + payload.write(run - 1); // control byte of a literal run of N bytes is N-1 + for (int i = 0; i < run; ++i) { + payload.write('A' + ((written + i) % 26)); + } + written += run; + } + return payload.toByteArray(); + } + + private void _assertBothFail(byte[] payload, int chunkOffset, int declaredLen, String desc) + { + for (ChunkDecoder decoder : new ChunkDecoder[] { _safe, _optimal }) { + byte[] output = new byte[chunkOffset + declaredLen]; + Arrays.fill(output, (byte) 'P'); + assertNotNull(_decode(decoder, payload, chunkOffset, declaredLen, output), + decoder.getClass().getSimpleName()+" accepted invalid content: "+desc + +" (chunk offset "+chunkOffset+")"); + } + } + + /** + * @return Output produced by the decoders, which has to be identical for both + */ + private byte[] _assertBothSucceed(byte[] payload, int chunkOffset, int declaredLen, String desc) + { + byte[] result = null; + for (ChunkDecoder decoder : new ChunkDecoder[] { _safe, _optimal }) { + byte[] output = new byte[chunkOffset + declaredLen]; + Arrays.fill(output, (byte) 'P'); + LZFException failure = _decode(decoder, payload, chunkOffset, declaredLen, output); + assertNull(failure, decoder.getClass().getSimpleName()+" rejected valid content: "+desc + +" (chunk offset "+chunkOffset+")"); + if (result == null) { + result = output; + } else { + assertArrayEquals(result, output, "Decoders produced different output for "+desc); + } + } + return result; + } + + // Note: `decodeChunk(byte[], int, int, byte[], int, int)` does not use decoder state, + // so single instances can be reused for all cases + private final ChunkDecoder _safe = ChunkDecoderFactory.safeInstance(); + private final ChunkDecoder _optimal = ChunkDecoderFactory.optimalInstance(); + + private void _assertSameOutcome(byte[] payload, int chunkOffset, int declaredLen) + { + byte[] safeOutput = new byte[chunkOffset + declaredLen]; + byte[] optimalOutput = new byte[chunkOffset + declaredLen]; + // Prefill, so that content of an earlier chunk (or stale buffer content) is + // distinguishable from output actually produced for this chunk + Arrays.fill(safeOutput, (byte) 'P'); + Arrays.fill(optimalOutput, (byte) 'P'); + + LZFException safeFailure = _decode(_safe, payload, chunkOffset, declaredLen, safeOutput); + LZFException optimalFailure = _decode(_optimal, payload, chunkOffset, declaredLen, optimalOutput); + + if ((safeFailure == null) != (optimalFailure == null)) { + fail("Decoders disagree for "+_describe(payload, chunkOffset, declaredLen) + +": safe "+_outcome(safeFailure)+", optimal "+_outcome(optimalFailure)); + } + if (safeFailure == null) { + assertArrayEquals(safeOutput, optimalOutput, + "Decoders produced different output for "+_describe(payload, chunkOffset, declaredLen)); + } + } + + /** + * @return `LZFException` thrown by the decoder, if any; `null` if decoding succeeded. + * Fails the test for any other exception: arguments passed are valid, so content + * problems have to be reported as `LZFException` + */ + private LZFException _decode(ChunkDecoder decoder, byte[] payload, int chunkOffset, int declaredLen, + byte[] output) + { + try { + decoder.decodeChunk(payload, 0, payload.length, output, chunkOffset, chunkOffset + declaredLen); + return null; + } catch (LZFException e) { + return e; + } catch (RuntimeException e) { + fail(decoder.getClass().getSimpleName()+" threw "+e.getClass().getName() + +" instead of LZFException for "+_describe(payload, chunkOffset, declaredLen), e); + return null; // never gets here + } + } + + private String _outcome(LZFException e) { + return (e == null) ? "succeeded" : "failed ("+e.getMessage()+")"; + } + + private String _describe(byte[] payload, int chunkOffset, int declaredLen) { + StringBuilder sb = new StringBuilder("payload 0x"); + for (byte b : payload) { + sb.append(String.format("%02x", b)); + } + return sb.append(", chunk offset ").append(chunkOffset) + .append(", declared uncompressed length ").append(declaredLen).toString(); + } +} diff --git a/src/test/java/com/ning/compress/lzf/TestLZFInputStream.java b/src/test/java/com/ning/compress/lzf/TestLZFInputStream.java index 31e1332..e608582 100644 --- a/src/test/java/com/ning/compress/lzf/TestLZFInputStream.java +++ b/src/test/java/com/ning/compress/lzf/TestLZFInputStream.java @@ -6,6 +6,7 @@ import java.security.SecureRandom; import com.ning.compress.BaseForTests; +import com.ning.compress.lzf.util.ChunkDecoderFactory; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; @@ -227,6 +228,70 @@ private void doDecompressReadByte(byte[] bytes, byte[] reference) throws IOExcep is.close(); } + /** + * Verifies that malformed compressed content is reported as {@link LZFException} when read + * through the stream, and never as an unchecked exception; and that the "safe" and "optimal" + * decoders agree on which content is malformed. Covers all 256 possible control bytes, + * since the stream path reaches decoder methods that the block API does not. + */ + @Test + public void testMalformedContentReporting() + { + ChunkDecoder safe = ChunkDecoderFactory.safeInstance(); + ChunkDecoder optimal = ChunkDecoderFactory.optimalInstance(); + + for (int ctrl = 0; ctrl < 256; ++ctrl) { + for (byte[] tail : new byte[][] { {}, { 0x00 }, { 0x00, 0x41 }, { 0x41, 0x42, 0x43 } }) { + byte[] payload = new byte[1 + tail.length]; + payload[0] = (byte) ctrl; + System.arraycopy(tail, 0, payload, 1, tail.length); + + for (int uncompLen : new int[] { 1, 3, 9, 40 }) { + byte[] input = _asSingleChunk(payload, uncompLen); + String desc = "control byte 0x"+String.format("%02x", ctrl) + +", compressed length "+payload.length + +", declared uncompressed length "+uncompLen; + assertEquals(_readOutcome(safe, input, desc), _readOutcome(optimal, input, desc), + "Decoders disagree for "+desc); + } + } + } + } + + /** + * @return Description of the outcome of reading given content: either the number of bytes + * read, or the fact that `LZFException` was thrown. Fails the test for any other + * exception: content problems have to be reported as `LZFException` + */ + private String _readOutcome(ChunkDecoder decoder, byte[] input, String desc) + { + try (LZFInputStream in = new LZFInputStream(decoder, new ByteArrayInputStream(input))) { + byte[] buffer = new byte[64]; + int total = 0, count; + while ((count = in.read(buffer)) != -1) { + total += count; + } + return "read "+total+" bytes"; + } catch (LZFException e) { + return "LZFException"; + } catch (IOException e) { + fail(decoder.getClass().getSimpleName()+" threw "+e.getClass().getName() + +" instead of LZFException for "+desc, e); + } catch (RuntimeException e) { + fail(decoder.getClass().getSimpleName()+" threw "+e.getClass().getName() + +" instead of LZFException for "+desc, e); + } + return null; // never gets here + } + + private byte[] _asSingleChunk(byte[] payload, int uncompLen) + { + byte[] chunk = new byte[LZFChunk.HEADER_LEN_COMPRESSED + payload.length]; + LZFChunk.appendCompressedHeader(uncompLen, payload.length, chunk, 0); + System.arraycopy(payload, 0, chunk, LZFChunk.HEADER_LEN_COMPRESSED, payload.length); + return chunk; + } + private void doDecompressReadBlock(byte[] bytes, byte[] reference) throws IOException { ByteArrayInputStream bis = new ByteArrayInputStream(bytes);