Skip to content

dictBuilder: prevent training size underflow and div-by-zero in cover/fastcover - #4793

Closed
filtede98 wants to merge 1 commit into
meta:devfrom
filtede98:fix-cover-training-size-underflow-and-divzero
Closed

filtede98 wants to merge 1 commit into
meta:devfrom
filtede98:fix-cover-training-size-underflow-and-divzero

Conversation

@filtede98

Copy link
Copy Markdown

Summary

This PR addresses two security and stability defects in lib/dictBuilder/cover.c and lib/dictBuilder/fastcover.c:

  1. Unchecked trainingSamplesSize Integer Underflow & Buffer Overflow:
    In COVER_ctx_init() and FASTCOVER_ctx_init(), totalSamplesSize is verified against MAX(d, sizeof(U64)), but trainingSamplesSize is never checked.
    When splitPoint < 1.0 (which is standard practice and the default in FASTCOVER where FASTCOVER_DEFAULT_SPLITPOINT = 0.75), trainingSamplesSize is calculated solely from the sum of the first nbTrainSamples samples.
    If trainingSamplesSize < MAX(d, sizeof(U64)), calculating:

    • In cover.c: ctx->suffixSize = trainingSamplesSize - MAX(d, sizeof(U64)) + 1;
    • In fastcover.c: ctx->nbDmers = trainingSamplesSize - MAX(d, sizeof(U64)) + 1;
      causes unsigned integer underflow on size_t (e.g. 5 - 8 + 1 wraps around to SIZE_MAX - 1). In cover.c, this results in an oversized allocation or integer multiplication overflow in malloc(ctx->suffixSize * sizeof(U32)), followed by an out-of-bounds heap write loop in ctx->suffix[i] = i;. In fastcover.c, ctx->nbDmers wrapping leads to out-of-bounds loop bounds and crashes.
  2. Hardware Division by Zero in COVER_computeEpochs():
    In COVER_computeEpochs():

    • If k == 0 or passes == 0, maxDictSize / k / passes causes a SIGFPE division by zero.
    • If nbDmers == 0, epochs.size = MIN(minEpochSize, nbDmers) evaluates to 0, causing a SIGFPE division by zero on epochs.num = nbDmers / epochs.size.
    • Furthermore, epochs.num is now guaranteed to be >= 1, preventing any downstream modulo-by-zero crashes in for (epoch = 0; tail > 0; epoch = (epoch + 1) % epochs.num).

Changes

  • Reject trainingSamplesSize < MAX(d, sizeof(U64)) in both COVER_ctx_init() and FASTCOVER_ctx_init() with ERROR(srcSize_wrong).
  • Harden COVER_computeEpochs() to safely handle zero values for k, passes, nbDmers, and epochs.size, ensuring epochs.num >= 1.
  • Add regression tests in tests/fuzzer.c to verify that ZDICT_optimizeTrainFromBuffer_cover() and ZDICT_optimizeTrainFromBuffer_fastCover() properly reject inputs with underflowing training sample sizes.

…/fastcover

In COVER_ctx_init() and FASTCOVER_ctx_init(), totalSamplesSize is
verified against MAX(d, sizeof(U64)), but trainingSamplesSize is never
checked. When splitPoint < 1.0, trainingSamplesSize is computed from
the first nbTrainSamples samples only. If trainingSamplesSize is
smaller than MAX(d, sizeof(U64)), computing ctx->suffixSize and
ctx->nbDmers:
    trainingSamplesSize - MAX(d, sizeof(U64)) + 1
underflows size_t to a value near SIZE_MAX. This leads to massive
out-of-bounds heap operations and crashes.

Additionally, in COVER_computeEpochs():
1. If k == 0 or passes == 0, maxDictSize / k / passes triggers a
   SIGFPE division by zero.
2. If nbDmers == 0, epochs.size evaluates to 0, causing a subsequent
   nbDmers / epochs.size SIGFPE division by zero.
3. If epochs.num ever evaluated to 0, callers computing
   (epoch + 1) % epochs.num would trigger modulo by zero.

Fix both issues by:
- Rejecting trainingSamplesSize < MAX(d, sizeof(U64)) with
  ERROR(srcSize_wrong) in COVER_ctx_init and FASTCOVER_ctx_init.
- Hardening COVER_computeEpochs against zero values for k, passes,
  and epochs.size, guaranteeing epochs.num >= 1.
- Adding regression tests to tests/fuzzer.c.

Signed-off-by: Filippo Tedeschi <filippotedeschi98@gmail.com>
@meta-cla meta-cla Bot added the CLA Signed label Sep 25, 2026
@filtede98

Copy link
Copy Markdown
Author

Closing. I tried the case this describes (cover and fastCover, splitPoint 0.75,
samples of 1, 1, 1 and 100 bytes, so the training part is shorter than d) on
dev with ASan and both return an error ("Src size is incorrect") instead of
underflowing. k == 0 and passes == 0 are rejected earlier by the parameter
checks. Sorry for the noise.

@filtede98 filtede98 closed this Oct 5, 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