Skip to content

Unsoundness in as_uninit_slice_mut #33

Description

@shinmao

Unsoundness with as_uninit_slice_mut

Hi, we found two soundness bugs, which both can trigger undefined behaviors via as_uninit_slice_mut():

  1. Bug 1: as_uninit_slice_mut() allows to overwrite existing initialized elements, which can trigger UB.
  2. Bug 2: Drain::next() can make uninitialized memory exposure.

Both can be triggered in pure-safe Rust code.

Direct UB via as_uninit_slice_mut()

arraydeque/src/lib.rs

Lines 1076 to 1078 in 2ccf305

pub fn as_uninit_slice_mut(&mut self) -> &mut [MaybeUninit<T>] {
unsafe { std::slice::from_raw_parts_mut(self.xs.as_mut_ptr().cast(), CAP) }
}

This method exposes the whole buffer(capacity CAP)as &mut [MaybeUninit<T>], which allows the caller to:

  1. Overwrite the existing initialized T as MaybeUninit::uninit()
  2. Not triggering original drop
  3. Break the invariant hold for len, which counts for initialized elements

PoC 1: Direct Read UB

use arraydeque::ArrayDeque;
use std::mem::MaybeUninit;

fn main() {
    let mut deque: ArrayDeque<String, 2> = ArrayDeque::new();
    deque.push_back(String::from("hello"));
    deque.push_back(String:: from("world"));
    
    let slice = deque.as_uninit_slice_mut();
    slice[0] = MaybeUninit::uninit();
    
    // uninitialized memory access
    println!("{}", deque[0]);
}

UB in Drain::next() via Drop

arraydeque/src/lib.rs

Lines 2432 to 2435 in 2ccf305

#[inline]
fn next(&mut self) -> Option<T> {
self.iter.next().map(|elt| unsafe { ptr::read(elt) })
}

Location: Line 2389 (in Drain::drop)

fn drop(&mut self) {
    for _ in self.by_ref() {}  // call next()
    // ... 
}

Location: Line 2074-2078 (in ArrayDeque::drop)

impl<T, const CAP: usize, B: Behavior> Drop for ArrayDeque<T, CAP, B> {
    fn drop(&mut self) {
        self.clear();  // calls drain(. .)
    }
}

When as_uninit_slice_mut() breaks the invariant hold by len thenArrayDeque is dropped:

  1. [Drop::drop]() calls clear() (lib.rs#L2074-L2077)
  2. clear() calls drain(..)
  3. Drain::drop calls next()
  4. next() calls ptr::read(&T) while T actuallys is uninitialized
  5. Read uninitalized memory as String(e.g., Vec.cap)

PoC 2: Drop UB

use arraydeque::ArrayDeque;
use std::mem::MaybeUninit;

fn main() {
    let mut deque: ArrayDeque<String, 1> = ArrayDeque::new();
    deque.push_back(String::from("hello"));
    
    let slice = deque.as_uninit_slice_mut();
    slice[0] = MaybeUninit::uninit();
    
    // when dropped
    // Drop::drop → clear() → drain(..) → Drain::drop → next() → ptr::read()
} // UB occurs

Miri Output:

error: Undefined Behavior: constructing invalid value at .vec.buf.inner.cap.0: encountered uninitialized memory, but expected an integer
    --> /home/usr/.cargo/registry/src/index.crates.io-1949cf8c6b5b557f/arraydeque-0.5.1/src/lib.rs:2434:45
     |
2434 |         self.iter.next().map(|elt| unsafe { ptr::read(elt) })
     |                                             ^^^^^^^^^^^^^^ Undefined Behavior occurred here
     |
     = help: this indicates a bug in the program: it performed an invalid operation, and caused Undefined Behavior

Recommended Fixes

Option 1: Make as_uninit_slice_mut() Unsafe (Minimal Fix)

/// # Safety
/// 
/// Caller must not overwrite initialized elements (indices where the deque 
/// has valid data) with `MaybeUninit::uninit()`. Doing so will cause 
/// undefined behavior when the deque is dropped or the elements are accessed.
/// 
/// The initialized region is from index `tail` to `tail + len` (wrapping).
pub unsafe fn as_uninit_slice_mut(&mut self) -> &mut [MaybeUninit<T>] {
    unsafe { std::slice::from_raw_parts_mut(self.xs. as_mut_ptr().cast(), CAP) }
}

Option 2: Only Expose Uninitialized Regions

Remove as_uninit_slice_mut() entirely, or redesign it to only return unused capacity:

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions