panic safety bug may happen in 3 functions

[JOE1994]

Hello 🦀 ,
we (Rust group @sslab-gatech) found a memory-safety/soundness issue in this crate while scanning Rust code on crates.io for potential vulnerabilities.

Issue Description

• common::Slice::<T, H>::new
  Drop uninitialized memory upon panic within T::default().
  

  arenavec/src/common.rs

  Lines 73 to 89 in f931efb

  	impl<T, H: AllocHandle> Slice<T, H> {
  	/// Create a new slice of default-initialized objects using the provided handle.
  	pub fn new(handle: H, len: usize) -> Self
  	where
  	T: Default,
  	{
  	let mut res = unsafe { Self::new_empty(handle, len) };
  	res.len = len;
  	for i in 0..len {
  	unsafe {
  	ptr::write(res.ptr.as_ptr().add(i), T::default());
  	}
  	}
  	res
  	}

• common::SliceVec::<T, H>::resize_with
  double free upon panic within T::drop in line 438.
  

  arenavec/src/common.rs

  Lines 417 to 443 in f931efb

  	/// Resize the vector to hold `len` elements, initialized to the return value of `f` if necessary.
  	pub fn resize_with<F>(&mut self, len: usize, mut f: F)
  	where
  	F: FnMut() -> T,
  	{
  	let old_len = self.slice.len;
  	if self.capacity < len {
  	self.reserve(len - old_len);
  	}
  	for i in old_len..len.saturating_sub(1) {
  	unsafe { ptr::write(self.slice.ptr.as_ptr().add(i), f()) }
  	}
  	if len > old_len {
  	unsafe {
  	ptr::write(self.slice.ptr.as_ptr().add(len - 1), f());
  	}
  	} else if len < old_len {
  	unsafe {
  	ptr::drop_in_place(&mut self.slice[len..old_len]);
  	}
  	}
  	self.slice.len = len;
  	}

• common::SliceVec::<T, H>::resize
  double free upon panic within T::drop in line 466.
  

  arenavec/src/common.rs

  Lines 445 to 471 in f931efb

  	/// Resize the vector to hold `len` elements, initialized to `value` if necessary.
  	pub fn resize(&mut self, len: usize, value: T)
  	where
  	T: Clone,
  	{
  	let old_len = self.slice.len;
  	if self.capacity < len {
  	self.reserve(len - old_len);
  	}
  	for i in old_len..len.saturating_sub(1) {
  	unsafe { ptr::write(self.slice.ptr.as_ptr().add(i), value.clone()) }
  	}
  	if len > old_len {
  	unsafe {
  	ptr::write(self.slice.ptr.as_ptr().add(len - 1), value);
  	}
  	} else if len < old_len {
  	unsafe {
  	ptr::drop_in_place(&mut self.slice[len..old_len]);
  	}
  	}
  	self.slice.len = len;
  	}

Proof of Concept

Example program below exhibits a double drop on the same object.

// tested with rustc 1.50.0-nightly (7f9c43cf9 2020-12-23) on Ubuntu 18.04
use arenavec::rc::{Arena, SliceVec};
use arenavec::ArenaBacking;
use std::sync::atomic::{
    AtomicBool,
    Ordering::SeqCst,
};

#[derive(Clone)]
struct Foo(usize, Option<u64>);
impl Drop for Foo {
    fn drop(&mut self) {
        println!("Dropping {:?}", self.0);
        if self.0 == 1 && ATOMIC_TRUE.compare_and_swap(true, false, SeqCst) {
            println!("THIS WILL PANIC {:?}", self.1.as_ref().unwrap());
        }
    }
}

static ATOMIC_TRUE: AtomicBool = AtomicBool::new(true);
const DEFAULT_CAPACITY: usize = 4096 << 8;
fn main() {
    let arena = Arena::init_capacity(ArenaBacking::SystemAllocation, DEFAULT_CAPACITY).unwrap();

    let mut vec: SliceVec<Foo> = SliceVec::new(arena.inner());
    vec.push(Foo(0, Some(12)));
    vec.push(Foo(1, None));
    assert_eq!(vec.len(), 2);

    vec.resize(1, Foo(99, Some(78)));
}

Program Output

The message Dropping 1 is printed twice, indicating the same object was dropped twice.

Dropping 1
thread 'main' panicked at 'called `Option::unwrap()` on a `None` value', examples/arenavec.rs:14:62
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
Dropping 99
Dropping 0
Dropping 1

Suggested Fix

• common::Slice::<T, H>::new:
  Move res.len = len; to after all writes are done.
• common::SliceVec::<T, H>::resize_with & common::SliceVec::<T, H>::resize:
  Move self.slice.len = len; to before drop_in_place().

Thank you for checking out this issue!

[Shnatsel]

Heads up: this issue has been included in the RustSec advisory database. It will be surfaced by tools such as cargo-audit or cargo-deny from now on.

Once a fix is released to crates.io, please open a pull request to update the advisory with the patched version, or file an issue on the advisory database repository.