fix UB lz4:988 and lz4:1178

ensure `dictBase` is only used
when there is an actual dictionary content.
This commit is contained in:
Yann Collet 2021-05-28 00:26:30 -07:00
parent c2c0c47d5f
commit 59273b7127
3 changed files with 44 additions and 29 deletions

View File

@ -652,10 +652,10 @@ typedef enum { clearedTable = 0, byPtr, byU32, byU16 } tableType_t;
* - usingExtDict : Like withPrefix64k, but the preceding content is somewhere * - usingExtDict : Like withPrefix64k, but the preceding content is somewhere
* else in memory, starting at ctx->dictionary with length * else in memory, starting at ctx->dictionary with length
* ctx->dictSize. * ctx->dictSize.
* - usingDictCtx : Like usingExtDict, but everything concerning the preceding * - usingDictCtx : Everything concerning the preceding content is
* content is in a separate context, pointed to by * in a separate context, pointed to by ctx->dictCtx.
* ctx->dictCtx. ctx->dictionary, ctx->dictSize, and table * ctx->dictionary, ctx->dictSize, and table entries
* entries in the current context that refer to positions * in the current context that refer to positions
* preceding the beginning of the current compression are * preceding the beginning of the current compression are
* ignored. Instead, ctx->dictCtx->dictionary and ctx->dictCtx * ignored. Instead, ctx->dictCtx->dictionary and ctx->dictCtx
* ->dictSize describe the location and size of the preceding * ->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; } int LZ4_sizeofState(void) { return LZ4_STREAMSIZE; }
/*-************************************ /*-****************************************
* Internal Definitions used in Tests * Internal Definitions, used only in Tests
**************************************/ *******************************************/
#if defined (__cplusplus) #if defined (__cplusplus)
extern "C" { extern "C" {
#endif #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 /* Adding a gap, so all previous entries are > LZ4_DISTANCE_MAX back,
* than compressing without a gap. However, compressing with * is faster than compressing without a gap.
* currentOffset == 0 is faster still, so we preserve that case. * However, compressing with currentOffset == 0 is faster still,
* so we preserve that case.
*/ */
if (cctx->currentOffset != 0 && tableType == byU32) { if (cctx->currentOffset != 0 && tableType == byU32) {
DEBUGLOG(5, "LZ4_prepareTable: adding 64KB to currentOffset"); 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, /* the dictCtx currentOffset is indexed on the start of the dictionary,
* while a dictionary in the current context precedes the currentOffset */ * 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 - dictCtx->currentOffset :
dictionary + dictSize - startIndex; dictionary + dictSize - startIndex;
@ -985,6 +987,7 @@ LZ4_FORCE_INLINE int LZ4_compress_generic_validated(
if (matchIndex < startIndex) { if (matchIndex < startIndex) {
DEBUGLOG(7, "extDict candidate: matchIndex=%5u < startIndex=%5u", matchIndex, startIndex); DEBUGLOG(7, "extDict candidate: matchIndex=%5u < startIndex=%5u", matchIndex, startIndex);
assert(startIndex - matchIndex >= MINMATCH); assert(startIndex - matchIndex >= MINMATCH);
assert(dictBase);
match = dictBase + matchIndex; match = dictBase + matchIndex;
lowLimit = dictionary; lowLimit = dictionary;
} else { } else {
@ -1173,6 +1176,7 @@ _next_match:
} }
} else if (dictDirective==usingExtDict) { } else if (dictDirective==usingExtDict) {
if (matchIndex < startIndex) { if (matchIndex < startIndex) {
assert(dictBase);
match = dictBase + matchIndex; match = dictBase + matchIndex;
lowLimit = dictionary; /* required for match length counter */ lowLimit = dictionary; /* required for match length counter */
} else { } else {
@ -1514,8 +1518,9 @@ int LZ4_loadDict (LZ4_stream_t* LZ4_dict, const char* dictionary, int dictSize)
return (int)dict->dictSize; return (int)dict->dictSize;
} }
void LZ4_attach_dictionary(LZ4_stream_t* workingStream, const LZ4_stream_t* dictionaryStream) { void LZ4_attach_dictionary(LZ4_stream_t* workingStream, const LZ4_stream_t* dictionaryStream)
const LZ4_stream_t_internal* dictCtx = dictionaryStream == NULL ? NULL : {
const LZ4_stream_t_internal* dictCtx = (dictionaryStream == NULL) ? NULL :
&(dictionaryStream->internal_donotuse); &(dictionaryStream->internal_donotuse);
DEBUGLOG(4, "LZ4_attach_dictionary (%p, %p, size %u)", 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; const tableType_t tableType = byU32;
LZ4_stream_t_internal* const streamPtr = &LZ4_stream->internal_donotuse; 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 */ LZ4_renormDictT(streamPtr, inputSize); /* fix index overflow */
if (acceleration < 1) acceleration = LZ4_ACCELERATION_DEFAULT; if (acceleration < 1) acceleration = LZ4_ACCELERATION_DEFAULT;
if (acceleration > LZ4_ACCELERATION_MAX) acceleration = LZ4_ACCELERATION_MAX; if (acceleration > LZ4_ACCELERATION_MAX) acceleration = LZ4_ACCELERATION_MAX;
/* invalidate tiny dictionaries */ /* invalidate tiny dictionaries */
if ( (streamPtr->dictSize-1 < 4-1) /* intentional underflow */ if ( (streamPtr->dictSize < 4) /* tiny dictionary : not enough for a hash */
&& (dictEnd != (const BYTE*)source) ) { && (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); 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->dictSize = 0;
streamPtr->dictionary = (const BYTE*)source; streamPtr->dictionary = (const BYTE*)source;
dictEnd = (const BYTE*)source; dictEnd = source;
} }
/* Check overlapping input/dictionary space */ /* Check overlapping input/dictionary space */
{ const BYTE* const sourceEnd = (const BYTE*)source + inputSize; { const char* const sourceEnd = source + inputSize;
if ((sourceEnd > streamPtr->dictionary) && (sourceEnd < dictEnd)) { if ((sourceEnd > (const char*)streamPtr->dictionary) && (sourceEnd < dictEnd)) {
streamPtr->dictSize = (U32)(dictEnd - sourceEnd); streamPtr->dictSize = (U32)(dictEnd - sourceEnd);
if (streamPtr->dictSize > 64 KB) streamPtr->dictSize = 64 KB; if (streamPtr->dictSize > 64 KB) streamPtr->dictSize = 64 KB;
if (streamPtr->dictSize < 4) streamPtr->dictSize = 0; 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 */ /* prefix mode : source data follows dictionary */
if (dictEnd == (const BYTE*)source) { if (dictEnd == source) {
if ((streamPtr->dictSize < 64 KB) && (streamPtr->dictSize < streamPtr->currentOffset)) if ((streamPtr->dictSize < 64 KB) && (streamPtr->dictSize < streamPtr->currentOffset))
return LZ4_compress_generic(streamPtr, source, dest, inputSize, NULL, maxOutputSize, limitedOutput, tableType, withPrefix64k, dictSmall, acceleration); return LZ4_compress_generic(streamPtr, source, dest, inputSize, NULL, maxOutputSize, limitedOutput, tableType, withPrefix64k, dictSmall, acceleration);
else else
@ -1661,21 +1670,22 @@ int LZ4_compress_forceExtDict (LZ4_stream_t* LZ4_dict, const char* source, char*
/*! LZ4_saveDict() : /*! LZ4_saveDict() :
* If previously compressed data block is not guaranteed to remain available at its memory location, * If previously compressed data block is not guaranteed to remain available at its memory location,
* save it into a safer place (char* safeBuffer). * save it into a safer place (char* safeBuffer).
* Note : you don't need to call LZ4_loadDict() afterwards, * Note : no need to call LZ4_loadDict() afterwards, dictionary is immediately usable,
* dictionary is immediately usable, you can therefore call LZ4_compress_fast_continue(). * one can therefore call LZ4_compress_fast_continue() right after.
* Return : saved dictionary size in bytes (necessarily <= dictSize), or 0 if error. * @return : saved dictionary size in bytes (necessarily <= dictSize), or 0 if error.
*/ */
int LZ4_saveDict (LZ4_stream_t* LZ4_dict, char* safeBuffer, int dictSize) int LZ4_saveDict (LZ4_stream_t* LZ4_dict, char* safeBuffer, int dictSize)
{ {
LZ4_stream_t_internal* const dict = &LZ4_dict->internal_donotuse; LZ4_stream_t_internal* const dict = &LZ4_dict->internal_donotuse;
const BYTE* const previousDictEnd = dict->dictionary + dict->dictSize; 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 > 64 KB) { dictSize = 64 KB; } /* useless to define a dictionary > 64 KB */
if ((U32)dictSize > dict->dictSize) { dictSize = (int)dict->dictSize; } if ((U32)dictSize > dict->dictSize) { dictSize = (int)dict->dictSize; }
if (safeBuffer == NULL) assert(dictSize == 0); if (safeBuffer == NULL) assert(dictSize == 0);
if (dictSize > 0) if (dictSize > 0) memmove(safeBuffer, previousDictEnd - dictSize, dictSize);
memmove(safeBuffer, previousDictEnd - dictSize, dictSize);
dict->dictionary = (const BYTE*)safeBuffer; dict->dictionary = (const BYTE*)safeBuffer;
dict->dictSize = (U32)dictSize; dict->dictSize = (U32)dictSize;

View File

@ -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) 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; int const acceleration = (level < 0) ? -level + 1 : 1;
DEBUGLOG(5, "LZ4F_compressBlock (srcSize=%i)", srcSize);
LZ4F_initStream(ctx, cdict, level, LZ4F_blockIndependent); LZ4F_initStream(ctx, cdict, level, LZ4F_blockIndependent);
if (cdict) { if (cdict) {
return LZ4_compress_fast_continue((LZ4_stream_t*)ctx, src, dst, srcSize, dstCapacity, acceleration); 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; int const acceleration = (level < 0) ? -level + 1 : 1;
(void)cdict; /* init once at beginning of frame */ (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); return LZ4_compress_fast_continue((LZ4_stream_t*)ctx, src, dst, srcSize, dstCapacity, acceleration);
} }

View File

@ -558,7 +558,10 @@ int basicTests(U32 seed, double compressibility)
cdict, NULL) ); cdict, NULL) );
DISPLAYLEVEL(3, "compressed %u bytes into %u bytes \n", DISPLAYLEVEL(3, "compressed %u bytes into %u bytes \n",
(unsigned)dictSize, (unsigned)cSizeWithDict); (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); crcOrig = XXH64(CNBuffer, dictSize, 0);
DISPLAYLEVEL(3, "LZ4F_decompress_usingDict : "); DISPLAYLEVEL(3, "LZ4F_decompress_usingDict : ");