From e07a37d712c87b6d47d043b018e4ff86d31996b3 Mon Sep 17 00:00:00 2001 From: Yann Collet Date: Tue, 9 Oct 2018 14:25:18 -0700 Subject: added a test for LZ4F_compressEnd() which actively tries to make it write out of bound. For this scenario to be possible, it's necessary to set dstCapacity < LZ4F_compressBound() When a compression operation fails, the CCtx context is left in an undefined state, therefore compression cannot resume. As a consequence : - round trip tests must be aborted, since there is nothing valid to decompress - most users avoid this situation, by ensuring that dstCapacity >= LZ4F_compressBound() For these reasons, this use case was poorly tested up to now. --- lib/lz4frame.c | 25 ++++++++++++++----------- lib/lz4frame.h | 2 ++ tests/frametest.c | 14 ++++++++++++-- 3 files changed, 28 insertions(+), 13 deletions(-) diff --git a/lib/lz4frame.c b/lib/lz4frame.c index 547afa3..e688f72 100644 --- a/lib/lz4frame.c +++ b/lib/lz4frame.c @@ -686,7 +686,7 @@ size_t LZ4F_compressBegin_usingCDict(LZ4F_cctx* cctxPtr, * dstBuffer must be >= LZ4F_HEADER_SIZE_MAX bytes. * preferencesPtr can be NULL, in which case default parameters are selected. * @return : number of bytes written into dstBuffer for the header - * or an error code (can be tested using LZ4F_isError()) + * or an error code (can be tested using LZ4F_isError()) */ size_t LZ4F_compressBegin(LZ4F_cctx* cctxPtr, void* dstBuffer, size_t dstCapacity, @@ -934,25 +934,28 @@ size_t LZ4F_flush(LZ4F_cctx* cctxPtr, void* dstBuffer, size_t dstCapacity, const /*! LZ4F_compressEnd() : - * When you want to properly finish the compressed frame, just call LZ4F_compressEnd(). - * It will flush whatever data remained within compressionContext (like LZ4_flush()) - * but also properly finalize the frame, with an endMark and a checksum. - * The result of the function is the number of bytes written into dstBuffer (necessarily >= 4 (endMark size)) - * The function outputs an error code if it fails (can be tested using LZ4F_isError()) - * The LZ4F_compressOptions_t structure is optional : you can provide NULL as argument. - * compressionContext can then be used again, starting with LZ4F_compressBegin(). The preferences will remain the same. + * When you want to properly finish the compressed frame, just call LZ4F_compressEnd(). + * It will flush whatever data remained within compressionContext (like LZ4_flush()) + * but also properly finalize the frame, with an endMark and an (optional) checksum. + * LZ4F_compressOptions_t structure is optional : you can provide NULL as argument. + * @return: the number of bytes written into dstBuffer (necessarily >= 4 (endMark size)) + * or an error code if it fails (can be tested using LZ4F_isError()) + * The context can then be used again to compress a new frame, starting with LZ4F_compressBegin(). */ -size_t LZ4F_compressEnd(LZ4F_cctx* cctxPtr, void* dstBuffer, size_t dstMaxSize, const LZ4F_compressOptions_t* compressOptionsPtr) +size_t LZ4F_compressEnd(LZ4F_cctx* cctxPtr, + void* dstBuffer, size_t dstCapacity, + const LZ4F_compressOptions_t* compressOptionsPtr) { BYTE* const dstStart = (BYTE*)dstBuffer; BYTE* dstPtr = dstStart; - size_t const flushSize = LZ4F_flush(cctxPtr, dstBuffer, dstMaxSize, compressOptionsPtr); + size_t const flushSize = LZ4F_flush(cctxPtr, dstBuffer, dstCapacity, compressOptionsPtr); if (LZ4F_isError(flushSize)) return flushSize; + assert(flushSize <= dstCapacity); dstPtr += flushSize; LZ4F_writeLE32(dstPtr, 0); - dstPtr+=4; /* endMark */ + dstPtr += 4; /* endMark */ if (cctxPtr->prefs.frameInfo.contentChecksumFlag == LZ4F_contentChecksumEnabled) { U32 const xxh = XXH32_digest(&(cctxPtr->xxh)); diff --git a/lib/lz4frame.h b/lib/lz4frame.h index fc30f6f..599f17e 100644 --- a/lib/lz4frame.h +++ b/lib/lz4frame.h @@ -295,6 +295,7 @@ LZ4FLIB_API size_t LZ4F_compressUpdate(LZ4F_cctx* cctx, * `cOptPtr` is optional : it's possible to provide NULL, all options will be set to default. * @return : nb of bytes written into dstBuffer (can be zero, when there is no data stored within cctx) * or an error code if it fails (which can be tested using LZ4F_isError()) + * Note : LZ4F_flush() is guaranteed to be successful when dstCapacity >= LZ4F_compressBound(0, prefsPtr). */ LZ4FLIB_API size_t LZ4F_flush(LZ4F_cctx* cctx, void* dstBuffer, size_t dstCapacity, @@ -307,6 +308,7 @@ LZ4FLIB_API size_t LZ4F_flush(LZ4F_cctx* cctx, * `cOptPtr` is optional : NULL can be provided, in which case all options will be set to default. * @return : nb of bytes written into dstBuffer, necessarily >= 4 (endMark), * or an error code if it fails (which can be tested using LZ4F_isError()) + * Note : LZ4F_compressEnd() is guaranteed to be successful when dstCapacity >= LZ4F_compressBound(0, prefsPtr). * A successful call to LZ4F_compressEnd() makes `cctx` available again for another compression task. */ LZ4FLIB_API size_t LZ4F_compressEnd(LZ4F_cctx* cctx, diff --git a/tests/frametest.c b/tests/frametest.c index f8498b7..6d2cdd0 100644 --- a/tests/frametest.c +++ b/tests/frametest.c @@ -857,8 +857,18 @@ int fuzzerTests(U32 seed, unsigned nbTests, unsigned startTest, double compressi } } } CHECK(op>=oend, "LZ4F_compressFrameBound overflow"); - result = LZ4F_compressEnd(cCtx, op, oend-op, &cOptions); - CHECK(LZ4F_isError(result), "Compression completion failed (error %i : %s)", (int)result, LZ4F_getErrorName(result)); + { size_t const dstEndSafeSize = LZ4F_compressBound(0, prefsPtr); + int const tooSmallDstEnd = ((FUZ_rand(&randState) & 31) == 3); + size_t const dstEndTooSmallSize = (FUZ_rand(&randState) % dstEndSafeSize) + 1; + size_t const dstEndSize = tooSmallDstEnd ? dstEndTooSmallSize : dstEndSafeSize; + BYTE const canaryByte = (BYTE)(FUZ_rand(&randState) & 255); + op[dstEndSize] = canaryByte; + result = LZ4F_compressEnd(cCtx, op, dstEndSize, &cOptions); + CHECK(op[dstEndSize] != canaryByte, "LZ4F_compressEnd writes beyond dstCapacity !"); + if (LZ4F_isError(result)) { + if (tooSmallDstEnd) /* failure is allowed */ continue; + CHECK(1, "Compression completion failed (error %i : %s)", (int)result, LZ4F_getErrorName(result)); + } } op += result; cSize = op-(BYTE*)compressedBuffer; DISPLAYLEVEL(5, "\nCompressed %u bytes into %u \n", (U32)srcSize, (U32)cSize); -- cgit v0.12