huggingface / huggingface/candle
Shape::elem_count() silently wraps in release, producing a Tensor with corrupted metadata (dims disagree with backing storage)
- Dominant language
- Rust
- Stars
- 21k
- Forks
- 1.8k
- Avg merge
- 16h 42m
- Merged PRs (30d)
- 25
Description
## Summary
Loading a GGUF file with attacker/model-controlled dimensions that overflow `usize` (e.g. `[2^32, 2^32]`) causes `Shape::elem_count()` to silently wrap to a small value while `Shape::dims()` continues to report the original (huge) dimensions. The result is a `Tensor`/`QTensor` whose reported shape and actual backing storage disagree — a silently corrupted tensor, not a crash and not (as far as I can tell) an out-of-bounds access.
Not filed as a security report: every code path I could find that actually touches the mismatched data is bounds-checked and panics before any memory access happens (see "Why it never becomes unsafe" below), so I believe this is contained. Filing as a correctness/robustness issue because the corrupted `Tensor` state itself is visible and surprising.
## Root cause
`src/shape.rs:131`
```rust
pub fn elem_count(&self) -> usize {
self.0.iter().product() // unchecked usize product
}
```
This wraps in `usize` with no overflow check. **The wrap only exists in release builds** — a debug build panics at the multiply (`attempt to multiply with overflow`), so this is a release-only defect that debug testing will never catch.
## Reproduction
A 384-byte GGUF file declaring a tensor with `dims = [4294967296, 4294967296]`:
```
declared dims = [4294967296, 4294967296]
Shape::elem_count() = 0 <- wrapped (2^32 * 2^32 mod 2^64 = 0)
QTensor accepted: storage_size_in_bytes = 0
dequantize() -> Ok
Tensor reports dims=[4294967296, 4294967296] but elem_count=0
```
The file is accepted, candle allocates 0 bytes, and hands back a `Tensor` claiming ~4.3 billion × 4.3 billion elements over an empty backing store. `dequantize()` returns `Ok` rather than an error.
The desync propagates through ordinary ops that all return `Ok` on the corrupted tensor: `flatten_all`, `t()`, `contiguous()`, `copy()`, `narrow()`, `reshape()`, `affine()`, `matmul()`. Sharpest case — `narrow()` followed by `contiguous()`:
```
base: dims=[2^32, 2^32] elem_count=0
narrowed: dims=[1, 2^32] elem_count=4294967296 <- no longer zero
contiguous() -> dims=[1, 4294967296] elem_count=4294967296
real storage len = 0 bytes
peak RSS ~1.9 MB <- no 16 GiB allocation ever happened
```
`narrow()` is the interesting step because it defeats the `elem_count() == 0` guard elsewhere: the narrowed view's element count no longer wraps to zero, even though the storage behind it is still empty.
## Why it never becomes unsafe (three independent reasons, so I'm not filing this as a vulnerability)
1. `elem_count()` is used for *both* buffer sizing and empty-tensor short-circuits (e.g. `tensor.rs:1516`: `if c_shape.elem_count() == 0 || k == 0 { return Tensor::zeros(...) }`), so the same wrap that corrupts the shape also routes most ops onto a no-op path. `matmul` never reaches gemm on the base (unwrapped-view) case.
2. Every CPU backend path that actually reads/writes the mismatched data panics on a Rust slice bounds check before any access — e.g. `narrowed.affine(...)` panics at `cpu_backend/utils.rs:331` with "range end index 4294967296 ... length 0", not a memory fault.
3. Confirmed with Miri (`-C overflow-checks=off` to force release semantics, `-Zmiri-disable-alignment-check` to isolate from an unrelated alignment issue I filed separately, #3815): no out-of-bounds access, no aliasing/provenance UB, across the read -> dequantize -> narrow -> contiguous -> flatten -> transpose -> matmul chain.
## Suggested fix
`checked_product` (or fold with `checked_mul`, returning a `Result`/erroring shape) in `Shape::elem_count()`, or reject a `Shape` at construction time whose dimension product overflows `usize`. Either closes the silent-corruption case; I don't think a panic-on-overflow is even necessary given containment above, but a `Result`-returning path would let GGUF loaders reject the file cleanly instead of accepting a tensor with impossible metadata.
Happy to share the crafted GGUF + a Miri test harness if useful.
Contributor guide
No contributing guide indexed for this repository
Research direction
Start at src/shape.rs:131 and trace callers of Shape::elem_count(), especially tensor.rs:1516 and the GGUF loading path. Review cpu_backend/utils.rs:331 and the supplied overflow reproduction to understand the current bounds-check behavior. Done means overflowing dimensions are rejected or reported as an error instead of producing a tensor whose metadata disagrees with its storage, with regression coverage for release semantics.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- rust
- Domain
- machine-learning
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Quiet
- Clarity
- Clearly specified
- Newbie friendliness
- 65/100