split_at can create unsound aliasing violations if Borrow<Idx> returns different indexes

[ammaraskar]

Hi there, we (Rust group @sslab-gatech) are scanning crates on crates.io for potential soundness bugs. We noticed that in the split_at functions:

nano-arena/src/lib.rs

Lines 219 to 223 in 100568c

	if let Some(value) = self.get_mut(selected.borrow()) {
	Some((
	unsafe { (value as *mut T).as_mut().unwrap() },
	ArenaSplit {
	selected: selected.borrow().clone(),

selected.borrow() is called twice, the first time to select the value from the arena and then the second time to create the split. Since the Borrow trait is not required to return the same thing twice, this can be used to create two mutable references to the same object:

#![forbid(unsafe_code)]

use nano_arena::{Arena, ArenaAccess, Idx};
use std::{borrow::Borrow, cell::Cell};

struct MyIdx {
    idx1: Idx,
    idx2: Idx,
    state: Cell<bool>
}

impl MyIdx {
    fn new(idx1: Idx, idx2: Idx) -> Self {
        MyIdx { idx1, idx2, state: Cell::new(false) }
    }
}

// A borrow implementation that alternatingly returns two different indexes.
impl Borrow<Idx> for MyIdx {
    fn borrow(&self) -> &Idx {
        self.state.set(!self.state.get());
        if (self.state.get()) {
            &self.idx1
        } else {
            &self.idx2
        }
    }
}

fn main() {
    let mut arena = Arena::new();
    let idx1 = arena.alloc(1);
    let idx2 = arena.alloc(2);

    let custom_idx = MyIdx::new(idx1.clone(), idx2.clone());

    let (mutable_ref_one, mut split_arena) = arena.split_at(custom_idx).unwrap();
    let mutable_ref_two : &mut i32 = split_arena.get_mut(&idx1).unwrap();

    println!("{:p} {:p}", mutable_ref_one, mutable_ref_two);
    assert!(mutable_ref_one != mutable_ref_two);
}

This outputs:

0x55f7f0601ae8 0x55f7f0601ae8
thread 'main' panicked at 'assertion failed: mutable_ref_one != mutable_ref_two', src/main.rs:59:5

[bennetthardwick]

Wow, thanks @ammaraskar that's very instructive. I'll look into fixing it.

[ammaraskar]

Thank you for the quick fix, it looks good to me!