Bound unchecked expert-output size products in ds4quant (heap-buffer-overflow write)
- Dominant language
- C
- Stars
- 22.3k
- Forks
- 2.1k
- Avg merge
- 1d 3h
- Merged PRs (30d)
- 4
Description
### Summary
`generate_expert()` in `gguf-tools/deepseek4-quantize.c` sizes the routed-expert
output buffer with two unchecked `size_t` multiplies:
```c
// 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:
```c
// 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:
```c
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):
```sh
# 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 = 1010580540` ⇒ `per_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.
Contributor guide
Assessment
This issue has not been assessed yet.