Skip to content

Commit

Permalink
Fix out of bounds read due to negative length
Browse files Browse the repository at this point in the history
If the computed literal or match length was greater than
Integer.MAX_VALUE, the value would overflow to a negative
and cause a read before the beginning of the buffer.
  • Loading branch information
martint committed Feb 13, 2024
1 parent 99de82a commit 15e68df
Show file tree
Hide file tree
Showing 8 changed files with 224 additions and 0 deletions.
6 changes: 6 additions & 0 deletions src/main/java/io/airlift/compress/lz4/Lz4RawDecompressor.java
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,9 @@ public static int decompress(
}
while (value == 255 && input < inputLimit - 15);
}
if (literalLength < 0) {
throw new MalformedInputException(input - inputAddress);
}

// copy literal
long literalEnd = input + literalLength;
Expand Down Expand Up @@ -127,6 +130,9 @@ public static int decompress(
while (value == 255);
}
matchLength += MIN_MATCH; // implicit length from initial 4-byte match in encoder
if (matchLength < 0) {
throw new MalformedInputException(input - inputAddress);
}

long matchOutputLimit = output + matchLength;

Expand Down
7 changes: 7 additions & 0 deletions src/main/java/io/airlift/compress/lzo/LzoRawDecompressor.java
Original file line number Diff line number Diff line change
Expand Up @@ -248,6 +248,10 @@ else if ((command & 0b1100_0000) != 0) {
}
firstCommand = false;

if (matchLength < 0) {
throw new MalformedInputException(input - inputAddress);
}

