diff --git a/lib/decompress/zstd_decompress.c b/lib/decompress/zstd_decompress.c index 9eb98327ef3..7e6e01b4759 100644 --- a/lib/decompress/zstd_decompress.c +++ b/lib/decompress/zstd_decompress.c @@ -115,8 +115,7 @@ static size_t ZSTD_DDictHashSet_emplaceDDict(ZSTD_DDictHashSet* hashSet, const Z hashSet->ddictPtrTable[idx] = ddict; return 0; } - idx &= idxRangeMask; - idx++; + idx = (idx + 1) & idxRangeMask; } DEBUGLOG(4, "Final idx after probing for dictID %u is: %zu", dictID, idx); hashSet->ddictPtrTable[idx] = ddict; @@ -163,8 +162,7 @@ static const ZSTD_DDict* ZSTD_DDictHashSet_getDDict(ZSTD_DDictHashSet* hashSet, /* currDictID == 0 implies a NULL ddict entry */ break; } else { - idx &= idxRangeMask; /* Goes to start of table when we reach the end */ - idx++; + idx = (idx + 1) & idxRangeMask; /* Goes to start of table when we reach the end */ } } DEBUGLOG(4, "Final idx after probing for dictID %u is: %zu", dictID, idx); diff --git a/tests/fuzzer.c b/tests/fuzzer.c index 7b7c9d66631..32360b9a58d 100644 --- a/tests/fuzzer.c +++ b/tests/fuzzer.c @@ -3758,6 +3758,189 @@ static int basicUnitTests(U32 const seed, double compressibility) } DISPLAYLEVEL(3, "OK \n"); + DISPLAYLEVEL(3, "test%3i : DDict hashset OOB T283341343 (ASAN) : ", testNb++); + { + /* Simplified repro for T283341343 - relies on ASAN to catch OOB read. + * Original bug: probe does idx &= mask; idx++ -> idx can become 64 OOB. + * IDs 3 and 47 both hash XXH64(id) & 63 == 63 (last slot). + * Setup: register dict 3 (occupies slot 63), then decompress frame + * whose header dictID is patched to 47 (collides to same slot). + * Buggy getDDict reads slot 64 OOB. With ASAN=1 this is reported as + * heap-buffer-overflow even though glibc would see 0 and return + * "dictionary mismatch". No custom allocator needed. + * See P2447503582 mode 0. + */ + U32 victimID; + U32 attackerID; + size_t victimIdx; + size_t attackerIdx; + ZSTD_DCtx* dctx2; + ZSTD_CCtx* cctx2; + char* dictBufVictim; + ZSTD_DDict* victimDDict; + size_t srcSize; + BYTE* frame; + BYTE fhd; + int dictIDSizeCode; + int nb; + int singleSegment; + int pos; + int i; + + victimID = 3; + attackerID = 47; + victimIdx = XXH64(&victimID, sizeof(victimID), 0) & 63; + attackerIdx = XXH64(&attackerID, sizeof(attackerID), 0) & 63; + if (victimIdx != 63 || attackerIdx != 63) { + DISPLAY("hash assumption broken\n"); + goto _output_error; + } + + dctx2 = ZSTD_createDCtx(); + cctx2 = ZSTD_createCCtx(); + dictBufVictim = (char*)malloc(dictBufferFixedSize); + victimDDict = NULL; + + if (!dctx2 || !cctx2 || !dictBufVictim) { + DISPLAY("alloc failed\n"); + goto _oob_error2; + } + + ZSTD_memcpy(dictBufVictim, dictBufferFixed, dictBufferFixedSize); + MEM_writeLE32(dictBufVictim + ZSTD_FRAMEIDSIZE, victimID); + victimDDict = ZSTD_createDDict(dictBufVictim, dictBufferFixedSize); + if (!victimDDict) { + DISPLAY("DDict creation failed\n"); + goto _oob_error2; + } + + CHECK_Z( ZSTD_DCtx_setParameter(dctx2, ZSTD_d_refMultipleDDicts, ZSTD_rmd_refMultipleDDicts) ); + CHECK_Z( ZSTD_DCtx_refDDict(dctx2, victimDDict) ); + + ZSTD_CCtx_reset(cctx2, ZSTD_reset_session_and_parameters); + srcSize = MIN(CNBuffSize, 1 KB); + cSize = ZSTD_compress_usingDict(cctx2, compressedBuffer, compressedBufferSize, + CNBuffer, srcSize, + dictBufVictim, dictBufferFixedSize, 3); + if (ZSTD_isError(cSize)) { + DISPLAY("compress_usingDict failed %s\n", ZSTD_getErrorName(cSize)); + goto _oob_error2; + } + + /* Patch dictID in frame header to attackerID (47) */ + frame = (BYTE*)compressedBuffer; + fhd = frame[4]; + dictIDSizeCode = fhd & 3; + nb = (dictIDSizeCode == 3) ? 4 : dictIDSizeCode; + singleSegment = (fhd >> 5) & 1; + pos = 4 + 1 + (singleSegment ? 0 : 1); + if (nb == 0) { + DISPLAY("frame has no dictID field\n"); + goto _oob_error2; + } + for (i = 0; i < nb; i++) frame[pos + i] = (BYTE)(attackerID >> (8 * i)); + + /* With ASAN=1 buggy code triggers heap-buffer-overflow here. + * Without ASAN it just returns dictionary_wrong, which is OK for fixed code. + * So this test always passes without ASAN, but fails under ASAN if bug present. + */ + { + size_t const ret = ZSTD_decompressDCtx(dctx2, decodedBuffer, CNBuffSize, compressedBuffer, cSize); + if (!ZSTD_isError(ret)) { + DISPLAY("unexpected success %zu (should be dict mismatch)\n", ret); + goto _oob_error2; + } + /* Expected: dictionary_wrong / mismatch -> OK means OOB was not taken as success, + * but ASAN would have already aborted if OOB read happened. */ + } + + ZSTD_freeDDict(victimDDict); + ZSTD_freeDCtx(dctx2); + ZSTD_freeCCtx(cctx2); + free(dictBufVictim); + goto _oob_skip2; + _oob_error2: + if (victimDDict) ZSTD_freeDDict(victimDDict); + if (dctx2) ZSTD_freeDCtx(dctx2); + if (cctx2) ZSTD_freeCCtx(cctx2); + if (dictBufVictim) free(dictBufVictim); + goto _output_error; + _oob_skip2: ; + } + DISPLAYLEVEL(3, "OK (ASAN would report OOB read if vulnerable)\n"); + + DISPLAYLEVEL(3, "test%3i : DDict hashset OOB write T283341343 (ASAN) : ", testNb++); + { + /* Write path version of above: same colliding IDs 3 and 47 → slot 63. + * Insertion does while(table[idx]!=NULL) { idx&=mask; idx++; } + * Second insertion with colliding ID should wrap to slot 0, but buggy code + * writes to slot 64 OOB (8-byte write). With ASAN this is heap-buffer-overflow + * on write. Without ASAN glibc slack hides it. + */ + U32 victimID2; + U32 attackerID2; + size_t vIdx2; + size_t aIdx2; + ZSTD_DCtx* dctx3; + char* dictBufV2; + char* dictBufA2; + ZSTD_DDict* victimDDict2; + ZSTD_DDict* attackerDDict2; + + victimID2 = 3; + attackerID2 = 47; + vIdx2 = XXH64(&victimID2, sizeof(victimID2), 0) & 63; + aIdx2 = XXH64(&attackerID2, sizeof(attackerID2), 0) & 63; + if (vIdx2 != 63 || aIdx2 != 63) { + DISPLAY("hash assumption broken for write test\n"); + goto _output_error; + } + + dctx3 = ZSTD_createDCtx(); + dictBufV2 = (char*)malloc(dictBufferFixedSize); + dictBufA2 = (char*)malloc(dictBufferFixedSize); + victimDDict2 = NULL; + attackerDDict2 = NULL; + + if (!dctx3 || !dictBufV2 || !dictBufA2) { + DISPLAY("alloc failed for OOB write test\n"); + goto _oob_write_error2; + } + + ZSTD_memcpy(dictBufV2, dictBufferFixed, dictBufferFixedSize); + ZSTD_memcpy(dictBufA2, dictBufferFixed, dictBufferFixedSize); + MEM_writeLE32(dictBufV2 + ZSTD_FRAMEIDSIZE, victimID2); + MEM_writeLE32(dictBufA2 + ZSTD_FRAMEIDSIZE, attackerID2); + + victimDDict2 = ZSTD_createDDict(dictBufV2, dictBufferFixedSize); + attackerDDict2 = ZSTD_createDDict(dictBufA2, dictBufferFixedSize); + if (!victimDDict2 || !attackerDDict2) { + DISPLAY("DDict creation failed for write test\n"); + goto _oob_write_error2; + } + + CHECK_Z( ZSTD_DCtx_setParameter(dctx3, ZSTD_d_refMultipleDDicts, ZSTD_rmd_refMultipleDDicts) ); + CHECK_Z( ZSTD_DCtx_refDDict(dctx3, victimDDict2) ); + /* Second colliding insertion - triggers OOB write in buggy emplaceDDict */ + CHECK_Z( ZSTD_DCtx_refDDict(dctx3, attackerDDict2) ); + + ZSTD_freeDDict(victimDDict2); + ZSTD_freeDDict(attackerDDict2); + ZSTD_freeDCtx(dctx3); + free(dictBufV2); + free(dictBufA2); + goto _oob_write_skip2; + _oob_write_error2: + if (victimDDict2) ZSTD_freeDDict(victimDDict2); + if (attackerDDict2) ZSTD_freeDDict(attackerDDict2); + if (dctx3) ZSTD_freeDCtx(dctx3); + if (dictBufV2) free(dictBufV2); + if (dictBufA2) free(dictBufA2); + goto _output_error; + _oob_write_skip2: ; + } + DISPLAYLEVEL(3, "OK (ASAN would report OOB write if vulnerable)\n"); + ZSTD_freeCCtx(cctx); free(dictBuffer); free(samplesSizes);