Skip to content

Fix issue T283341343 - #4731

Open
Cyan4973 wants to merge 3 commits into
facebook:devfrom
Cyan4973:t283341343-repro-oob
Open

Fix issue T283341343#4731
Cyan4973 wants to merge 3 commits into
facebook:devfrom
Cyan4973:t283341343-repro-oob

Conversation

@Cyan4973

@Cyan4973 Cyan4973 commented Aug 10, 2026

Copy link
Copy Markdown
Contributor

DDict hashset OOB with ZSTD_d_refMultipleDDicts (T283341343)

  1. new oob test passes without ASAN but abort under ASAN.
  2. commit the Fix, all tests green
  3. added another relevant test, on the preparation stage

Bug: ZSTD_DDictHashSet_getDDict and emplaceDDict do idx &= mask; idx++
which allows idx to become tableSize (64) OOB when probing collides on
last slot. IDs 3 and 47 both XXH64 hash to slot 63.

Repro (mode 0 from P2447503582): register dictID 3 (slot 63), compress,
patch frame header dictID to 47, decompress with refMultipleDDicts.
With ASAN, buggy code triggers heap-buffer-overflow at slot 64.
Without ASAN it returns dictionary mismatch and hides the bug, so CI
must run with -fsanitize=address to catch it.

This is the first commit on the fix branch - vulnerability still present
so test passes without ASAN but would abort under ASAN, proving the bug.
Fix will be second commit.

Test: test217 DDict hashset OOB T283341343 (ASAN)

Ref: T283341343
@Cyan4973 Cyan4973 self-assigned this Aug 10, 2026
@meta-cla meta-cla Bot added the CLA Signed label Aug 10, 2026
Probe loop did idx &= mask; idx++ which allows idx to become tableSize
(64) OOB when collision hits last slot. With 3 and 47 both hashing to 63,
getDDict reads past allocation. On glibc it returns NULL via slack and
hides as dict mismatch; on jemalloc/ASAN it crashes / heap-buffer-overflow.

Fix to idx = (idx + 1) & mask in both emplaceDDict and getDDict.

This is the second commit on the fix branch, makes test217 pass under ASAN
and fixes the reported wild pointer read.

Fixes T283341343
Second probe bug in ZSTD_DDictHashSet_emplaceDDict did idx &= mask; idx++
allowing 8-byte OOB write at slot 64 when inserting colliding IDs 3 and 47
both hashing to 63.

Rely on ASAN to catch heap-buffer-overflow on write (same as read path).
Without ASAN glibc slack hides it, so test passes but would be caught
in ASAN CI.

Complements test217 which covered getDDict read path.

Fixes T283341343 write path
@Cyan4973 Cyan4973 changed the title add repro for CI tests Fix issue T283341343 Aug 10, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant