huggingface / huggingface/candle

Unsound APIs (`Layout::new`, `Layout::contiguous_with_offset`) — unchecked `start_offset` leads to heap-buffer-overflow

Open
#3,986 0 comments 0 reactions 0 assignees View on GitHub
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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.