huggingface / huggingface/candle
Tensor::from_slice / from_raw_buffer return Ok when the declared shape does not match the storage element count
- Dominant language
- Rust
- Stars
- 21k
- Forks
- 1.8k
- Avg merge
- 16h 42m
- Merged PRs (30d)
- 25
Description
### Summary
`candle_core::Tensor::from_slice` (and `from_raw_buffer`, which routes through `from_slice` internally) does not validate that the declared `shape` matches the actual storage element count. The `ShapeWithOneHole` trait is designed for this purpose (the `into_shape(self, el_count)` method takes element count as a parameter), but the blanket impl ignores it:
```rust
// candle-core/src/shape.rs:474
impl> ShapeWithOneHole for S {
fn into_shape(self, _el_count: usize) -> Result {
Ok(self.into()) // <- _el_count parameter is ignored
}
}
```
As a result, `Tensor::from_slice(&[0.0], vec![10], &Device::Cpu)` returns `Ok` even though the storage has 1 element and the shape declares 10. The resulting tensor reports `shape.elem_count() == 10` while its actual storage holds 1 `f32`. Subsequent reads or iterations against the shape then read past the storage, surfacing as a `Vec` slice panic.
This is a contract bug in `from_slice` rather than an ONNX-specific issue. It is reachable from ordinary user code (e.g. constructing a tensor from any in-memory buffer with caller-supplied shape) and from any loader frontend that passes externally-derived shape and bytes through this API. `candle-onnx` is the simplest concrete trigger we have, but the surface is broader.
The current trigger via safe `Vec` indexing yields a panic, not undefined behavior. However, the lying tensor itself is constructed silently and any consumer that uses `get_unchecked`, `from_raw_parts`, or other shape-derived unsafe slicing would turn this into a true out-of-bounds read.
### Minimal reproducer (no ONNX, no external input)
```rust
use candle_core::{Device, Tensor};
fn main() {
let storage = vec![0.0f32]; // 1 element
let lying_shape = vec![1usize << 32]; // declares 2^32 elements
let t = Tensor::from_slice(&storage, lying_shape.as_slice(), &Device::Cpu).unwrap();
println!("declared elem_count = {}", t.shape().elem_count());
// declared elem_count = 4294967296
let _ = t.to_vec1::();
// panicked at candle-core/src/tensor.rs:1942:39:
// range end index 4294967296 out of range for slice of length 1
}
```
Smaller mismatches behave the same way. `Tensor::from_slice(&[0.0], vec![10], &Device::Cpu)` returns `Ok`, then `to_vec1::()` panics with `range end index 10 out of range for slice of length 1`. The bug is the missing contract check, not a magic threshold.
`from_raw_buffer` reaches the same lying-tensor state:
```rust
let raw = vec![0u8; 4]; // 1 f32 worth of bytes
let t = Tensor::from_raw_buffer(&raw, DType::F32, &[1usize << 30], &Device::Cpu).unwrap();
// declared elem_count = 1073741824, backed by 1 element
```
### ONNX trigger path (one concrete frontend)
`candle-onnx/src/eval.rs:192` `get_tensor` constructs the dim vector from `TensorProto.dims` and passes it to `Tensor::from_slice` without checking it against the embedded data buffer:
```rust
pub fn get_tensor(t: &onnx::TensorProto, name: &str) -> Result {
let dims: Vec = t.dims.iter().map(|&x| x as usize).collect();
...
Tensor::from_slice(&t.int64_data, dims.as_slice(), &Device::Cpu)
```
So an `.onnx` model that declares an initializer with `dims = [1 << 32]` and 1 actual element produces a lying tensor inside the loaded model. Downstream eval ops then trip the same OOB-slice panic when they iterate based on the declared shape. A 155-byte ONNX file with a `OneHot` node consuming such an initializer demonstrates this end-to-end:
```python
import onnx
from onnx import helper, TensorProto
indices = helper.make_tensor("indices", TensorProto.INT64, [1 << 32], [0]) # shape claims 2^32, storage has 1
depth = helper.make_tensor("depth", TensorProto.INT64, [], [1 << 32])
values = helper.make_tensor("values", TensorProto.FLOAT, [2], [0.0, 1.0])
node = helper.make_node("OneHot",
inputs=["indices", "depth", "values"],
outputs=["out"], axis=-1)
graph = onnx.GraphProto()
graph.name = "evil"
graph.initializer.extend([indices, depth, values])
graph.node.append(node)
out = helper.make_tensor_value_info("out", TensorProto.FLOAT, None)
graph.output.append(out)
model = helper.make_model(graph, opset_imports=[helper.make_opsetid("", 11)])
model.ir_version = 7
open("/tmp/evil.onnx", "wb").write(model.SerializeToString())
# total: 155 bytes
```
```rust
let model = candle_onnx::read_file("/tmp/evil.onnx")?;
let _ = candle_onnx::simple_eval(&model, std::collections::HashMap::new());
// panicked at candle-core/src/tensor.rs:1942:39
```
This is one path; any future loader that constructs tensors from caller-controlled (shape, bytes) tuples is exposed to the same root cause until the contract check is added in candle-core.
### Related observation in candle-onnx
While locating a concrete trigger, an intra-file inconsistency in `candle-onnx/src/eval.rs` showed up: the internal `get` impl for attribute tensors at L112 rejects negative `dims` entries (`if dim < 0 { bail!(...) }`), but `get_tensor` at L192 does the same `i64 -> usize` cast with no such check. Both eventually call into `Tensor::from_slice` / `from_raw_buffer` with the resulting `dims`, so the root candle-core fix is the load-bearing one, but the L192 path is worth aligning with L112 for the same defensive shape.
### Suggested fix
Two layers:
1. `candle-core` (root cause): the blanket `ShapeWithOneHole for S where S: Into` should validate the shape product matches `el_count`, or the impl should be removed and replaced with explicit-shape impls that accept a `&[D]` length argument. A non-breaking option is to add the check behind a `debug_assert` first, then promote to a hard error.
```rust
impl> ShapeWithOneHole for S {
fn into_shape(self, el_count: usize) -> Result {
let shape: Shape = self.into();
if shape.elem_count() != el_count {
crate::bail!(
"shape {shape:?} declares {} elements but storage has {el_count}",
shape.elem_count()
);
}
Ok(shape)
}
}
```
2. `candle-onnx` (additional defense): the intra-file inconsistency at `eval.rs:112` vs `eval.rs:192` should be resolved. The internal `get` impl rejects negative dims; `get_tensor` should do the same. Apply the same shape-element check before calling `Tensor::from_slice`.
Regression tests: the 155-byte PoC above should return `Err`, not panic. Additionally, a unit test should assert that `Tensor::from_slice(&[0.0], vec![10, 10], &Device::Cpu)` returns `Err` (1 element vs 100 declared).
### CWE
- CWE-1284 (Improper Validation of Specified Quantity in Input)
- CWE-125 (Out-of-bounds Read; latent. surfaced as panic by safe `Vec` indexing in this trigger path)
### CVE
If a CVE assignment is in scope here, happy to coordinate.
Contributor guide
No contributing guide indexed for this repository
Research direction
Start at candle-core/src/shape.rs:474 and trace how ShapeWithOneHole::into_shape is used by Tensor::from_slice and from_raw_buffer. Add regression coverage for mismatched storage and shape counts, then inspect candle-onnx/src/eval.rs:112 and :192 and run the 155-byte PoC to confirm malformed input returns Err rather than panicking.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- rust
- Domain
- machine-learning, security
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Quiet
- Clarity
- Clearly specified
- Newbie friendliness
- 58/100