Panic Safety and soundness issue in insert_row

[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 TooDee::insert_row function:

toodee/src/toodee.rs

Lines 674 to 683 in 676fe64

	unsafe {
	let mut p = self.data.as_mut_ptr().add(start);
	// shift everything to make space for the new row
	ptr::copy(p, p.add(self.num_cols), len - start);
	for e in iter {
	ptr::write(p, e);
	p = p.add(1);
	}
	self.data.set_len(len + self.num_cols);
	}

During this part, the elements are shifted over which can potentially duplicate them. After this, for e in iter is called which can potentially panic. If this occurs, the duplicated elements can be dropped twice leading to a double free, see this example:

#![forbid(unsafe_code)]

use toodee::TooDee;

struct DropDetector(u32);

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

// An iterator that panics.
// -----
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 vec = vec![DropDetector(1), DropDetector(2), DropDetector(3)];
    let mut toodee : TooDee<_> = TooDee::from_vec(1, 3, vec);

    toodee.insert_row(0, PanickingIterator());
}

This outputs:

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

---

Secondly, the function reserves space based on the len() provided by ExactSizeIterator. However, this trait shouldn't be trusted in unsafe code and can potentially lead to issues such as using undefined memory when it is implemented incorrectly like so:

#![forbid(unsafe_code)]

use toodee::TooDee;

struct IteratorWithWrongLength();

impl Iterator for IteratorWithWrongLength {
    type Item = Box<u8>;

    fn next(&mut self) -> Option<Self::Item> { None }
}

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

fn main() {
    let vec = vec![Box::<u8>::new(1)];
    let mut toodee : TooDee<_> = TooDee::from_vec(1, 1, vec);

    toodee.insert_row(1, IteratorWithWrongLength());

    println!("{}", toodee[1][0]);
}

This outputs:

Terminated with signal 11 (SIGSEGV)

[antonmarsden]

@ammaraskar Awesome, thanks for the bug report. I'll look into fixing these issues.

[antonmarsden]

@ammaraskar If you have time, please take a look at my commit and assess. I wasn't willing to sacrifice too much speed, so the code remains in an unsafe block. Note also that insert_col() had similar issues, so I've addressed them too.

[ammaraskar]

Thank you for the quick fixes, they both look good to me!