From c2c0c47d5f27752097ae379645e34bcba4d604ed Mon Sep 17 00:00:00 2001 From: Yann Collet Date: Thu, 27 May 2021 23:20:28 -0700 Subject: fix NULL ptr arithmetic of lz4:1572 was blindly adding an offset (0) to `dictionary` which could be `NULL`. --- lib/lz4.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/lib/lz4.c b/lib/lz4.c index c2f504e..5d4cfdd 100644 --- a/lib/lz4.c +++ b/lib/lz4.c @@ -1568,12 +1568,12 @@ int LZ4_compress_fast_continue (LZ4_stream_t* LZ4_stream, int acceleration) { const tableType_t tableType = byU32; - LZ4_stream_t_internal* streamPtr = &LZ4_stream->internal_donotuse; - const BYTE* dictEnd = streamPtr->dictionary + streamPtr->dictSize; + LZ4_stream_t_internal* const streamPtr = &LZ4_stream->internal_donotuse; + const BYTE* dictEnd = streamPtr->dictSize ? streamPtr->dictionary + streamPtr->dictSize : streamPtr->dictionary; DEBUGLOG(5, "LZ4_compress_fast_continue (inputSize=%i)", inputSize); - LZ4_renormDictT(streamPtr, inputSize); /* avoid index overflow */ + LZ4_renormDictT(streamPtr, inputSize); /* fix index overflow */ if (acceleration < 1) acceleration = LZ4_ACCELERATION_DEFAULT; if (acceleration > LZ4_ACCELERATION_MAX) acceleration = LZ4_ACCELERATION_MAX; @@ -1587,7 +1587,7 @@ int LZ4_compress_fast_continue (LZ4_stream_t* LZ4_stream, } /* Check overlapping input/dictionary space */ - { const BYTE* sourceEnd = (const BYTE*) source + inputSize; + { const BYTE* const sourceEnd = (const BYTE*)source + inputSize; if ((sourceEnd > streamPtr->dictionary) && (sourceEnd < dictEnd)) { streamPtr->dictSize = (U32)(dictEnd - sourceEnd); if (streamPtr->dictSize > 64 KB) streamPtr->dictSize = 64 KB; @@ -1623,7 +1623,7 @@ int LZ4_compress_fast_continue (LZ4_stream_t* LZ4_stream, } else { result = LZ4_compress_generic(streamPtr, source, dest, inputSize, NULL, maxOutputSize, limitedOutput, tableType, usingDictCtx, noDictIssue, acceleration); } - } else { + } else { /* small data <= 4 KB */ if ((streamPtr->dictSize < 64 KB) && (streamPtr->dictSize < streamPtr->currentOffset)) { result = LZ4_compress_generic(streamPtr, source, dest, inputSize, NULL, maxOutputSize, limitedOutput, tableType, usingExtDict, dictSmall, acceleration); } else { -- cgit v0.12 From 59273b7127417182aecdf0190e6d69f5534c4dd4 Mon Sep 17 00:00:00 2001 From: Yann Collet Date: Fri, 28 May 2021 00:26:30 -0700 Subject: fix UB lz4:988 and lz4:1178 ensure `dictBase` is only used when there is an actual dictionary content. --- lib/lz4.c | 66 ++++++++++++++++++++++++++++++++----------------------- lib/lz4frame.c | 2 ++ tests/frametest.c | 5 ++++- 3 files changed, 44 insertions(+), 29 deletions(-) diff --git a/lib/lz4.c b/lib/lz4.c index 5d4cfdd..106dd58 100644 --- a/lib/lz4.c +++ b/lib/lz4.c @@ -652,10 +652,10 @@ typedef enum { clearedTable = 0, byPtr, byU32, byU16 } tableType_t; * - usingExtDict : Like withPrefix64k, but the preceding content is somewhere * else in memory, starting at ctx->dictionary with length * ctx->dictSize. - * - usingDictCtx : Like usingExtDict, but everything concerning the preceding - * content is in a separate context, pointed to by - * ctx->dictCtx. ctx->dictionary, ctx->dictSize, and table - * entries in the current context that refer to positions + * - usingDictCtx : Everything concerning the preceding content is + * in a separate context, pointed to by ctx->dictCtx. + * ctx->dictionary, ctx->dictSize, and table entries + * in the current context that refer to positions * preceding the beginning of the current compression are * ignored. Instead, ctx->dictCtx->dictionary and ctx->dictCtx * ->dictSize describe the location and size of the preceding @@ -675,9 +675,9 @@ int LZ4_compressBound(int isize) { return LZ4_COMPRESSBOUND(isize); } int LZ4_sizeofState(void) { return LZ4_STREAMSIZE; } -/*-************************************ -* Internal Definitions used in Tests -**************************************/ +/*-**************************************** +* Internal Definitions, used only in Tests +*******************************************/ #if defined (__cplusplus) extern "C" { #endif @@ -827,9 +827,10 @@ LZ4_prepareTable(LZ4_stream_t_internal* const cctx, } } - /* Adding a gap, so all previous entries are > LZ4_DISTANCE_MAX back, is faster - * than compressing without a gap. However, compressing with - * currentOffset == 0 is faster still, so we preserve that case. + /* Adding a gap, so all previous entries are > LZ4_DISTANCE_MAX back, + * is faster than compressing without a gap. + * However, compressing with currentOffset == 0 is faster still, + * so we preserve that case. */ if (cctx->currentOffset != 0 && tableType == byU32) { DEBUGLOG(5, "LZ4_prepareTable: adding 64KB to currentOffset"); @@ -885,7 +886,8 @@ LZ4_FORCE_INLINE int LZ4_compress_generic_validated( /* the dictCtx currentOffset is indexed on the start of the dictionary, * while a dictionary in the current context precedes the currentOffset */ - const BYTE* dictBase = !dictionary ? NULL : (dictDirective == usingDictCtx) ? + const BYTE* dictBase = (dictionary == NULL) ? NULL : + (dictDirective == usingDictCtx) ? dictionary + dictSize - dictCtx->currentOffset : dictionary + dictSize - startIndex; @@ -981,10 +983,11 @@ LZ4_FORCE_INLINE int LZ4_compress_generic_validated( match = base + matchIndex; lowLimit = (const BYTE*)source; } - } else if (dictDirective==usingExtDict) { + } else if (dictDirective == usingExtDict) { if (matchIndex < startIndex) { DEBUGLOG(7, "extDict candidate: matchIndex=%5u < startIndex=%5u", matchIndex, startIndex); assert(startIndex - matchIndex >= MINMATCH); + assert(dictBase); match = dictBase + matchIndex; lowLimit = dictionary; } else { @@ -1173,6 +1176,7 @@ _next_match: } } else if (dictDirective==usingExtDict) { if (matchIndex < startIndex) { + assert(dictBase); match = dictBase + matchIndex; lowLimit = dictionary; /* required for match length counter */ } else { @@ -1514,8 +1518,9 @@ int LZ4_loadDict (LZ4_stream_t* LZ4_dict, const char* dictionary, int dictSize) return (int)dict->dictSize; } -void LZ4_attach_dictionary(LZ4_stream_t* workingStream, const LZ4_stream_t* dictionaryStream) { - const LZ4_stream_t_internal* dictCtx = dictionaryStream == NULL ? NULL : +void LZ4_attach_dictionary(LZ4_stream_t* workingStream, const LZ4_stream_t* dictionaryStream) +{ + const LZ4_stream_t_internal* dictCtx = (dictionaryStream == NULL) ? NULL : &(dictionaryStream->internal_donotuse); DEBUGLOG(4, "LZ4_attach_dictionary (%p, %p, size %u)", @@ -1569,35 +1574,39 @@ int LZ4_compress_fast_continue (LZ4_stream_t* LZ4_stream, { const tableType_t tableType = byU32; LZ4_stream_t_internal* const streamPtr = &LZ4_stream->internal_donotuse; - const BYTE* dictEnd = streamPtr->dictSize ? streamPtr->dictionary + streamPtr->dictSize : streamPtr->dictionary; + const char* dictEnd = streamPtr->dictSize ? (const char*)streamPtr->dictionary + streamPtr->dictSize : NULL; - DEBUGLOG(5, "LZ4_compress_fast_continue (inputSize=%i)", inputSize); + DEBUGLOG(5, "LZ4_compress_fast_continue (inputSize=%i, dictSize=%u)", inputSize, streamPtr->dictSize); LZ4_renormDictT(streamPtr, inputSize); /* fix index overflow */ if (acceleration < 1) acceleration = LZ4_ACCELERATION_DEFAULT; if (acceleration > LZ4_ACCELERATION_MAX) acceleration = LZ4_ACCELERATION_MAX; /* invalidate tiny dictionaries */ - if ( (streamPtr->dictSize-1 < 4-1) /* intentional underflow */ - && (dictEnd != (const BYTE*)source) ) { + if ( (streamPtr->dictSize < 4) /* tiny dictionary : not enough for a hash */ + && (dictEnd != source) /* prefix mode */ + && (inputSize > 0) /* tolerance : don't lose history, in case next invocation would use prefix mode */ + && (streamPtr->dictCtx == NULL) /* usingDictCtx */ + ) { DEBUGLOG(5, "LZ4_compress_fast_continue: dictSize(%u) at addr:%p is too small", streamPtr->dictSize, streamPtr->dictionary); + /* remove dictionary existence from history, to employ faster prefix mode */ streamPtr->dictSize = 0; streamPtr->dictionary = (const BYTE*)source; - dictEnd = (const BYTE*)source; + dictEnd = source; } /* Check overlapping input/dictionary space */ - { const BYTE* const sourceEnd = (const BYTE*)source + inputSize; - if ((sourceEnd > streamPtr->dictionary) && (sourceEnd < dictEnd)) { + { const char* const sourceEnd = source + inputSize; + if ((sourceEnd > (const char*)streamPtr->dictionary) && (sourceEnd < dictEnd)) { streamPtr->dictSize = (U32)(dictEnd - sourceEnd); if (streamPtr->dictSize > 64 KB) streamPtr->dictSize = 64 KB; if (streamPtr->dictSize < 4) streamPtr->dictSize = 0; - streamPtr->dictionary = dictEnd - streamPtr->dictSize; + streamPtr->dictionary = (const BYTE*)dictEnd - streamPtr->dictSize; } } /* prefix mode : source data follows dictionary */ - if (dictEnd == (const BYTE*)source) { + if (dictEnd == source) { if ((streamPtr->dictSize < 64 KB) && (streamPtr->dictSize < streamPtr->currentOffset)) return LZ4_compress_generic(streamPtr, source, dest, inputSize, NULL, maxOutputSize, limitedOutput, tableType, withPrefix64k, dictSmall, acceleration); else @@ -1661,21 +1670,22 @@ int LZ4_compress_forceExtDict (LZ4_stream_t* LZ4_dict, const char* source, char* /*! LZ4_saveDict() : * If previously compressed data block is not guaranteed to remain available at its memory location, * save it into a safer place (char* safeBuffer). - * Note : you don't need to call LZ4_loadDict() afterwards, - * dictionary is immediately usable, you can therefore call LZ4_compress_fast_continue(). - * Return : saved dictionary size in bytes (necessarily <= dictSize), or 0 if error. + * Note : no need to call LZ4_loadDict() afterwards, dictionary is immediately usable, + * one can therefore call LZ4_compress_fast_continue() right after. + * @return : saved dictionary size in bytes (necessarily <= dictSize), or 0 if error. */ int LZ4_saveDict (LZ4_stream_t* LZ4_dict, char* safeBuffer, int dictSize) { LZ4_stream_t_internal* const dict = &LZ4_dict->internal_donotuse; const BYTE* const previousDictEnd = dict->dictionary + dict->dictSize; + DEBUGLOG(5, "LZ4_saveDict : dictSize=%i, safeBuffer=%p, prevDictEnd=%p", dictSize, safeBuffer, previousDictEnd); + if ((U32)dictSize > 64 KB) { dictSize = 64 KB; } /* useless to define a dictionary > 64 KB */ if ((U32)dictSize > dict->dictSize) { dictSize = (int)dict->dictSize; } if (safeBuffer == NULL) assert(dictSize == 0); - if (dictSize > 0) - memmove(safeBuffer, previousDictEnd - dictSize, dictSize); + if (dictSize > 0) memmove(safeBuffer, previousDictEnd - dictSize, dictSize); dict->dictionary = (const BYTE*)safeBuffer; dict->dictSize = (U32)dictSize; diff --git a/lib/lz4frame.c b/lib/lz4frame.c index 892b04b..21dc47f 100644 --- a/lib/lz4frame.c +++ b/lib/lz4frame.c @@ -766,6 +766,7 @@ static size_t LZ4F_makeBlock(void* dst, static int LZ4F_compressBlock(void* ctx, const char* src, char* dst, int srcSize, int dstCapacity, int level, const LZ4F_CDict* cdict) { int const acceleration = (level < 0) ? -level + 1 : 1; + DEBUGLOG(5, "LZ4F_compressBlock (srcSize=%i)", srcSize); LZ4F_initStream(ctx, cdict, level, LZ4F_blockIndependent); if (cdict) { return LZ4_compress_fast_continue((LZ4_stream_t*)ctx, src, dst, srcSize, dstCapacity, acceleration); @@ -778,6 +779,7 @@ static int LZ4F_compressBlock_continue(void* ctx, const char* src, char* dst, in { int const acceleration = (level < 0) ? -level + 1 : 1; (void)cdict; /* init once at beginning of frame */ + DEBUGLOG(5, "LZ4F_compressBlock_continue (srcSize=%i)", srcSize); return LZ4_compress_fast_continue((LZ4_stream_t*)ctx, src, dst, srcSize, dstCapacity, acceleration); } diff --git a/tests/frametest.c b/tests/frametest.c index f06d9b7..b3b4df8 100644 --- a/tests/frametest.c +++ b/tests/frametest.c @@ -558,7 +558,10 @@ int basicTests(U32 seed, double compressibility) cdict, NULL) ); DISPLAYLEVEL(3, "compressed %u bytes into %u bytes \n", (unsigned)dictSize, (unsigned)cSizeWithDict); - if ((LZ4_DISTANCE_MAX > dictSize) && (cSizeWithDict >= cSizeNoDict)) goto _output_error; /* must be more efficient */ + if ((LZ4_DISTANCE_MAX > dictSize) && (cSizeWithDict >= cSizeNoDict)) { + DISPLAYLEVEL(3, "cSizeWithDict (%zu) should have been more compact than cSizeNoDict(%zu) \n", cSizeWithDict, cSizeNoDict); + goto _output_error; /* must be more efficient */ + } crcOrig = XXH64(CNBuffer, dictSize, 0); DISPLAYLEVEL(3, "LZ4F_decompress_usingDict : "); -- cgit v0.12 From 539c783c98f1c9c6ad8db0a97da4295c36701ca7 Mon Sep 17 00:00:00 2001 From: Yann Collet Date: Fri, 28 May 2021 01:07:18 -0700 Subject: fix NULL ptr arithmetic in lz4:1680 only do arithmetic if offset > 0 --- lib/lz4.c | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/lib/lz4.c b/lib/lz4.c index 106dd58..9659962 100644 --- a/lib/lz4.c +++ b/lib/lz4.c @@ -1677,15 +1677,18 @@ int LZ4_compress_forceExtDict (LZ4_stream_t* LZ4_dict, const char* source, char* int LZ4_saveDict (LZ4_stream_t* LZ4_dict, char* safeBuffer, int dictSize) { LZ4_stream_t_internal* const dict = &LZ4_dict->internal_donotuse; - const BYTE* const previousDictEnd = dict->dictionary + dict->dictSize; - DEBUGLOG(5, "LZ4_saveDict : dictSize=%i, safeBuffer=%p, prevDictEnd=%p", dictSize, safeBuffer, previousDictEnd); + DEBUGLOG(5, "LZ4_saveDict : dictSize=%i, safeBuffer=%p", dictSize, safeBuffer); if ((U32)dictSize > 64 KB) { dictSize = 64 KB; } /* useless to define a dictionary > 64 KB */ if ((U32)dictSize > dict->dictSize) { dictSize = (int)dict->dictSize; } if (safeBuffer == NULL) assert(dictSize == 0); - if (dictSize > 0) memmove(safeBuffer, previousDictEnd - dictSize, dictSize); + if (dictSize > 0) { + const BYTE* const previousDictEnd = dict->dictionary + dict->dictSize; + assert(dict->dictionary); + memmove(safeBuffer, previousDictEnd - dictSize, dictSize); + } dict->dictionary = (const BYTE*)safeBuffer; dict->dictSize = (U32)dictSize; -- cgit v0.12 From dfc431fb3d03bc8abceeff0aa9197fd165171561 Mon Sep 17 00:00:00 2001 From: Yann Collet Date: Fri, 28 May 2021 01:10:41 -0700 Subject: fix NULL ptr arithmetic at lz4:2299 --- lib/lz4.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/lib/lz4.c b/lib/lz4.c index 9659962..62a6f6c 100644 --- a/lib/lz4.c +++ b/lib/lz4.c @@ -2298,8 +2298,13 @@ int LZ4_freeStreamDecode (LZ4_streamDecode_t* LZ4_stream) int LZ4_setStreamDecode (LZ4_streamDecode_t* LZ4_streamDecode, const char* dictionary, int dictSize) { LZ4_streamDecode_t_internal* lz4sd = &LZ4_streamDecode->internal_donotuse; - lz4sd->prefixSize = (size_t) dictSize; - lz4sd->prefixEnd = (const BYTE*) dictionary + dictSize; + lz4sd->prefixSize = (size_t)dictSize; + if (dictSize) { + assert(dictionary != NULL); + lz4sd->prefixEnd = (const BYTE*) dictionary + dictSize; + } else { + lz4sd->prefixEnd = (const BYTE*) dictionary; + } lz4sd->externalDict = NULL; lz4sd->extDictSize = 0; return 1; -- cgit v0.12