Bound unchecked expert-output size products in ds4quant (heap-buffer-overflow write)

Open Beginner friendly
#823 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Assessment

Difficulty
2/5
Estimated time
1-3 hours
Newbie friendliness
85/100
Issue type
Bug
Clarity
Clearly specified
Activity status
Active
Tech stack
c
Domain
security, tooling

Research direction

Start in gguf-tools/deepseek4-quantize.c at generate_expert() lines 1487-1488 and inspect checked_size_product plus the sibling FP8/FP4 paths. Run poc/gen_poc.py and the supplied ASan command, then rebuild with the existing make command and confirm the crafted input is rejected with the allocation-too-large diagnostic instead of reaching the overflow.

Written by the indexing model from the issue text.

Description

Summary

generate_expert() in gguf-tools/deepseek4-quantize.c sizes the routed-expert
output buffer with two unchecked size_t multiplies:

// gguf-tools/deepseek4-quantize.c:1487-1488
const size_t per_expert = (size_t)nrows * ds4q_row_size(target, ncols);
byte_buf out = { .size = per_expert * (size_t)n_experts,
                 .data = xmalloc(per_expert * (size_t)n_experts) };

nrows / ncols come from the untrusted GGUF template (tmpl->ne[], read
straight from the file at load_gguf_metadata, line 1749 — only the rank is
bounded to 1..DS4Q_MAX_DIMS, the dim values themselves are not range-checked),
and n_experts from the deepseek4.expert_count KV (or --n-experts), capped
at INT_MAX.

Neither per_expert nor per_expert * n_experts is overflow-checked, in
contrast to the sibling dequant paths which were hardened in
a968c08 ("Make model conversion size checks overflow-safe") and c7689db.
That fix added checked_shape_product / checked_size_product to
dequant_fp8_weight / dequant_fp4_weight but missed the expert-output
allocation.

When per_expert * n_experts wraps size_t, xmalloc gets the small wrapped
value while the per-expert quantization still produces per_expert real bytes.
The worker then writes the full per_expert payload into the undersized buffer:

// gguf-tools/deepseek4-quantize.c:1425 (MXFP4 branch) / :1450 (quantized branch)
memcpy(j->out->data + (size_t)xid * j->per_expert, q.data, q.size);

The q.size != j->per_expert guard (lines 1424 / 1449) does not catch this:
q.size is the single-expert size, computed with the same nrows * row_size
arithmetic, so it is self-consistent and equal to the real (unwrapped)
per_expert. Only the n_experts-fold product wraps. This is a
heap-buffer-overflow write — a memory-safety bug triggered by a crafted
model file (GGUF template + safetensors shard), the same trust boundary as
#542 / #543.

This is a write, not a read: it corrupts the heap rather than leaking it. Under
ASan it aborts deterministically; without ASan the multi-GiB write runs into an
unmapped page and the process crashes (local DoS) while corrupting adjacent
allocations first. No control-flow sink is reached from the corrupted bytes
(the overflow is a single large linear memcpy of quantized output), so this is
a reliability/memory-safety issue, not an RCE.

Fix

Bound the two multiplies like the FP8/FP4 paths already do. Minimal version:

const size_t per_expert = checked_size_product((size_t)nrows,
        ds4q_row_size(target, ncols), "expert per-expert size");
byte_buf out = { .size = checked_size_product(per_expert, (size_t)n_experts,
                    "expert total size"),
                 .data = xmalloc(checked_size_product(per_expert, (size_t)n_experts,
                    "expert total size")) };

(checked_size_product already exists in this file, added by a968c08.)

Verification

Reproducer (a ~200-byte template GGUF + a 17 GiB sparse safetensors shard
that occupies ~4 KiB on disk — the generator creates the sparse file, no large
download needed):

# generator: ds4/poc/gen_poc.py
python3 gen_poc.py poc_out
# builds a template.gguf, model.safetensors.index.json, and a sparse
# model-00001-of-00001.safetensors (17 GiB logical, ~4 KiB resident).

make -C gguf-tools -B
# ASan build for a clean diagnostic:
clang -O1 -g -fsanitize=address,undefined -fno-omit-frame-pointer \
      -std=c11 -o gguf-tools/deepseek4-quantize-asan \
      gguf-tools/deepseek4-quantize.c gguf-tools/quants.c -lm -pthread

ASAN_OPTIONS=detect_leaks=0:allocator_may_return_null=1 \
gguf-tools/deepseek4-quantize-asan \
    --hf poc_out --template poc_out/template.gguf \
    --out /tmp/out.gguf --n-experts 1010580540 --threads 1 --overwrite

Parameters chosen so the wrapped alloc stays below the real per-expert size:
nrows = 1073741825, ncols = 32 (MXFP4, block_bytes = 17),
n_experts = 1010580540per_expert = 18253611025 (≈17 GiB),
per_expert * n_experts = 2^64 + 12884901884, so xmalloc(12884901884) (~12 GiB)
but memcpy writes 18253611025 (~17 GiB).

ASan output (full log in poc/asan_poc.txt):

==22760==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x0006000047fc
WRITE of size 18253611025 at 0x0006000047fc thread T0
    #0 __asan_memcpy ...
    #1 generate_one_expert deepseek4-quantize.c:1425
    #2 expert_worker      deepseek4-quantize.c:1463
    #3 generate_expert    deepseek4-quantize.c:1504
    #4 generate_tensor    deepseek4-quantize.c:1515
    #5 write_full_gguf     deepseek4-quantize.c:1863
    #6 main               deepseek4-quantize.c:2984

0x... is located 0 bytes after 12884901884-byte region [...,...)
allocated by thread T0 here:
    #0 malloc ...
    #1 xmalloc         deepseek4-quantize.c:77
    #2 generate_expert deepseek4-quantize.c:1488
SUMMARY: AddressSanitizer: heap-buffer-overflow deepseek4-quantize.c:1425

Exit 134 (SIGABRT). Before the fix the quantizer corrupts the heap / crashes;
after adding checked_size_product at the two multiplies it rejects the file
with "expert total size allocation is too large" instead of overflowing.

Scope / impact
  • Reachable from: an attacker-supplied GGUF template + safetensors shard,
    the inputs people download and quantize. Same boundary as #542 / #543.
  • Impact: heap-buffer-overflow write → memory corruption / crash. Not a
    read (no secret leak), not RCE (no controlled control-flow sink on the
    overflow path). Aligns with the severity of the previously accepted #542 / #543.
  • Note: the template-dim path bypasses the strict runtime shape validation in
    ds4.c (tensor_expect_layout / config_expect_*), because the quantizer
    reads template dims independently and never applies those architecture
    constants — so a template with extreme ne[] is accepted by ds4quant even
    though the same dims would be rejected by the inference loader.

Found during a security review of ds4 following the methodology used for #542 / #543.
The reproducer generator and ASan log are attached (poc/gen_poc.py,
poc/asan_poc.txt); the sparse shard itself regenerates from the script.

Dominant language
C
Stars
22.5k
Forks
2.2k
Avg merge
2d 13h
Merged PRs (30d)
5

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

More from antirez/ds4

All issues in antirez/ds4

Similar issues

More C issues

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.