ocl icon indicating copy to clipboard operation
ocl copied to clipboard

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

Open JOE1994 opened this issue 4 years ago • 1 comments

Hello :crab: , 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.

https://github.com/cogciprocate/ocl/blob/03086863e95e43033ae67f1530801cb57032e1a3/ocl/src/standard/event.rs#L1000-L1014

https://github.com/cogciprocate/ocl/blob/03086863e95e43033ae67f1530801cb57032e1a3/ocl/src/standard/event.rs#L1037-L1050

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 :+1:

JOE1994 avatar Jan 04 '21 13:01 JOE1994

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

eslerm avatar Jul 30 '23 05:07 eslerm