From 89736e4e27358ed463c2482553cfae1b1176c797 Mon Sep 17 00:00:00 2001 From: Yann Collet Date: Sun, 27 Sep 2020 23:59:56 -0700 Subject: ensure last match not too close to end must respect MFLIMIT distance from oend --- lib/lz4.c | 17 +++++++++++------ lib/lz4hc.c | 44 +++++++++++++++++++++++++++++--------------- tests/fuzzer.c | 14 ++++++++------ 3 files changed, 48 insertions(+), 27 deletions(-) diff --git a/lib/lz4.c b/lib/lz4.c index c192d5b..21c8b4c 100644 --- a/lib/lz4.c +++ b/lib/lz4.c @@ -1824,10 +1824,10 @@ LZ4_decompress_generic( length = token & ML_MASK; if (length == ML_MASK) { - variable_length_error error = ok; - if ((checkOffset) && (unlikely(match + dictSize < lowPrefix))) { goto _output_error; } /* Error : offset outside buffers */ - length += read_variable_length(&ip, iend - LASTLITERALS + 1, endOnInput, 0, &error); - if (error != ok) { goto _output_error; } + variable_length_error error = ok; + if ((checkOffset) && (unlikely(match + dictSize < lowPrefix))) { goto _output_error; } /* Error : offset outside buffers */ + length += read_variable_length(&ip, iend - LASTLITERALS + 1, endOnInput, 0, &error); + if (error != ok) { goto _output_error; } if ((safeDecode) && unlikely((uptrval)(op)+length<(uptrval)op)) { goto _output_error; } /* overflow detection */ length += MINMATCH; if (op + length >= oend - FASTLOOP_SAFE_DISTANCE) { @@ -1853,7 +1853,7 @@ LZ4_decompress_generic( continue; } } } - if ((checkOffset) && (unlikely(match + dictSize < lowPrefix))) { goto _output_error; } /* Error : offset outside buffers */ + if (checkOffset && (unlikely(match + dictSize < lowPrefix))) { goto _output_error; } /* Error : offset outside buffers */ /* match starting within external dictionary */ if ((dict==usingExtDict) && (match < lowPrefix)) { if (unlikely(op+length > oend-LASTLITERALS)) { @@ -2003,7 +2003,12 @@ LZ4_decompress_generic( /* We must be on the last sequence (or invalid) because of the parsing limitations * so check that we exactly consume the input and don't overrun the output buffer. */ - if ((endOnInput) && ((ip+length != iend) || (cpy > oend))) { goto _output_error; } + if ((endOnInput) && ((ip+length != iend) || (cpy > oend))) { + DEBUGLOG(6, "should have been last run of literals") + DEBUGLOG(6, "ip(%p) + length(%i) = %p != iend (%p)", ip, (int)length, ip+length, iend); + DEBUGLOG(6, "or cpy(%p) > oend(%p)", cpy, oend); + goto _output_error; + } } memmove(op, ip, length); /* supports overlapping memory regions; only matters for in-place decompression scenarios */ ip += length; diff --git a/lib/lz4hc.c b/lib/lz4hc.c index 7c7b990..6e4d732 100644 --- a/lib/lz4hc.c +++ b/lib/lz4hc.c @@ -492,7 +492,11 @@ LZ4_FORCE_INLINE int LZ4HC_encodeSequence ( length = (size_t)(*ip - *anchor); LZ4_STATIC_ASSERT(notLimited == 0); /* Check output limit */ - if (limit && ((*op + (length / 255) + length + (2 + 1 + LASTLITERALS)) > oend)) return 1; + if (limit && ((*op + (length / 255) + length + (2 + 1 + LASTLITERALS)) > oend)) { + DEBUGLOG(6, "Not enough room to write %i literals (%i bytes remaining)", + (int)length, (int)(oend-*op)); + return 1; + } if (length >= RUN_MASK) { size_t len = length - RUN_MASK; *token = (RUN_MASK << ML_BITS); @@ -513,7 +517,10 @@ LZ4_FORCE_INLINE int LZ4HC_encodeSequence ( /* Encode MatchLength */ assert(matchLength >= MINMATCH); length = (size_t)matchLength - MINMATCH; - if (limit && (*op + (length / 255) + (1 + LASTLITERALS) > oend)) return 1; /* Check output limit */ + if (limit && (*op + (length / 255) + (1 + LASTLITERALS) > oend)) { + DEBUGLOG(6, "Not enough room to write match length"); + return 1; /* Check output limit */ + } if (length >= ML_MASK) { *token += ML_MASK; length -= ML_MASK; @@ -721,7 +728,7 @@ _last_literals: if (limit && (op + totalSize > oend)) { if (limit == limitedOutput) return 0; /* adapt lastRunSize to fill 'dest' */ - lastRunSize = (size_t)(oend - op) - 1; + lastRunSize = (size_t)(oend - op) - 1 /*token*/; llAdd = (lastRunSize + 256 - RUN_MASK) / 256; lastRunSize -= llAdd; } @@ -754,13 +761,14 @@ _dest_overflow: DEBUGLOG(6, "Last sequence overflowing"); op = optr; /* restore correct out pointer */ if (op + ll_totalCost <= maxLitPos) { - /* ll validated; now how many matches ? */ + /* ll validated; now adjust match length */ size_t const bytesLeftForMl = (size_t)(maxLitPos - (op+ll_totalCost)); size_t const maxMlSize = MINMATCH + (ML_MASK-1) + (bytesLeftForMl * 255); assert(maxMlSize < INT_MAX); assert(ml >= 0); if ((size_t)ml > maxMlSize) ml = (int)maxMlSize; - LZ4HC_encodeSequence(UPDATABLE(ip, op, anchor), ml, ref, notLimited, oend); - } + if ((oend + LASTLITERALS) - (op + ll_totalCost + 2) - 1 + ml >= MFLIMIT) { + LZ4HC_encodeSequence(UPDATABLE(ip, op, anchor), ml, ref, notLimited, oend); + } } goto _last_literals; } /* compression failed */ @@ -1539,8 +1547,8 @@ encode: /* cur, last_match_pos, best_mlen, best_off must be set */ _last_literals: /* Encode Last Literals */ { size_t lastRunSize = (size_t)(iend - anchor); /* literals */ - size_t litLength = (lastRunSize + 255 - RUN_MASK) / 255; - size_t const totalSize = 1 + litLength + lastRunSize; + size_t llAdd = (lastRunSize + 255 - RUN_MASK) / 255; + size_t const totalSize = 1 + llAdd + lastRunSize; if (limit == fillOutput) oend += LASTLITERALS; /* restore correct value */ if (limit && (op + totalSize > oend)) { if (limit == limitedOutput) { /* Check output limit */ @@ -1548,11 +1556,12 @@ _last_literals: goto _return_label; } /* adapt lastRunSize to fill 'dst' */ - lastRunSize = (size_t)(oend - op) - 1; - litLength = (lastRunSize + 255 - RUN_MASK) / 255; - lastRunSize -= litLength; + lastRunSize = (size_t)(oend - op) - 1 /*token*/; + llAdd = (lastRunSize + 256 - RUN_MASK) / 256; + lastRunSize -= llAdd; } - ip = anchor + lastRunSize; + DEBUGLOG(6, "Final literal run : %i literals", (int)lastRunSize); + ip = anchor + lastRunSize; /* can be != iend if limit==fillOutput */ if (lastRunSize >= RUN_MASK) { size_t accumulator = lastRunSize - RUN_MASK; @@ -1578,15 +1587,20 @@ if (limit == fillOutput) { size_t const ll_addbytes = (ll + 240) / 255; size_t const ll_totalCost = 1 + ll_addbytes + ll; BYTE* const maxLitPos = oend - 3; /* 2 for offset, 1 for token */ + DEBUGLOG(6, "Last sequence overflowing (only %i bytes remaining)", (int)(oend-1-opSaved)); op = opSaved; /* restore correct out pointer */ if (op + ll_totalCost <= maxLitPos) { - /* ll validated; now how many matches ? */ + /* ll validated; now adjust match length */ size_t const bytesLeftForMl = (size_t)(maxLitPos - (op+ll_totalCost)); size_t const maxMlSize = MINMATCH + (ML_MASK-1) + (bytesLeftForMl * 255); assert(maxMlSize < INT_MAX); assert(ovml >= 0); if ((size_t)ovml > maxMlSize) ovml = (int)maxMlSize; - LZ4HC_encodeSequence(UPDATABLE(ip, op, anchor), ovml, ovref, notLimited, oend); - } + if ((oend + LASTLITERALS) - (op + ll_totalCost + 2) - 1 + ovml >= MFLIMIT) { + DEBUGLOG(6, "Space to end : %i + ml (%i)", (int)((oend + LASTLITERALS) - (op + ll_totalCost + 2) - 1), ovml); + DEBUGLOG(6, "Before : ip = %p, anchor = %p", ip, anchor); + LZ4HC_encodeSequence(UPDATABLE(ip, op, anchor), ovml, ovref, notLimited, oend); + DEBUGLOG(6, "After : ip = %p, anchor = %p", ip, anchor); + } } goto _last_literals; } _return_label: diff --git a/tests/fuzzer.c b/tests/fuzzer.c index ef9577d..14d1190 100644 --- a/tests/fuzzer.c +++ b/tests/fuzzer.c @@ -446,10 +446,10 @@ static int FUZ_test(U32 seed, U32 nbCycles, const U32 startCycle, const double c FUZ_DISPLAYTEST(); decodedBuffer[srcSize] = canary; { int const dSize = LZ4_decompress_safe(compressedBuffer, decodedBuffer, cSize, srcSize); - FUZ_CHECKTEST(dSize<0, "LZ4_decompress_safe() failed on data compressed by LZ4_compressHC_destSize"); - FUZ_CHECKTEST(dSize!=srcSize, "LZ4_decompress_safe() failed : decompressed %i bytes, was supposed to decompress %i bytes", dSize, srcSize); + FUZ_CHECKTEST(dSize<0, "LZ4_decompress_safe failed (%i) on data compressed by LZ4_compressHC_destSize", dSize); + FUZ_CHECKTEST(dSize!=srcSize, "LZ4_decompress_safe failed : decompressed %i bytes, was supposed to decompress %i bytes", dSize, srcSize); } - FUZ_CHECKTEST(decodedBuffer[srcSize] != canary, "LZ4_decompress_safe() overwrite dst buffer !"); + FUZ_CHECKTEST(decodedBuffer[srcSize] != canary, "LZ4_decompress_safe overwrite dst buffer !"); { U32 const crcDec = XXH32(decodedBuffer, (size_t)srcSize, 0); FUZ_CHECKTEST(crcDec!=crcBase, "LZ4_decompress_safe() corrupted decoded data"); } } @@ -931,7 +931,8 @@ static int FUZ_test(U32 seed, U32 nbCycles, const U32 startCycle, const double c LZ4_resetStreamHC_fast (LZ4_streamHC, compressionLevel); LZ4_attach_HC_dictionary(LZ4_streamHC, LZ4dictHC); ret = LZ4_compress_HC_continue(LZ4_streamHC, block, compressedBuffer, blockSize, blockContinueCompressedSize); - FUZ_CHECKTEST(ret!=blockContinueCompressedSize, "LZ4_compress_HC_continue using ExtDictCtx and fast reset size is different (%i != %i)", ret, blockContinueCompressedSize); + FUZ_CHECKTEST(ret!=blockContinueCompressedSize, "LZ4_compress_HC_continue using ExtDictCtx and fast reset size is different (%i != %i)", + ret, blockContinueCompressedSize); FUZ_CHECKTEST(ret<=0, "LZ4_compress_HC_continue using ExtDictCtx and fast reset should work : enough size available within output buffer"); FUZ_CHECKTEST(LZ4_streamHC->internal_donotuse.dirty, "Context should be clean"); @@ -957,7 +958,8 @@ static int FUZ_test(U32 seed, U32 nbCycles, const U32 startCycle, const double c LZ4_loadDictHC(LZ4dictHC, dict, dictSize); LZ4_setCompressionLevel(LZ4dictHC, compressionLevel); blockContinueCompressedSize = LZ4_compress_HC_continue_destSize(LZ4dictHC, block, compressedBuffer, &consumedSize, availableSpace); - DISPLAYLEVEL(5, " LZ4_compress_HC_continue_destSize : compressed %6i/%6i into %6i/%6i at cLevel=%i\n", consumedSize, blockSize, blockContinueCompressedSize, availableSpace, compressionLevel); + DISPLAYLEVEL(5, " LZ4_compress_HC_continue_destSize : compressed %6i/%6i into %6i/%6i at cLevel=%i \n", + consumedSize, blockSize, blockContinueCompressedSize, availableSpace, compressionLevel); FUZ_CHECKTEST(blockContinueCompressedSize==0, "LZ4_compress_HC_continue_destSize failed"); FUZ_CHECKTEST(blockContinueCompressedSize > availableSpace, "LZ4_compress_HC_continue_destSize write overflow"); FUZ_CHECKTEST(consumedSize > blockSize, "LZ4_compress_HC_continue_destSize read overflow"); @@ -965,7 +967,7 @@ static int FUZ_test(U32 seed, U32 nbCycles, const U32 startCycle, const double c FUZ_DISPLAYTEST(); decodedBuffer[consumedSize] = 0; ret = LZ4_decompress_safe_usingDict(compressedBuffer, decodedBuffer, blockContinueCompressedSize, consumedSize, dict, dictSize); - FUZ_CHECKTEST(ret!=consumedSize, "LZ4_decompress_safe_usingDict did not regenerate original data"); + FUZ_CHECKTEST(ret != consumedSize, "LZ4_decompress_safe_usingDict regenerated %i bytes (%i expected)", ret, consumedSize); FUZ_CHECKTEST(decodedBuffer[consumedSize], "LZ4_decompress_safe_usingDict overrun specified output buffer size") { U32 const crcSrc = XXH32(block, (size_t)consumedSize, 0); U32 const crcDst = XXH32(decodedBuffer, (size_t)consumedSize, 0); -- cgit v0.12