v0.0.3: panic safety issue in impl FromIterator<T> for Vector/Matrix

[JOE1994]

Hello 🦀 ,
we (Rust group @sslab-gatech) found a memory-safety/soundness issue in this crate while scanning Rust code on crates.io for potential vulnerabilities.

Issue Description

In the code that matches the latest release on crates.io (v0.0.3),
trait implementation of FromIterator<T> on Vector<> & Matrix<> can lead to a segmentation fault in safe Rust code.
In both cases, code will panic if the input iterator i has shorter length than N.
Upon panic, the program will drop partially uninitialized memory (v), which can lead to undefined behavior.

• From src/vector.rs

impl<T, N> FromIterator<T> for Vector<T, N>
  where N: ArrayLength<T> {
  #[inline]
  fn from_iter<I>(i: I) -> Self
    where I: IntoIterator<Item=T> {
    let mut v: Self = unsafe { std::mem::uninitialized() };
    let mut i = i.into_iter();
    for p in &mut v[..] {
      unsafe { std::ptr::write(p, i.next().unwrap()) };
    }
    v
  }

• From src/matrix.rs

impl<T, R, C> FromIterator<T> for Matrix<T, R, C>
  where C: ArrayLength<T>,
        R: ArrayLength<Vector<T, C>> {
  fn from_iter<I>(i: I) -> Self
    where I: IntoIterator<Item=T> {
    let mut m: Self = unsafe { std::mem::uninitialized() };
    let mut i = i.into_iter();
    for v in &mut m[..] {
      for p in &mut v[..] {
        unsafe { std::ptr::write(p, i.next().unwrap()) };
      }
    }
    m
  }
}

Proof of Concept

The below example exhibits a segmentation fault by only using safe Rust code with Vector<T, N>.
A similar example can also be crafted with Matrix<T, R, C>.

// tested with `rustc 1.47.0-nightly (bf4342114 2020-08-25)`
use adtensor::vector::Vector;
use std::iter::{FromIterator, Iterator, repeat};
use typenum::U40;
use generic_array::{GenericArray, ArrayLength};

fn main() {
    type T = String;
    let x = Vector::<T, U40>::from_iter(
        repeat(String::from("Hello World")).take(10)
    );

    panic!("Program will segfault before this point {:?}", x);
}

• Program output

thread 'main' panicked at 'called `Option::unwrap()` on a `None` value', /home/lonelyjoe/.cargo/registry/src/git.luolix.top-1ecc6299db9ec823/adtensor-0.0.3/src/vector.rs:71:44
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
Segmentation fault (core dumped)

Suggested Fix

It seems that the current github repo is totally different from the code that corresponds to the latest release (v0.0.3) on crates.io. We suggest to yank the affected versions on crates.io.

Thank you for checking out 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.