Skip to content

compress: validate repcodes against retained dictionary content - #4915

Merged
terrelln merged 1 commit into
meta:devfrom
terrelln:2026-11-10-large-cdict
Nov 11, 2026
Merged

terrelln merged 1 commit into
meta:devfrom
terrelln:2026-11-10-large-cdict

Conversation

@terrelln

Copy link
Copy Markdown
Contributor

Full-format dictionaries may contain repeat offsets into content discarded by the short-cache CDict limit or the general index limit. Revalidate all three repcodes after loading the retained suffix and enforce the same bound before attaching a CDict, preventing wrapped dictMatchState indices and wild reads.

Add regression coverage for 20 MiB dictionaries, every repcode slot, fast/dfast levels, deferred and streaming dictionary loading, the short-cache boundary, and valid non-truncating controls.

NOTE: This will reject dictionaries that have repcodes > 16 MiB when using the short-cache CDict. But I think this is reasonable, and if it becomes a problem in the future, we can fix it then, as it is an encoder-side only limitation.

@meta-cla meta-cla Bot added the CLA Signed label Nov 10, 2026

@Cyan4973 Cyan4973 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The new check at the end of ZSTD_loadZstdDictionary() also runs when the dictionary is loaded directly into a CCtx.
In this case, the window may have been reduced by overflow correction,
so a CCtx which has already compressed a few GB can reject a valid dictionary that a fresh CCtx would accept.
The CCtx path already discards out-of-range repcodes, so this check is only needed for the CDict:
I would limit it to tfp == ZSTD_tfp_forCDict.

minor: the + 1 in tests/fuzzer.c:1130 aligns src on rep - 1, so the boundary cases never actually encode a match at rep.

minor: when the dictionary is rejected, it shows up as a NULL CDict, which the CLI reports as "Allocation error". That's a bit misleading.

Full-format dictionaries may contain repeat offsets into content discarded by the short-cache CDict limit or the general index limit. Revalidate all three repcodes after loading the retained suffix and enforce the same bound before attaching a CDict, preventing wrapped dictMatchState indices and wild reads.

Add regression coverage for 20 MiB dictionaries, every repcode slot, fast/dfast levels, deferred and streaming dictionary loading, the short-cache boundary, and valid non-truncating controls.
@terrelln
terrelln force-pushed the 2026-11-10-large-cdict branch from 02522f6 to 4f5dce9 Compare November 10, 2026 23:05
@terrelln

Copy link
Copy Markdown
Contributor Author
  • Fixed the issue with overflow correction & added a test case that catches the bug.
  • Fixed the CLI allocation error messaging
  • Fixed the test case

@terrelln
terrelln merged commit 719e528 into meta:dev Nov 11, 2026
105 checks passed
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.

2 participants