Skip to content

Commit

Permalink
Merge pull request from GHSA-grg4-wf29-r9vv
Browse files Browse the repository at this point in the history
Motivation:

We should do the Bzip2 decoding in a streaming fashion and so ensure we propagate the buffer as soon as possible through the pipeline. This allows the users to release these buffers as fast as possible.

Modification:

- Change the Bzip2Decoder to do the decompression of data in a streaming fashion.
- Add some safety check to ensure the block length never execeeds the maximum (as defined in the spec)

Result:

No more risk of an OOME by decompress some large data via bzip2.

Thanks to Ori Hollander of JFrog Security for reporting the issue.

(we got acquired during the process and now Vdoo is part of JFrog company)
  • Loading branch information
normanmaurer committed Sep 9, 2021
1 parent deb0489 commit 41d3d61
Show file tree
Hide file tree
Showing 3 changed files with 15 additions and 7 deletions.
Expand Up @@ -228,6 +228,11 @@ boolean decodeHuffmanData(final Bzip2HuffmanStageDecoder huffmanDecoder) {
bwtBlock[bwtBlockLength++] = nextByte;
}
}
if (bwtBlockLength > MAX_BLOCK_LENGTH) {
throw new DecompressionException("block length exceeds max block length: "
+ bwtBlockLength + " > " + MAX_BLOCK_LENGTH);
}

this.bwtBlockLength = bwtBlockLength;
initialiseInverseBWT();
return true;
Expand Down
Expand Up @@ -49,6 +49,8 @@ final class Bzip2Constants {
static final int MIN_BLOCK_SIZE = 1;
static final int MAX_BLOCK_SIZE = 9;

static final int MAX_BLOCK_LENGTH = MAX_BLOCK_SIZE * BASE_BLOCK_SIZE;

/**
* Maximum possible Huffman alphabet size.
*/
Expand Down
Expand Up @@ -291,26 +291,27 @@ protected void decode(ChannelHandlerContext ctx, ByteBuf in, List<Object> out) t
}

final int blockLength = blockDecompressor.blockLength();
final ByteBuf uncompressed = ctx.alloc().buffer(blockLength);
boolean success = false;
ByteBuf uncompressed = ctx.alloc().buffer(blockLength);
try {
int uncByte;
while ((uncByte = blockDecompressor.read()) >= 0) {
uncompressed.writeByte(uncByte);
}

// We did read all the data, lets reset the state and do the CRC check.
currentState = State.INIT_BLOCK;
int currentBlockCRC = blockDecompressor.checkCRC();
streamCRC = (streamCRC << 1 | streamCRC >>> 31) ^ currentBlockCRC;

out.add(uncompressed);
success = true;
uncompressed = null;
} finally {
if (!success) {
if (uncompressed != null) {
uncompressed.release();
}
}
currentState = State.INIT_BLOCK;
break;
// Return here so the ByteBuf that was put in the List will be forwarded to the user and so can be
// released as soon as possible.
return;
case EOF:
in.skipBytes(in.readableBytes());
return;
Expand Down

0 comments on commit 41d3d61

Please sign in to comment.