double drop may happen upon panic in EventList as std::convert::From<[E; n]>>::from

[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

We found 2 cases (below) where a double drop of an objects can happen
if a panic occurs within the user-provided Into<Event> implementation.

ocl/ocl/src/standard/event.rs

Lines 1000 to 1014 in 0308686

	macro_rules! from_event_option_array_into_event_list(
	($e:ty, $len:expr) => (
	impl<'e> From<[Option<$e>; $len]> for EventList {
	fn from(events: [Option<$e>; $len]) -> EventList {
	let mut el = EventList::with_capacity(events.len());
	for idx in 0..events.len() {
	let event_opt = unsafe { ptr::read(events.get_unchecked(idx)) };
	if let Some(event) = event_opt { el.push::<Event>(event.into()); }
	}
	mem::forget(events);
	el
	}
	}
	)
	);

ocl/ocl/src/standard/event.rs

Lines 1037 to 1050 in 0308686

	impl<'e, E> From<[E; $len]> for EventList where E: Into<Event> {
	fn from(events: [E; $len]) -> EventList {
	let mut el = EventList::with_capacity(events.len());
	for idx in 0..events.len() {
	let event = unsafe { ptr::read(events.get_unchecked(idx)) };
	el.push(event.into());
	}
	// Ownership has been unsafely transfered to the new event
	// list without modifying the event reference count. Not
	// forgetting the source array would cause a double drop.
	mem::forget(events);
	el
	}
	}

Proof of Concept

The example program below exhibits a double-drop.

use fil_ocl::{Event, EventList};
use std::convert::Into;

struct Foo(Option<i32>);

impl Into<Event> for Foo {
    fn into(self) -> Event {
        /*
        According to the docs, `Into<T>` implementations shouldn't panic.
        However rustc doesn't check whether panics can happen in the Into implementation,
        so it's possible for a user-provided `into()` to panic..
        */
        println!("LOUSY PANIC : {}", self.0.unwrap());
        
        Event::empty()
    }
}

impl Drop for Foo {
    fn drop(&mut self) {
        println!("I'm dropping");
    }
}

fn main() {
    let eventlist: EventList = [Foo(None)].into();
    dbg!(eventlist);
}

Suggested Fix

In this case, using ManuallyDrop can help guard against the potential panic within into().
I'll submit a PR with the suggested fix right away.

Thank you for checking out this issue 👍

[eslerm]

this issue was assigned CVE-2021-25908 (aka GHSA-x3v2-fgr6-3wmm and RUSTSEC-2021-0011)