huggingface / huggingface/candle
Unsound APIs (`Layout::new`, `Layout::contiguous_with_offset`) — unchecked `start_offset` leads to heap-buffer-overflow
- Dominant language
- Rust
- Stars
- 21k
- Forks
- 1.8k
- Avg merge
- 16h 42m
- Merged PRs (30d)
- 25
Description
## Summary
`Layout::new` and `Layout::contiguous_with_offset` accept an arbitrary `start_offset` value without any validation. Downstream unsafe code in `cpu_backend::utils::unary_map` unconditionally trusts this offset via `get_unchecked`, resulting in out-of-bounds memory access. This is a soundness bug: undefined behavior is reachable without writing any `unsafe` code.
Confirmed on 0.11.0
## Root Cause
The `Layout` type stores a `start_offset: usize` field that determines where element access begins within a backing storage buffer. The two public constructors accept any `start_offset` without validation:
```rust
// layout.rs:14 — accepts any usize
pub fn new(shape: Shape, stride: Vec, start_offset: usize) -> Self
// layout.rs:22 — same
pub fn contiguous_with_offset>(shape: S, start_offset: usize) -> Self
```
Once a `Layout` is constructed with a valid `start_offset`, all derived operations (`narrow`, `transpose`, `permute`, `broadcast_as`) preserve the invariant that `start_offset` stays within bounds — `narrow`'s `start + len <= dims[dim]` check guarantees the new offset cannot overflow given a valid initial state. The bug is therefore entirely in the constructors: they are the sole entry point for an invalid `start_offset`.
## PoC
```rust
#[test]
fn poc_layout_unchecked_offset_oob() {
use candle_core::backend::BackendStorage;
use candle_core::cpu_backend::CpuStorage;
use candle_core::Layout;
// Step 1: Create a layout with start_offset = usize::MAX.
// Layout::contiguous_with_offset accepts this without any validation.
let layout = Layout::contiguous_with_offset(&[4usize, 3usize], usize::MAX);
// Step 2: narrow(dim=1, start=0, len=1)
// New offset: usize::MAX + stride[1] * 0 = usize::MAX (unchanged).
let layout = layout.narrow(1, 0, 1).unwrap();
// Step 3: Trigger OOB read via elu().
// unary_map → get_unchecked(usize::MAX + ...) → heap-buffer-overflow
let storage = CpuStorage::F32(vec![0.5f32; 20]);
let _result = storage.elu(&layout, 1.0);
}
```
ASan output:
```
==540235==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x7116f01e001c
SUMMARY: AddressSanitizer: heap-buffer-overflow
candle_core::cpu_backend::utils::unary_map::
```
## Impact
- **Heap-buffer-overflow** : confirmed via ASan.
- Reachable from safe Rust through public API: `Layout::contiguous_with_offset` → `Layout::narrow` → any unary/binary storage operation (`elu`, `relu`, `gelu`, `cos`, `sin`, etc.).
Contributor guide
No contributing guide indexed for this repository
Research direction
Start in layout.rs at Layout::new and Layout::contiguous_with_offset, then trace how cpu_backend::utils::unary_map uses start_offset through get_unchecked. Add a regression test based on poc_layout_unchecked_offset_oob and run it with the relevant test suite and ASan. Done means arbitrary offsets cannot reach the unchecked access or cause a heap-buffer-overflow.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- rust
- Domain
- backend, security
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Active
- Clarity
- Clearly specified
- Newbie friendliness
- 52/100