From 8e46846287c60bb34ab8157113715b94e39261b9 Mon Sep 17 00:00:00 2001 From: Yann Collet Date: Thu, 27 May 2021 22:59:22 -0700 Subject: fix UB of lz4frame:907 now line 912 by ensuring pointer arithmetic is only performed if there is a reason for an internal buffer to be used. --- lib/lz4frame.c | 119 ++++++++++++++++++++++++++++++--------------------------- lib/lz4frame.h | 6 +-- 2 files changed, 66 insertions(+), 59 deletions(-) diff --git a/lib/lz4frame.c b/lib/lz4frame.c index ec02c92..892b04b 100644 --- a/lib/lz4frame.c +++ b/lib/lz4frame.c @@ -228,9 +228,9 @@ typedef struct LZ4F_cctx_s const LZ4F_CDict* cdict; size_t maxBlockSize; size_t maxBufferSize; - BYTE* tmpBuff; - BYTE* tmpIn; - size_t tmpInSize; + BYTE* tmpBuff; /* internal buffer, for streaming */ + BYTE* tmpIn; /* starting position of data compress within internal buffer (>= tmpBuff) */ + size_t tmpInSize; /* amount of data to compress after tmpIn */ U64 totalInSize; XXH32_state_t xxh; void* lz4CtxPtr; @@ -539,7 +539,7 @@ LZ4F_errorCode_t LZ4F_createCompressionContext(LZ4F_cctx** LZ4F_compressionConte if (cctxPtr==NULL) return err0r(LZ4F_ERROR_allocation_failed); cctxPtr->version = version; - cctxPtr->cStage = 0; /* Next stage : init stream */ + cctxPtr->cStage = 0; /* Uninitialized. Next stage : init cctx */ *LZ4F_compressionContextPtr = cctxPtr; @@ -590,9 +590,9 @@ static void LZ4F_initStream(void* ctx, /*! LZ4F_compressBegin_usingCDict() : - * init streaming compression and writes frame header into dstBuffer. - * dstBuffer must be >= LZ4F_HEADER_SIZE_MAX bytes. - * @return : number of bytes written into dstBuffer for the header + * init streaming compression AND writes frame header into @dstBuffer. + * @dstCapacity must be >= LZ4F_HEADER_SIZE_MAX bytes. + * @return : number of bytes written into @dstBuffer for the header * or an error code (can be tested using LZ4F_isError()) */ size_t LZ4F_compressBegin_usingCDict(LZ4F_cctx* cctxPtr, @@ -600,19 +600,18 @@ size_t LZ4F_compressBegin_usingCDict(LZ4F_cctx* cctxPtr, const LZ4F_CDict* cdict, const LZ4F_preferences_t* preferencesPtr) { - LZ4F_preferences_t prefNull; + LZ4F_preferences_t const prefNull = LZ4F_INIT_PREFERENCES; BYTE* const dstStart = (BYTE*)dstBuffer; BYTE* dstPtr = dstStart; - BYTE* headerStart; if (dstCapacity < maxFHSize) return err0r(LZ4F_ERROR_dstMaxSize_tooSmall); - MEM_INIT(&prefNull, 0, sizeof(prefNull)); if (preferencesPtr == NULL) preferencesPtr = &prefNull; cctxPtr->prefs = *preferencesPtr; - /* Ctx Management */ + /* cctx Management */ { U16 const ctxTypeID = (cctxPtr->prefs.compressionLevel < LZ4HC_CLEVEL_MIN) ? 1 : 2; if (cctxPtr->lz4CtxAlloc < ctxTypeID) { + /* not enough space allocated */ FREEMEM(cctxPtr->lz4CtxPtr); if (cctxPtr->prefs.compressionLevel < LZ4HC_CLEVEL_MIN) { cctxPtr->lz4CtxPtr = LZ4_createStream(); @@ -624,13 +623,13 @@ size_t LZ4F_compressBegin_usingCDict(LZ4F_cctx* cctxPtr, cctxPtr->lz4CtxAlloc = ctxTypeID; cctxPtr->lz4CtxState = ctxTypeID; } else if (cctxPtr->lz4CtxState != ctxTypeID) { - /* otherwise, a sufficient buffer is allocated, but we need to - * reset it to the correct context type */ + /* otherwise, a sufficient buffer is already allocated, + * but we need to reset it to the correct context type */ if (cctxPtr->prefs.compressionLevel < LZ4HC_CLEVEL_MIN) { - LZ4_initStream((LZ4_stream_t *) cctxPtr->lz4CtxPtr, sizeof (LZ4_stream_t)); + LZ4_initStream((LZ4_stream_t*)cctxPtr->lz4CtxPtr, sizeof(LZ4_stream_t)); } else { - LZ4_initStreamHC((LZ4_streamHC_t *) cctxPtr->lz4CtxPtr, sizeof(LZ4_streamHC_t)); - LZ4_setCompressionLevel((LZ4_streamHC_t *) cctxPtr->lz4CtxPtr, cctxPtr->prefs.compressionLevel); + LZ4_initStreamHC((LZ4_streamHC_t*)cctxPtr->lz4CtxPtr, sizeof(LZ4_streamHC_t)); + LZ4_setCompressionLevel((LZ4_streamHC_t*)cctxPtr->lz4CtxPtr, cctxPtr->prefs.compressionLevel); } cctxPtr->lz4CtxState = ctxTypeID; } @@ -669,31 +668,32 @@ size_t LZ4F_compressBegin_usingCDict(LZ4F_cctx* cctxPtr, /* Magic Number */ LZ4F_writeLE32(dstPtr, LZ4F_MAGICNUMBER); dstPtr += 4; - headerStart = dstPtr; - - /* FLG Byte */ - *dstPtr++ = (BYTE)(((1 & _2BITS) << 6) /* Version('01') */ - + ((cctxPtr->prefs.frameInfo.blockMode & _1BIT ) << 5) - + ((cctxPtr->prefs.frameInfo.blockChecksumFlag & _1BIT ) << 4) - + ((unsigned)(cctxPtr->prefs.frameInfo.contentSize > 0) << 3) - + ((cctxPtr->prefs.frameInfo.contentChecksumFlag & _1BIT ) << 2) - + (cctxPtr->prefs.frameInfo.dictID > 0) ); - /* BD Byte */ - *dstPtr++ = (BYTE)((cctxPtr->prefs.frameInfo.blockSizeID & _3BITS) << 4); - /* Optional Frame content size field */ - if (cctxPtr->prefs.frameInfo.contentSize) { - LZ4F_writeLE64(dstPtr, cctxPtr->prefs.frameInfo.contentSize); - dstPtr += 8; - cctxPtr->totalInSize = 0; - } - /* Optional dictionary ID field */ - if (cctxPtr->prefs.frameInfo.dictID) { - LZ4F_writeLE32(dstPtr, cctxPtr->prefs.frameInfo.dictID); - dstPtr += 4; + { BYTE* const headerStart = dstPtr; + + /* FLG Byte */ + *dstPtr++ = (BYTE)(((1 & _2BITS) << 6) /* Version('01') */ + + ((cctxPtr->prefs.frameInfo.blockMode & _1BIT ) << 5) + + ((cctxPtr->prefs.frameInfo.blockChecksumFlag & _1BIT ) << 4) + + ((unsigned)(cctxPtr->prefs.frameInfo.contentSize > 0) << 3) + + ((cctxPtr->prefs.frameInfo.contentChecksumFlag & _1BIT ) << 2) + + (cctxPtr->prefs.frameInfo.dictID > 0) ); + /* BD Byte */ + *dstPtr++ = (BYTE)((cctxPtr->prefs.frameInfo.blockSizeID & _3BITS) << 4); + /* Optional Frame content size field */ + if (cctxPtr->prefs.frameInfo.contentSize) { + LZ4F_writeLE64(dstPtr, cctxPtr->prefs.frameInfo.contentSize); + dstPtr += 8; + cctxPtr->totalInSize = 0; + } + /* Optional dictionary ID field */ + if (cctxPtr->prefs.frameInfo.dictID) { + LZ4F_writeLE32(dstPtr, cctxPtr->prefs.frameInfo.dictID); + dstPtr += 4; + } + /* Header CRC Byte */ + *dstPtr = LZ4F_headerChecksum(headerStart, (size_t)(dstPtr - headerStart)); + dstPtr++; } - /* Header CRC Byte */ - *dstPtr = LZ4F_headerChecksum(headerStart, (size_t)(dstPtr - headerStart)); - dstPtr++; cctxPtr->cStage = 1; /* header written, now request input data block */ return (size_t)(dstPtr - dstStart); @@ -701,9 +701,9 @@ size_t LZ4F_compressBegin_usingCDict(LZ4F_cctx* cctxPtr, /*! LZ4F_compressBegin() : - * init streaming compression and writes frame header into dstBuffer. - * dstBuffer must be >= LZ4F_HEADER_SIZE_MAX bytes. - * preferencesPtr can be NULL, in which case default parameters are selected. + * init streaming compression AND writes frame header into @dstBuffer. + * @dstCapacity 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()) */ @@ -806,6 +806,7 @@ static compressFunc_t LZ4F_selectCompression(LZ4F_blockMode_t blockMode, int lev return LZ4F_compressBlockHC_continue; } +/* Save history (up to 64KB) into @tmpBuff */ static int LZ4F_localSaveDict(LZ4F_cctx_t* cctxPtr) { if (cctxPtr->prefs.compressionLevel < LZ4HC_CLEVEL_MIN) @@ -815,19 +816,23 @@ static int LZ4F_localSaveDict(LZ4F_cctx_t* cctxPtr) typedef enum { notDone, fromTmpBuffer, fromSrcBuffer } LZ4F_lastBlockStatus; +static const LZ4F_compressOptions_t k_cOptionsNull = { 0, { 0, 0, 0 } }; + /*! LZ4F_compressUpdate() : * LZ4F_compressUpdate() can be called repetitively to compress as much data as necessary. - * dstBuffer MUST be >= LZ4F_compressBound(srcSize, preferencesPtr). - * LZ4F_compressOptions_t structure is optional : you can provide NULL as argument. + * When successful, the function always entirely consumes @srcBuffer. + * src data is either buffered or compressed into @dstBuffer. + * @dstCapacity MUST be >= LZ4F_compressBound(srcSize, preferencesPtr). + * @compressOptionsPtr is optional : provide NULL to mean "default". * @return : the number of bytes written into dstBuffer. It can be zero, meaning input data was just buffered. * or an error code if it fails (which can be tested using LZ4F_isError()) + * After an error, the state is left in a UB state, and must be re-initialized. */ size_t LZ4F_compressUpdate(LZ4F_cctx* cctxPtr, void* dstBuffer, size_t dstCapacity, const void* srcBuffer, size_t srcSize, const LZ4F_compressOptions_t* compressOptionsPtr) { - LZ4F_compressOptions_t cOptionsNull; size_t const blockSize = cctxPtr->maxBlockSize; const BYTE* srcPtr = (const BYTE*)srcBuffer; const BYTE* const srcEnd = srcPtr + srcSize; @@ -838,15 +843,15 @@ size_t LZ4F_compressUpdate(LZ4F_cctx* cctxPtr, DEBUGLOG(4, "LZ4F_compressUpdate (srcSize=%zu)", srcSize); - if (cctxPtr->cStage != 1) return err0r(LZ4F_ERROR_GENERIC); + if (cctxPtr->cStage != 1) return err0r(LZ4F_ERROR_GENERIC); /* state must be initialized and waiting for next block */ if (dstCapacity < LZ4F_compressBound_internal(srcSize, &(cctxPtr->prefs), cctxPtr->tmpInSize)) return err0r(LZ4F_ERROR_dstMaxSize_tooSmall); - MEM_INIT(&cOptionsNull, 0, sizeof(cOptionsNull)); - if (compressOptionsPtr == NULL) compressOptionsPtr = &cOptionsNull; + if (compressOptionsPtr == NULL) compressOptionsPtr = &k_cOptionsNull; /* complete tmp buffer */ if (cctxPtr->tmpInSize > 0) { /* some data already within tmp buffer */ size_t const sizeToCopy = blockSize - cctxPtr->tmpInSize; + assert(blockSize > cctxPtr->tmpInSize); if (sizeToCopy > srcSize) { /* add src to tmpIn buffer */ memcpy(cctxPtr->tmpIn + cctxPtr->tmpInSize, srcBuffer, srcSize); @@ -867,8 +872,7 @@ size_t LZ4F_compressUpdate(LZ4F_cctx* cctxPtr, if (cctxPtr->prefs.frameInfo.blockMode==LZ4F_blockLinked) cctxPtr->tmpIn += blockSize; cctxPtr->tmpInSize = 0; - } - } + } } while ((size_t)(srcEnd - srcPtr) >= blockSize) { /* compress full blocks */ @@ -882,7 +886,7 @@ size_t LZ4F_compressUpdate(LZ4F_cctx* cctxPtr, } if ((cctxPtr->prefs.autoFlush) && (srcPtr < srcEnd)) { - /* compress remaining input < blockSize */ + /* autoFlush : remaining input (< blockSize) is compressed */ lastBlockCompressed = fromSrcBuffer; dstPtr += LZ4F_makeBlock(dstPtr, srcPtr, (size_t)(srcEnd - srcPtr), @@ -892,10 +896,10 @@ size_t LZ4F_compressUpdate(LZ4F_cctx* cctxPtr, srcPtr = srcEnd; } - /* preserve dictionary if necessary */ + /* preserve dictionary within @tmpBuff whenever necessary */ if ((cctxPtr->prefs.frameInfo.blockMode==LZ4F_blockLinked) && (lastBlockCompressed==fromSrcBuffer)) { if (compressOptionsPtr->stableSrc) { - cctxPtr->tmpIn = cctxPtr->tmpBuff; + cctxPtr->tmpIn = cctxPtr->tmpBuff; /* src is stable : dictionary remains in src across invocations */ } else { int const realDictSize = LZ4F_localSaveDict(cctxPtr); if (realDictSize==0) return err0r(LZ4F_ERROR_GENERIC); @@ -904,11 +908,14 @@ size_t LZ4F_compressUpdate(LZ4F_cctx* cctxPtr, } /* keep tmpIn within limits */ - if ((cctxPtr->tmpIn + blockSize) > (cctxPtr->tmpBuff + cctxPtr->maxBufferSize) /* necessarily LZ4F_blockLinked && lastBlockCompressed==fromTmpBuffer */ - && !(cctxPtr->prefs.autoFlush)) + if (!(cctxPtr->prefs.autoFlush) /* no autoflush : there may be some data left within internal buffer */ + && (cctxPtr->tmpIn + blockSize) > (cctxPtr->tmpBuff + cctxPtr->maxBufferSize) ) /* not enough room to store next block */ { + /* only preserve 64KB within internal buffer. Ensures there is enough room for next block. + * note: this situation necessarily implies lastBlockCompressed==fromTmpBuffer */ int const realDictSize = LZ4F_localSaveDict(cctxPtr); cctxPtr->tmpIn = cctxPtr->tmpBuff + realDictSize; + assert((cctxPtr->tmpIn + blockSize) <= (cctxPtr->tmpBuff + cctxPtr->maxBufferSize)); } /* some input data left, necessarily < blockSize */ diff --git a/lib/lz4frame.h b/lib/lz4frame.h index 267f289..9ac849f 100644 --- a/lib/lz4frame.h +++ b/lib/lz4frame.h @@ -248,7 +248,8 @@ LZ4FLIB_API unsigned LZ4F_getVersion(void); * The version provided MUST be LZ4F_VERSION. It is intended to track potential version mismatch, notably when using DLL. * The function will provide a pointer to a fully allocated LZ4F_cctx object. * If @return != zero, there was an error during context creation. - * Object can release its memory using LZ4F_freeCompressionContext(); + * Object can be released using LZ4F_freeCompressionContext(); + * Note: LZ4F_freeCompressionContext() works with NULL pointers (do nothing). */ LZ4FLIB_API LZ4F_errorCode_t LZ4F_createCompressionContext(LZ4F_cctx** cctxPtr, unsigned version); LZ4FLIB_API LZ4F_errorCode_t LZ4F_freeCompressionContext(LZ4F_cctx* cctx); @@ -301,8 +302,7 @@ LZ4FLIB_API size_t LZ4F_compressBound(size_t srcSize, const LZ4F_preferences_t* * Important rule: dstCapacity MUST be large enough to ensure operation success even in worst case situations. * This value is provided by LZ4F_compressBound(). * If this condition is not respected, LZ4F_compress() will fail (result is an errorCode). - * LZ4F_compressUpdate() doesn't guarantee error recovery. - * When an error occurs, compression context must be freed or resized. + * After an error, the state is left in a UB state, and must be re-initialized or freed. * `cOptPtr` is optional : NULL can be provided, in which case all options are set to default. * @return : number of bytes written into `dstBuffer` (it can be zero, meaning input data was just buffered). * or an error code if it fails (which can be tested using LZ4F_isError()) -- cgit v0.12