From aa385ece13b0a847380303a3918609213eea45e0 Mon Sep 17 00:00:00 2001 From: Danielle Rozenblit Date: Fri, 20 Jan 2023 10:32:35 -0800 Subject: [PATCH] fix sequence validation and bounds check in ZSTD_copySequencesToSeqStore() --- lib/compress/zstd_compress.c | 18 ++++---- tests/zstreamtest.c | 84 ++++++++++++++++++++++++++++++++++++ 2 files changed, 94 insertions(+), 8 deletions(-) diff --git a/lib/compress/zstd_compress.c b/lib/compress/zstd_compress.c index 3a48e7dcd..c7a573371 100644 --- a/lib/compress/zstd_compress.c +++ b/lib/compress/zstd_compress.c @@ -6156,8 +6156,8 @@ size_t ZSTD_compress2(ZSTD_CCtx* cctx, * @returns a ZSTD error code if sequence is not valid */ static size_t -ZSTD_validateSequence(U32 offCode, U32 matchLength, - size_t posInSrc, U32 windowLog, size_t dictSize) +ZSTD_validateSequence(U32 offCode, U32 matchLength, U32 minMatch, + size_t posInSrc, U32 windowLog, size_t dictSize, int useExternalMatchFinder) { U32 const windowSize = 1u << windowLog; /* posInSrc represents the amount of data the decoder would decode up to this point. @@ -6168,6 +6168,8 @@ ZSTD_validateSequence(U32 offCode, U32 matchLength, size_t const offsetBound = posInSrc > windowSize ? (size_t)windowSize : posInSrc + (size_t)dictSize; RETURN_ERROR_IF(offCode > OFFSET_TO_OFFBASE(offsetBound), corruption_detected, "Offset too large!"); RETURN_ERROR_IF(matchLength < MINMATCH, corruption_detected, "Matchlength too small"); + /* Validate maxNbSeq is large enough for the given matchLength and minMatch */ + RETURN_ERROR_IF(!useExternalMatchFinder && minMatch >= 4 && matchLength < 4, corruption_detected, "Matchlength too small for the minMatch"); return 0; } @@ -6220,11 +6222,11 @@ ZSTD_copySequencesToSeqStoreExplicitBlockDelim(ZSTD_CCtx* cctx, DEBUGLOG(6, "Storing sequence: (of: %u, ml: %u, ll: %u)", offBase, matchLength, litLength); if (cctx->appliedParams.validateSequences) { seqPos->posInSrc += litLength + matchLength; - FORWARD_IF_ERROR(ZSTD_validateSequence(offBase, matchLength, seqPos->posInSrc, - cctx->appliedParams.cParams.windowLog, dictSize), + FORWARD_IF_ERROR(ZSTD_validateSequence(offBase, matchLength, cctx->appliedParams.cParams.minMatch, seqPos->posInSrc, + cctx->appliedParams.cParams.windowLog, dictSize, cctx->appliedParams.useExternalMatchFinder), "Sequence validation failed"); } - RETURN_ERROR_IF(idx - seqPos->idx > cctx->seqStore.maxNbSeq, memory_allocation, + RETURN_ERROR_IF(idx - seqPos->idx >= cctx->seqStore.maxNbSeq, memory_allocation, "Not enough memory allocated. Try adjusting ZSTD_c_minMatch."); ZSTD_storeSeq(&cctx->seqStore, litLength, ip, iend, offBase, matchLength); ip += matchLength + litLength; @@ -6332,12 +6334,12 @@ ZSTD_copySequencesToSeqStoreNoBlockDelim(ZSTD_CCtx* cctx, ZSTD_sequencePosition* if (cctx->appliedParams.validateSequences) { seqPos->posInSrc += litLength + matchLength; - FORWARD_IF_ERROR(ZSTD_validateSequence(offBase, matchLength, seqPos->posInSrc, - cctx->appliedParams.cParams.windowLog, dictSize), + FORWARD_IF_ERROR(ZSTD_validateSequence(offBase, matchLength, cctx->appliedParams.cParams.minMatch, seqPos->posInSrc, + cctx->appliedParams.cParams.windowLog, dictSize, cctx->appliedParams.useExternalMatchFinder), "Sequence validation failed"); } DEBUGLOG(6, "Storing sequence: (of: %u, ml: %u, ll: %u)", offBase, matchLength, litLength); - RETURN_ERROR_IF(idx - seqPos->idx > cctx->seqStore.maxNbSeq, memory_allocation, + RETURN_ERROR_IF(idx - seqPos->idx >= cctx->seqStore.maxNbSeq, memory_allocation, "Not enough memory allocated. Try adjusting ZSTD_c_minMatch."); ZSTD_storeSeq(&cctx->seqStore, litLength, ip, iend, offBase, matchLength); ip += matchLength + litLength; diff --git a/tests/zstreamtest.c b/tests/zstreamtest.c index 4a621692d..cb5f7fea7 100644 --- a/tests/zstreamtest.c +++ b/tests/zstreamtest.c @@ -2066,6 +2066,90 @@ static int basicUnitTests(U32 seed, double compressibility, int bigTests) } DISPLAYLEVEL(3, "OK \n"); + /* Test Sequence Validation */ + DISPLAYLEVEL(3, "test%3i : Testing sequence validation: ", testNb++); + { + ZSTD_CCtx* cctx = ZSTD_createCCtx(); + + /* Test minMatch >= 4, matchLength < 4 */ + { + size_t srcSize = 11; + void* const src = CNBuffer; + size_t dstSize = ZSTD_compressBound(srcSize); + void* const dst = compressedBuffer; + size_t const kNbSequences = 4; + ZSTD_Sequence* sequences = malloc(sizeof(ZSTD_Sequence) * kNbSequences); + + memset(src, 'x', srcSize); + + sequences[0] = (ZSTD_Sequence) {1, 1, 3, 0}; + sequences[1] = (ZSTD_Sequence) {1, 0, 3, 0}; + sequences[2] = (ZSTD_Sequence) {1, 0, 3, 0}; + sequences[3] = (ZSTD_Sequence) {0, 1, 0, 0}; + + /* Test with sequence validation */ + CHECK_Z(ZSTD_CCtx_setParameter(cctx, ZSTD_c_minMatch, 5)); + CHECK_Z(ZSTD_CCtx_setParameter(cctx, ZSTD_c_blockDelimiters, ZSTD_sf_explicitBlockDelimiters)); + CHECK_Z(ZSTD_CCtx_setParameter(cctx, ZSTD_c_validateSequences, 1)); + + cSize = ZSTD_compressSequences(cctx, dst, dstSize, + sequences, kNbSequences, + src, srcSize); + + CHECK(!ZSTD_isError(cSize), "Should throw an error"); /* maxNbSeq is too small and an assert will fail */ + + ZSTD_CCtx_reset(cctx, ZSTD_reset_session_and_parameters); + + /* Test without sequence validation */ + CHECK_Z(ZSTD_CCtx_setParameter(cctx, ZSTD_c_minMatch, 5)); + CHECK_Z(ZSTD_CCtx_setParameter(cctx, ZSTD_c_blockDelimiters, ZSTD_sf_explicitBlockDelimiters)); + CHECK_Z(ZSTD_CCtx_setParameter(cctx, ZSTD_c_validateSequences, 0)); + + cSize = ZSTD_compressSequences(cctx, dst, dstSize, + sequences, kNbSequences, + src, srcSize); + + CHECK(!ZSTD_isError(cSize), "Should throw an error"); /* maxNbSeq is too small and an assert will fail */ + + free(sequences); + } + + ZSTD_CCtx_reset(cctx, ZSTD_reset_session_and_parameters); + + { /* Test case with two additional sequences */ + size_t srcSize = 19; + void* const src = CNBuffer; + size_t dstSize = ZSTD_compressBound(srcSize); + void* const dst = compressedBuffer; + size_t const kNbSequences = 7; + ZSTD_Sequence* sequences = malloc(sizeof(ZSTD_Sequence) * kNbSequences); + + memset(src, 'x', srcSize); + + sequences[0] = (ZSTD_Sequence) {1, 1, 3, 0}; + sequences[1] = (ZSTD_Sequence) {1, 0, 3, 0}; + sequences[2] = (ZSTD_Sequence) {1, 0, 3, 0}; + sequences[3] = (ZSTD_Sequence) {1, 0, 3, 0}; + sequences[4] = (ZSTD_Sequence) {1, 0, 3, 0}; + sequences[5] = (ZSTD_Sequence) {1, 0, 3, 0}; + sequences[6] = (ZSTD_Sequence) {0, 0, 0, 0}; + + CHECK_Z(ZSTD_CCtx_setParameter(cctx, ZSTD_c_minMatch, 5)); + CHECK_Z(ZSTD_CCtx_setParameter(cctx, ZSTD_c_blockDelimiters, ZSTD_sf_explicitBlockDelimiters)); + CHECK_Z(ZSTD_CCtx_setParameter(cctx, ZSTD_c_validateSequences, 1)); + + cSize = ZSTD_compressSequences(cctx, dst, dstSize, + sequences, kNbSequences, + src, srcSize); + + CHECK(!ZSTD_isError(cSize), "Should throw an error"); /* maxNbSeq is too small and an assert will fail */ + + free(sequences); + } + ZSTD_freeCCtx(cctx); + } + DISPLAYLEVEL(3, "OK \n"); + _end: FUZ_freeDictionary(dictionary); ZSTD_freeCStream(zc);