// copy match
if (matchLength != 0) {
// lzo encodes match offset minus one
Expand Down Expand Up @@ -316,6 +320,9 @@ else if ((command & 0b1100_0000) != 0) {
}

// copy literal
if (literalLength < 0) {
throw new MalformedInputException(input - inputAddress);
}
long literalOutputLimit = output + literalLength;
if (literalOutputLimit > fastOutputLimit || input + literalLength > inputLimit - SIZE_OF_LONG) {
if (literalOutputLimit > outputLimit) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,9 @@ private static int uncompressAll(

if ((opCode & 0x3) == LITERAL) {
int literalLength = length + trailer;
if (literalLength < 0) {
throw new MalformedInputException(input - inputAddress);
}

// copy literal
long literalOutputLimit = output + literalLength;
Expand Down Expand Up @@ -147,6 +150,9 @@ private static int uncompressAll(
// bit 8).
int matchOffset = entry & 0x700;
matchOffset += trailer;
if (matchOffset < 0) {
throw new MalformedInputException(input - inputAddress);
}

long matchAddress = output - matchOffset;
if (matchAddress < outputAddress || output + length > outputLimit) {
Expand Down
2 changes: 2 additions & 0 deletions src/main/java/io/airlift/compress/zstd/Huffman.java
Original file line number Diff line number Diff line change
Expand Up @@ -172,6 +172,8 @@ public void decode4Streams(final Object inputBase, final long inputAddress, fina
long start3 = start2 + (UNSAFE.getShort(inputBase, inputAddress + 2) & 0xFFFF);
long start4 = start3 + (UNSAFE.getShort(inputBase, inputAddress + 4) & 0xFFFF);

verify(start2 < start3 && start3 < start4 && start4 < inputLimit, inputAddress, "Input is corrupted");

BitInputStream.Initializer initializer = new BitInputStream.Initializer(inputBase, start1, start2);
initializer.initialize();
int stream1bitsConsumed = initializer.getBitsConsumed();
Expand Down
49 changes: 49 additions & 0 deletions src/test/java/io/airlift/compress/lz4/TestLz4.java
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,17 @@
import io.airlift.compress.AbstractTestCompression;
import io.airlift.compress.Compressor;
import io.airlift.compress.Decompressor;
import io.airlift.compress.MalformedInputException;
import io.airlift.compress.thirdparty.JPountzLz4Compressor;
import io.airlift.compress.thirdparty.JPountzLz4Decompressor;
import net.jpountz.lz4.LZ4Factory;
import org.testng.annotations.Test;

import java.io.ByteArrayOutputStream;
import java.io.IOException;
import java.util.Arrays;

import static org.assertj.core.api.Assertions.assertThatThrownBy;

public class TestLz4
extends AbstractTestCompression
Expand Down Expand Up @@ -46,4 +54,45 @@ protected Decompressor getVerifyDecompressor()
{
return new JPountzLz4Decompressor(LZ4Factory.fastestInstance());
}

@Test
public void testLiteralLengthOverflow()
throws IOException
{
ByteArrayOutputStream buffer = new ByteArrayOutputStream();
buffer.write((byte) 0b1111_0000); // token
// Causes overflow for `literalLength`
byte[] literalLengthBytes = new byte[Integer.MAX_VALUE / 255 + 1]; // ~9MB
Arrays.fill(literalLengthBytes, (byte) 255);
buffer.write(literalLengthBytes);
buffer.write(1);
buffer.write(new byte[20]);

byte[] data = buffer.toByteArray();

assertThatThrownBy(() -> new Lz4Decompressor().decompress(data, 0, data.length, new byte[2048], 0, 2048))
.isInstanceOf(MalformedInputException.class);
}

@Test
public void testMatchLengthOverflow()
throws IOException
{
ByteArrayOutputStream buffer = new ByteArrayOutputStream();
buffer.write((byte) 0b0000_1111); // token
buffer.write(new byte[2]); // offset

// Causes overflow for `matchLength`
byte[] literalLengthBytes = new byte[Integer.MAX_VALUE / 255 + 1]; // ~9MB
Arrays.fill(literalLengthBytes, (byte) 255);
buffer.write(literalLengthBytes);
buffer.write(1);

buffer.write(new byte[10]);

byte[] data = buffer.toByteArray();

assertThatThrownBy(() -> new Lz4Decompressor().decompress(data, 0, data.length, new byte[2048], 0, 2048))
.isInstanceOf(MalformedInputException.class);
}
}
81 changes: 81 additions & 0 deletions src/test/java/io/airlift/compress/lzo/TestLzo.java
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,15 @@
import io.airlift.compress.Compressor;
import io.airlift.compress.Decompressor;
import io.airlift.compress.HadoopNative;
import io.airlift.compress.MalformedInputException;
import io.airlift.compress.thirdparty.HadoopLzoCompressor;
import io.airlift.compress.thirdparty.HadoopLzoDecompressor;
import org.testng.annotations.Test;

import java.io.ByteArrayOutputStream;
import java.io.IOException;

import static org.assertj.core.api.Assertions.assertThatThrownBy;

public class TestLzo
extends AbstractTestCompression
Expand Down Expand Up @@ -50,4 +57,78 @@ protected Decompressor getVerifyDecompressor()
{
return new HadoopLzoDecompressor();
}

@Test
public void testLiteralLengthOverflow()
throws IOException
{
ByteArrayOutputStream buffer = new ByteArrayOutputStream();
// Command
buffer.write(0);
// Causes overflow for `literalLength`
buffer.write(new byte[Integer.MAX_VALUE / 255 + 1]); // ~9MB
buffer.write(1);

byte[] data = buffer.toByteArray();

assertThatThrownBy(() -> new LzoDecompressor().decompress(data, 0, data.length, new byte[20000], 0, 20000))
.isInstanceOf(MalformedInputException.class);
}

@Test
public void testMatchLengthOverflow1()
throws IOException
{
ByteArrayOutputStream buffer = new ByteArrayOutputStream();
// Write some data so that `matchOffset` validation later passes
// Command
buffer.write(0);
buffer.write(new byte[66]);
buffer.write(8);
buffer.write(new byte[2107 * 8]);

// Command
buffer.write(0b001_0000);
// Causes overflow for `matchLength`
buffer.write(new byte[Integer.MAX_VALUE / 255 + 1]); // ~9MB
buffer.write(1);
// Trailer
buffer.write(0b0000_0000);
buffer.write(0b0000_0100);

buffer.write(new byte[10]);

byte[] data = buffer.toByteArray();

assertThatThrownBy(() -> new LzoDecompressor().decompress(data, 0, data.length, new byte[20000], 0, 20000))
.isInstanceOf(MalformedInputException.class);
}

@Test
public void testMatchLengthOverflow2()
throws IOException
{
ByteArrayOutputStream buffer = new ByteArrayOutputStream();
// Write some data so that `matchOffset` validation later passes
// Command
buffer.write(0);
buffer.write(246);
buffer.write(new byte[264]);

// Command
buffer.write(0b0010_0000);
// Causes overflow for `matchLength`
buffer.write(new byte[Integer.MAX_VALUE / 255 + 1]); // ~9MB
buffer.write(1);
// Trailer
buffer.write(0b0000_0000);
buffer.write(0b0000_0100);

buffer.write(new byte[10]);

byte[] data = buffer.toByteArray();

assertThatThrownBy(() -> new LzoDecompressor().decompress(data, 0, data.length, new byte[20000], 0, 20000))
.isInstanceOf(MalformedInputException.class);
}
}
22 changes: 22 additions & 0 deletions src/test/java/io/airlift/compress/snappy/TestSnappy.java
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,12 @@
import io.airlift.compress.AbstractTestCompression;
import io.airlift.compress.Compressor;
import io.airlift.compress.Decompressor;
import io.airlift.compress.MalformedInputException;
import io.airlift.compress.thirdparty.XerialSnappyCompressor;
import io.airlift.compress.thirdparty.XerialSnappyDecompressor;
import org.testng.annotations.Test;

import static org.assertj.core.api.Assertions.assertThatThrownBy;

public class TestSnappy
extends AbstractTestCompression
Expand Down Expand Up @@ -45,4 +49,22 @@ protected Decompressor getVerifyDecompressor()
{
return new XerialSnappyDecompressor();
}

@Test
public void testInvalidLiteralLength()
{
byte[] data = {
// Encoded uncompressed length 1024
-128, 8,
// op-code
(byte) 252,
// Trailer value Integer.MAX_VALUE
(byte) 0b1111_1111, (byte) 0b1111_1111, (byte) 0b1111_1111, (byte) 0b0111_1111,
// Some arbitrary data
0, 0, 0, 0, 0, 0, 0, 0
};

assertThatThrownBy(() -> new SnappyDecompressor().decompress(data, 0, data.length, new byte[1024], 0, 1024))
.isInstanceOf(MalformedInputException.class);
}
}
51 changes: 51 additions & 0 deletions src/test/java/io/airlift/compress/zstd/TestZstd.java
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
import io.airlift.compress.thirdparty.ZstdJniDecompressor;
import org.testng.annotations.Test;

import java.io.ByteArrayOutputStream;
import java.io.IOException;
import java.io.UncheckedIOException;
import java.util.Arrays;
Expand Down Expand Up @@ -209,4 +210,54 @@ public void testDecompressIsMissingData()
.matches(e -> e instanceof MalformedInputException || e instanceof UncheckedIOException)
.hasMessageContaining("Not enough input bytes");
}

@Test
public void testBadHuffmanData()
throws IOException
{
ByteArrayOutputStream buffer = new ByteArrayOutputStream();
// Magic
buffer.write(new byte[] {
(byte) 0b0010_1000,
(byte) 0b1011_0101,
(byte) 0b0010_1111,
(byte) 0b1111_1101,
});
// Frame header
buffer.write(0);
buffer.write(0);
// Block header COMPRESSED_BLOCK
buffer.write(new byte[] {
(byte) 0b1111_0100,
(byte) 0b0000_0000,
(byte) 0b0000_0000,
});
// Literals header
buffer.write(new byte[] {
// literalsBlockType COMPRESSED_LITERALS_BLOCK
// + literals type
0b0000_1010,
// ... header remainder
0b0000_0000,
// compressedSize
0b0011_1100,
0b0000_0000,
});
// Huffman inputSize
buffer.write(128);
// weight value
buffer.write(0b0001_0000);
// Bad start values
buffer.write(new byte[] {(byte) 255, (byte) 255});
buffer.write(new byte[] {(byte) 255, (byte) 255});
buffer.write(new byte[] {(byte) 255, (byte) 255});

buffer.write(new byte[10]);

byte[] data = buffer.toByteArray();

assertThatThrownBy(() -> new ZstdDecompressor().decompress(data, 0, data.length, new byte[10], 0, 10))
.isInstanceOf(MalformedInputException.class)
.hasMessageStartingWith("Input is corrupted");
}
}

0 comments on commit 15e68df

Please sign in to comment.