insert_many can double-free items if the iterator panics

[ammaraskar]

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:

insert_many/src/lib.rs

Lines 37 to 47 in 697ec9f

	unsafe {
	let start_ptr = $s.as_mut_ptr().offset(index);
	ptr::copy(start_ptr, start_ptr.offset(num_items as isize), len - index as usize);
	for i in 0..num_items {
	let item = iter.next().expect("ExactSizeIterator produced too few items.");
	ptr::write(start_ptr.offset(i as isize), item);
	}
	$s.set_len(len + num_items);
	}

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.

[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.