Panic safety issue in SliceDeque::drain_filter

[ammaraskar]

Hi there, we (Rust group @sslab-gatech) are scanning crates on crates.io for potential soundness bugs. We noticed a panic safety issue in the DrainFilter returned by the SliceDeque::drain_filter function:

slice_deque/src/lib.rs

Lines 3017 to 3039 in 045fb28

	unsafe {
	while self.idx != self.old_len {
	let i = self.idx;
	self.idx += 1;
	let v = slice::from_raw_parts_mut(
	self.deq.as_mut_ptr(),
	self.old_len,
	);
	if (self.pred)(&mut v[i]) {
	self.del += 1;
	return Some(ptr::read(&v[i]));
	} else if self.del > 0 {
	let del = self.del;
	let src: *const T = &v[i];
	let dst: *mut T = &mut v[i - del];
	// This is safe because self.deq has length 0
	// thus its elements will not have Drop::drop
	// called on them in the event of a panic.
	ptr::copy_nonoverlapping(src, dst, 1);
	}
	}
	None
	}

Notably, the code increments self.idx before it calls self.pred which can potentially panic. This means for example, that it can leave the SliceDeque in an inconsistent state resulting in a double drop. Here is an example:

#![forbid(unsafe_code)]

use slice_deque::SliceDeque;

struct DropDetector(u32);

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

fn main() {
    let mut deq = SliceDeque::new();
    deq.push_back(DropDetector(1));
    deq.push_back(DropDetector(2));
    deq.push_back(DropDetector(3));

    let drained = deq
        .drain_filter(|x| {
            if x.0 == 1 {
                true
            } else if x.0 == 2 {
                false
            } else {
                panic!("predicate panicked!");
            }
        })
        .collect::<SliceDeque<_>>();;
}

This outputs:

thread 'main' panicked at 'predicate panicked!', src/main.rs:40:17
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
Dropping 1
Dropping 2
Dropping 2

Return code 101

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