rphmeier / insert_many Goto Github PK

Hi there, we (Rust group @sslab-gatech) are scanning crates on crates.io for potential soundness bugs. We noticed a potential issue in the provided insert_many implementation / impl_veclike! macro. Namely, impl_veclike pushes elements over for insertion using ptr::copy here:

This can duplicate items in the Vec, after which it calls iter.next(), which can potentially panic. This means that during this panic the copied elements can be dropped twice. Here's a quick example of this issue:

#![forbid(unsafe_code)]

use insert_many::InsertMany;

struct DropDetector(u32);

impl Drop for DropDetector {
    fn drop(&mut self) {
        println!("Dropping {}", self.0);
    }
}

// A type with an iterator that panics.
// -----
struct MyCollection();

impl IntoIterator for MyCollection {
    type Item = DropDetector;
    type IntoIter = PanickingIterator;

    fn into_iter(self) -> Self::IntoIter { PanickingIterator() }
}

struct PanickingIterator();

impl Iterator for PanickingIterator {
    type Item = DropDetector;

    fn next(&mut self) -> Option<Self::Item> { panic!("Iterator panicked"); }
}

impl ExactSizeIterator for PanickingIterator {
    fn len(&self) -> usize { 1 }
}
// -----


fn main() {
    let mut v = vec![DropDetector(1), DropDetector(2)];

    // Inserting many elements from a panicking iterator will cause a double-drop.
    v.insert_many(0, MyCollection());
}

This outputs:

thread 'main' panicked at 'Iterator panicked', src/main.rs:43:9
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
Dropping 1
Dropping 1

Return code 101

What's happening here is:

• v = [DropDetector(1), DropDetector(2)], length = 2
• Insert many called.
• v = [DropDetector(1), DropDetector(2), RESERVED], length = 2
• ptr::copy(start_ptr, start_ptr.offset(num_items as isize), len - index as usize);
• v = [DropDetector(1), DropDetector(1), DropDetector(2)], length = 2
• iter.next() panics
• DropDetector(1) gets dropped twice.

We'd recommend wrapping the vector in ManuallyDrop while performing these operations to avoid this issue.