From 4af1fafeb80db846d7931a5fdfd6c24108761770 Mon Sep 17 00:00:00 2001 From: Nick Terrell Date: Thu, 15 Mar 2018 18:52:38 -0700 Subject: [PATCH] Restore setting loadedDictEnd Setting `loadedDictEnd` was accidently removed from `ZSTD_loadDictionaryContent()`, which means that dictionary compression will only be able to reference the parts of the dictionary within the window. The spec allows us to reference the entire dictionary so long as even one byte is in the window. `ZSTD_enforceMaxDist()` incorrectly always allowed offsets up to `loadedDictEnd` beyond the window, even once the dictionary was out of range. When overflow protection kicked in, the check `current > loadedDictEnd + maxDist` is incorrect if `loadedDictEnd` isn't reset back to zero. `current` could be reset below the value, which would incorrectly allow references beyond the window. This bug is present in `master`, but is very hard to trigger, since it requires both dictionaries and data which triggers overflow correction. --- lib/compress/zstd_compress.c | 4 +++- lib/compress/zstd_compress_internal.h | 21 ++++++++++++++++----- lib/compress/zstd_ldm.c | 2 +- 3 files changed, 20 insertions(+), 7 deletions(-) diff --git a/lib/compress/zstd_compress.c b/lib/compress/zstd_compress.c index e70c2a54..db8d7a8c 100644 --- a/lib/compress/zstd_compress.c +++ b/lib/compress/zstd_compress.c @@ -1931,8 +1931,9 @@ static size_t ZSTD_compress_frameChunk (ZSTD_CCtx* cctx, ZSTD_reduceIndex(cctx, correction); if (ms->nextToUpdate < correction) ms->nextToUpdate = 0; else ms->nextToUpdate -= correction; + ms->loadedDictEnd = 0; } - ZSTD_window_enforceMaxDist(&ms->window, ip + blockSize, ms->loadedDictEnd + maxDist); + ZSTD_window_enforceMaxDist(&ms->window, ip + blockSize, maxDist, &ms->loadedDictEnd); if (ms->nextToUpdate < ms->window.lowLimit) ms->nextToUpdate = ms->window.lowLimit; { size_t cSize = ZSTD_compressBlock_internal(cctx, @@ -2118,6 +2119,7 @@ static size_t ZSTD_loadDictionaryContent(ZSTD_matchState_t* ms, ZSTD_CCtx_params ZSTD_compressionParameters const* cParams = ¶ms->cParams; ZSTD_window_update(&ms->window, src, srcSize); + ms->loadedDictEnd = params->forceWindow ? 0 : (U32)(iend - ms->window.base); if (srcSize <= HASH_READ_SIZE) return 0; diff --git a/lib/compress/zstd_compress_internal.h b/lib/compress/zstd_compress_internal.h index 0d8bbf29..865c080c 100644 --- a/lib/compress/zstd_compress_internal.h +++ b/lib/compress/zstd_compress_internal.h @@ -562,15 +562,24 @@ MEM_STATIC U32 ZSTD_window_correctOverflow(ZSTD_window_t* window, U32 cycleLog, /** * ZSTD_window_enforceMaxDist(): - * Sets lowLimit such that indices earlier than (srcEnd - base) - lowLimit are - * invalid. This allows a simple check index >= lowLimit to see if it is valid. - * Source pointers past srcEnd are not guaranteed to be valid. + * Updates lowLimit so that: + * (srcEnd - base) - lowLimit == maxDist + loadedDictEnd + * This allows a simple check that index >= lowLimit to see if index is valid. + * This must be called before a block compression call, with srcEnd as the block + * source end. + * If loadedDictEndPtr is not NULL, we set it to zero once we update lowLimit. + * This is because dictionaries are allowed to be referenced as long as the last + * byte of the dictionary is in the window, but once they are out of range, + * they cannot be referenced. If loadedDictEndPtr is NULL, we use + * loadedDictEnd == 0. */ MEM_STATIC void ZSTD_window_enforceMaxDist(ZSTD_window_t* window, - void const* srcEnd, U32 maxDist) + void const* srcEnd, U32 maxDist, + U32* loadedDictEndPtr) { U32 const current = (U32)((BYTE const*)srcEnd - window->base); - if (current > maxDist) { + U32 loadedDictEnd = loadedDictEndPtr != NULL ? *loadedDictEndPtr : 0; + if (current > maxDist + loadedDictEnd) { U32 const newLowLimit = current - maxDist; if (window->lowLimit < newLowLimit) window->lowLimit = newLowLimit; if (window->dictLimit < window->lowLimit) { @@ -578,6 +587,8 @@ MEM_STATIC void ZSTD_window_enforceMaxDist(ZSTD_window_t* window, window->lowLimit); window->dictLimit = window->lowLimit; } + if (loadedDictEndPtr) + *loadedDictEndPtr = 0; } } diff --git a/lib/compress/zstd_ldm.c b/lib/compress/zstd_ldm.c index fa395ed7..5c9c0d2b 100644 --- a/lib/compress/zstd_ldm.c +++ b/lib/compress/zstd_ldm.c @@ -514,7 +514,7 @@ size_t ZSTD_ldm_generateSequences( * * Try invalidation after the sequence generation and test the * the offset against maxDist directly. */ - ZSTD_window_enforceMaxDist(&ldmState->window, chunkEnd, maxDist); + ZSTD_window_enforceMaxDist(&ldmState->window, chunkEnd, maxDist, NULL); /* 3. Generate the sequences for the chunk, and get newLeftoverSize. */ newLeftoverSize = ZSTD_ldm_generateSequences_internal( ldmState, sequences, params, chunkStart, chunkSize);