Skip to content
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.

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

Fix sequence validation and seqStore bounds check #3439

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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/common/error_private.c
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ const char* ERR_getErrorString(ERR_enum code)
case PREFIX(dstBuffer_wrong): return "Destination buffer is wrong";
case PREFIX(srcBuffer_wrong): return "Source buffer is wrong";
case PREFIX(externalMatchFinder_failed): return "External matchfinder returned an error code";
case PREFIX(externalSequences_invalid): return "External sequences are not valid";
case PREFIX(maxCode):
default: return notErrorCode;
}
Expand Down
22 changes: 12 additions & 10 deletions lib/compress/zstd_compress.c
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -6166,8 +6166,10 @@ ZSTD_validateSequence(U32 offCode, U32 matchLength,
* window size. After output surpasses windowSize, we're limited to windowSize offsets again.
*/
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");
size_t const matchLenLowerBound = (minMatch == 3 || useExternalMatchFinder) ? 3 : 4;
RETURN_ERROR_IF(offCode > OFFSET_TO_OFFBASE(offsetBound), externalSequences_invalid, "Offset too large!");
/* Validate maxNbSeq is large enough for the given matchLength and minMatch */
RETURN_ERROR_IF(matchLength < matchLenLowerBound, externalSequences_invalid, "Matchlength too small for the minMatch");
return 0;
}

Expand Down Expand Up @@ -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, externalSequences_invalid,
"Not enough memory allocated. Try adjusting ZSTD_c_minMatch.");
ZSTD_storeSeq(&cctx->seqStore, litLength, ip, iend, offBase, matchLength);
ip += matchLength + litLength;
Expand Down Expand Up @@ -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, externalSequences_invalid,
"Not enough memory allocated. Try adjusting ZSTD_c_minMatch.");
ZSTD_storeSeq(&cctx->seqStore, litLength, ip, iend, offBase, matchLength);
ip += matchLength + litLength;
Expand Down
1 change: 1 addition & 0 deletions lib/zstd_errors.h
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,7 @@ typedef enum {
ZSTD_error_dstBuffer_wrong = 104,
ZSTD_error_srcBuffer_wrong = 105,
ZSTD_error_externalMatchFinder_failed = 106,
ZSTD_error_externalSequences_invalid = 107,
ZSTD_error_maxCode = 120 /* never EVER use this value directly, it can change in future versions! Use ZSTD_isError() instead */
} ZSTD_ErrorCode;

Expand Down
101 changes: 101 additions & 0 deletions tests/zstreamtest.c
Original file line number Diff line number Diff line change
Expand Up @@ -2066,6 +2066,107 @@ 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 */
embg marked this conversation as resolved.
Show resolved Hide resolved
CHECK(ZSTD_getErrorCode(cSize) != ZSTD_error_externalSequences_invalid, "Wrong error code: %s", ZSTD_getErrorName(cSize)); /* fails sequence validation */

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 */
CHECK(ZSTD_getErrorCode(cSize) != ZSTD_error_externalSequences_invalid, "Wrong error code: %s", ZSTD_getErrorName(cSize)); /* fails sequence validation */

free(sequences);
}

ZSTD_CCtx_reset(cctx, ZSTD_reset_session_and_parameters);

{ /* Test case with two additional sequences */
daniellerozenblit marked this conversation as resolved.
Show resolved Hide resolved
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 */
CHECK(ZSTD_getErrorCode(cSize) != ZSTD_error_externalSequences_invalid, "Wrong error code: %s", ZSTD_getErrorName(cSize)); /* fails sequence validation */

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 */
CHECK(ZSTD_getErrorCode(cSize) != ZSTD_error_externalSequences_invalid, "Wrong error code: %s", ZSTD_getErrorName(cSize)); /* fails sequence validation */

free(sequences);
}
ZSTD_freeCCtx(cctx);
}
DISPLAYLEVEL(3, "OK \n");

_end:
FUZ_freeDictionary(dictionary);
ZSTD_freeCStream(zc);
Expand Down