antirez / antirez/ds4

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

Đang mở
#823 0 bình luận 0 reaction 0 người được giao Xem trên GitHub
Ngôn ngữ chính
C
Star
22.3k
Fork
2.1k
Merge trung bình
1 ngày 3 giờ
Pull request đã merge (30 ngày)
4

Mô tả

### 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.

Hướng dẫn đóng góp

Mở hướng dẫn đóng góp

Đánh giá

Issue này chưa được đánh giá.

Nhận issue mới trong hộp thư của bạn

Bản tóm tắt ngắn những issue GitHub phù hợp với người mới.