Skip to content

decompress: reject output overrunning split literals and oversized blocks #4899

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
terrelln wants to merge 1 commit into
base: dev
Choose a base branch
from
Open

decompress: reject output overrunning split literals and oversized blocks #4899

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions lib/decompress/zstd_decompress.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1028,6 +1028,7 @@ static size_t ZSTD_decompressFrame(ZSTD_DCtx* dctx,
RETURN_ERROR(corruption_detected, "invalid block type");
}
FORWARD_IF_ERROR(decodedSize, "Block decompression failure");
RETURN_ERROR_IF(decodedSize > dctx->fParams.blockSizeMax, corruption_detected, "Decompressed Block Size Exceeds Maximum");
DEBUGLOG(5, "Decompressed block of dSize = %u", (unsigned)decodedSize);
if (dctx->validateChecksum) {
XXH64_update(&dctx->xxhState, op, decodedSize);
Expand Down
10 changes: 8 additions & 2 deletions lib/decompress/zstd_decompress_block.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -892,7 +892,10 @@ static void ZSTD_safecopyDstBeforeSrc(BYTE* op, const BYTE* ip, size_t length)
return;
}

if (op <= oend - WILDCOPY_OVERLENGTH && diff < -WILDCOPY_VECLEN) {
/* ZSTD_wildcopy() may write up to WILDCOPY_OVERLENGTH-1 bytes past its length.
* Those bytes must not land on source bytes the leftover loop has yet to read,
* so src must be at least WILDCOPY_OVERLENGTH bytes ahead of dst. */
if (op <= oend - WILDCOPY_OVERLENGTH && diff <= -WILDCOPY_OVERLENGTH) {
ZSTD_wildcopy(op, ip, (size_t)(oend - WILDCOPY_OVERLENGTH - op), ZSTD_no_overlap);
ip += oend - WILDCOPY_OVERLENGTH - op;
op += oend - WILDCOPY_OVERLENGTH - op;
Expand Down Expand Up @@ -977,8 +980,11 @@ size_t ZSTD_execSequenceEndSplitLitBuffer(BYTE* op,
assert(op < op + sequenceLength);
assert(oLitEnd < op + sequenceLength);

/* The literals still to be read live in dst at [*litPtr, litLimit).
* The output of this sequence must not run past the literals it consumes. */
RETURN_ERROR_IF(op + sequenceLength > iLitEnd, dstSize_tooSmall, "output should not catch up to and overwrite literal buffer");

/* copy literals */
RETURN_ERROR_IF(op > *litPtr && op < *litPtr + sequence.litLength, dstSize_tooSmall, "output should not catch up to and overwrite literal buffer");
ZSTD_safecopyDstBeforeSrc(op, *litPtr, sequence.litLength);
op = oLitEnd;
*litPtr = iLitEnd;
Expand Down
155 changes: 154 additions & 1 deletion tests/fuzzer.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -575,6 +575,126 @@ static void test_decompressBound(unsigned tnb)
DISPLAYLEVEL(3, "OK \n");
}

/* Size of the literals of the block built by FUZ_writeSplitLitBlock().
* It is larger than the decoder's 64 KB extra literal buffer, so the first
* kSplitLitInDst literals are stored at the end of the block's area in dst. */
#define kSplitLitSize (64 KB + 112)
#define kSplitLitInDst (kSplitLitSize - 64 KB)
#define kSplitLitBlockMaxSize (kSplitLitSize + 3 + 9)

/* Writes a compressed block (without block header) with raw literals and 2 sequences,
* which decodes to 48 literals, a match of 65539+seq1MLExtra bytes at offset 1,
* 49 literals, a match of 3 bytes at offset 1, and the remaining literals.
* The block decodes to more than ZSTD_BLOCKSIZE_MAX bytes, so the first match runs
* up to (or past) the literals that are stored in dst.
* Writes the expected output to expected, which must hold
* FUZ_splitLitBlockDecodedSize(seq1MLExtra) bytes.
* @return : size of the block */
static size_t FUZ_writeSplitLitBlock(BYTE* block, BYTE* expected, U32 seq1MLExtra)
{
U32 const seq1ML = 65539 + seq1MLExtra;
BYTE* op = block;
size_t i;

/* Literals section header: raw literals, 3 byte header */
MEM_writeLE24(op, (U32)(kSplitLitSize << 4) | (3 << 2));
op += 3;
for (i = 0; i < kSplitLitSize; i++) {
*op++ = i < kSplitLitInDst ? (BYTE)(i + 1) : 0xCC;
}

/* Sequences section header: 2 sequences, LL & OF RLE, ML predefined.
* LL code 24 (base 48, 4 extra bits), OF code 0 (repcode 1 = offset 1). */
*op++ = 2;
*op++ = 0x50;
*op++ = 24;
*op++ = 0;

/* Bitstream, read from the top down: ML initial state 57 (ML code 52: base 65539,
* 16 extra bits), sequence 1 ML extra bits, LL extra bits 0 (LL 48),
* ML state update to state 0 (ML 3), sequence 2 LL extra bits 1 (LL 49). */
{ U64 const bitStream = (1ULL << 36) | (57ULL << 30) | ((U64)seq1MLExtra << 14) | 1;
MEM_writeLE32(op, (U32)bitStream);
op[4] = (BYTE)(bitStream >> 32);
op += 5;
}

{ BYTE* eop = expected;
memcpy(eop, block + 3, 48);
eop += 48;
memset(eop, 48, seq1ML);
eop += seq1ML;
memcpy(eop, block + 3 + 48, 49);
eop += 49;
memset(eop, 97, 3);
eop += 3;
memcpy(eop, block + 3 + 97, kSplitLitSize - 97);
}

assert((size_t)(op - block) <= kSplitLitBlockMaxSize);
return (size_t)(op - block);
}

static size_t FUZ_splitLitBlockDecodedSize(U32 seq1MLExtra)
{
return kSplitLitSize + 65539 + seq1MLExtra + 3;
}

static void test_decompressSplitLitBuffer(unsigned tnb)
{
/* The literals are split, rather than stored after the block's output, because
* dstCapacity <= ZSTD_BLOCKSIZE_MAX + litSize + 2 * WILDCOPY_OVERLENGTH. */
size_t const dstCapacity = ZSTD_BLOCKSIZE_MAX + kSplitLitSize + 64;
size_t const srcCapacity = ZSTD_FRAMEHEADERSIZE_MAX + ZSTD_blockHeaderSize + kSplitLitBlockMaxSize;
/* seq1MLExtra = 65371 : The first match ends 18 bytes before the next literals,
* so the literals are copied to an overlapping position.
* seq1MLExtra = 65460 : The first match overwrites the next literals. */
U32 const seq1MLExtras[] = { 65371, 65460 };
BYTE* const src = (BYTE*)malloc(srcCapacity);
BYTE* const dst = (BYTE*)malloc(dstCapacity);
BYTE* const expected = (BYTE*)malloc(dstCapacity);
ZSTD_DCtx* const dctx = ZSTD_createDCtx();
size_t i;
int anySucceeded = 0;
CHECK(src != NULL && dst != NULL && expected != NULL && dctx != NULL);

DISPLAYLEVEL(3, "test%3u : decompress block with split literals overrun by output : ", tnb);
for (i = 0; i < COUNTOF(seq1MLExtras); i++) {
size_t const decodedSize = FUZ_splitLitBlockDecodedSize(seq1MLExtras[i]);
size_t blockSize;
size_t r;
CHECK(decodedSize <= dstCapacity);

/* The block API has no ZSTD_BLOCKSIZE_MAX limit on the decoded size,
* so the decoder must either reject the block or decode it correctly. */
blockSize = FUZ_writeSplitLitBlock(src, expected, seq1MLExtras[i]);
CHECK_Z( ZSTD_decompressBegin(dctx) );
r = ZSTD_decompressBlock(dctx, dst, dstCapacity, src, blockSize);
if (!ZSTD_isError(r)) {
CHECK_EQ(r, decodedSize);
CHECK(!memcmp(dst, expected, decodedSize));
anySucceeded = 1;
}

/* Wrap the block in a frame with windowLog 17, so blockSizeMax = ZSTD_BLOCKSIZE_MAX.
* The frame must be rejected, because the block decodes to more than blockSizeMax. */
MEM_writeLE32(src, ZSTD_MAGICNUMBER);
src[4] = 0x00; /* Frame header descriptor: no content size, no checksum, no dictID */
src[5] = 0x38; /* Window descriptor: windowLog 17 */
blockSize = FUZ_writeSplitLitBlock(src + 9, expected, seq1MLExtras[i]);
MEM_writeLE24(src + 6, 1 /* last block */ | ((U32)bt_compressed << 1) | (U32)(blockSize << 3));
r = ZSTD_decompress(dst, dstCapacity, src, 9 + blockSize);
CHECK(ZSTD_isError(r));
}
CHECK(anySucceeded); /* Must not reject all blocks */
DISPLAYLEVEL(3, "OK \n");

ZSTD_freeDCtx(dctx);
free(expected);
free(dst);
free(src);
}

static unsigned test_histCountWksp(unsigned seed, unsigned testNb)
{
static const unsigned symLowLimits[] = { 0, 27, 0, 0, 27, 42, 0, 0, 27, 42, 27, 42 };
Expand Down Expand Up @@ -1026,6 +1146,8 @@ static int basicUnitTests(U32 const seed, double compressibility)

test_decompressBound(testNb++);

test_decompressSplitLitBuffer(testNb++);

test_setCParams(testNb++);

DISPLAYLEVEL(3, "test%3u : ZSTD_adjustCParams : ", testNb++);
Expand Down Expand Up @@ -1372,7 +1494,7 @@ static int basicUnitTests(U32 const seed, double compressibility)
ZSTD_freeCCtx(cctx);
}

DISPLAYLEVEL(3, "test%3i : maxBlockSize = 2K", testNb++);
DISPLAYLEVEL(3, "test%3i : maxBlockSize = 2K : ", testNb++);
{
ZSTD_CCtx* cctx = ZSTD_createCCtx();
ZSTD_DCtx* dctx = ZSTD_createDCtx();
Expand All @@ -1390,6 +1512,37 @@ static int basicUnitTests(U32 const seed, double compressibility)
ZSTD_freeDCtx(dctx);
ZSTD_freeCCtx(cctx);
}
DISPLAYLEVEL(3, "OK \n");

DISPLAYLEVEL(3, "test%3i : maxBlockSize rejects raw & RLE blocks : ", testNb++);
{
ZSTD_CCtx* cctx = ZSTD_createCCtx();
ZSTD_DCtx* dctx = ZSTD_createDCtx();
size_t const srcSize = 8 KB;
BYTE* const src = (BYTE*)malloc(srcSize);
int i;
CHECK(src != NULL);
CHECK_Z(ZSTD_CCtx_setParameter(cctx, ZSTD_c_maxBlockSize, 2048));
CHECK_Z(ZSTD_DCtx_setParameter(dctx, ZSTD_d_maxBlockSize, 1024));

/* i = 0 : incompressible data, which is stored in raw blocks
* i = 1 : a single repeated byte, which is stored in RLE blocks */
for (i = 0; i < 2; i++) {
if (i == 0) {
RDG_genBuffer(src, srcSize, 0.0, 0.0, seed);
} else {
memset(src, 'a', srcSize);
}
cSize = ZSTD_compress2(cctx, compressedBuffer, compressedBufferSize, src, srcSize);
CHECK_Z(cSize);
CHECK(ZSTD_isError(ZSTD_decompressDCtx(dctx, decodedBuffer, CNBuffSize, compressedBuffer, cSize)));
}

free(src);
ZSTD_freeDCtx(dctx);
ZSTD_freeCCtx(cctx);
}
DISPLAYLEVEL(3, "OK \n");

DISPLAYLEVEL(3, "test%3i : ldm fill dict out-of-bounds check", testNb++);
{
Expand Down
Loading