Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

panic safety: double drop may happen within util::{mutate, mutate2} #2

Closed
JOE1994 opened this issue Jan 12, 2021 · 3 comments
Closed

Comments

@JOE1994
Copy link

JOE1994 commented Jan 12, 2021

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

#[inline]
pub fn mutate<T, F: FnOnce(T) -> T>(p: &mut T, f: F) { unsafe { ptr::write(p, f(ptr::read(p))) } }
#[inline]
pub fn mutate2<S, T, F: FnOnce(S, T) -> (S, T)>(p: &mut S, q: &mut T, f: F) { unsafe {
let (x, y) = f(ptr::read(p), ptr::read(q));
ptr::write(p, x);
ptr::write(q, y);
} }

Functions mutate & mutate2 temporarily duplicate the ownership of an item using the given p (and q).
In case the given function f panics, the duplicated item will be dropped twice.

Proof of Concept

The given program below exhibits a double free error.
fn mutate() is invoked within btree.insert_with().

// Tested with `rustc 1.50.0-nightly (7f9c43cf9 2020-12-23)` on Ubuntu 18.04
use containers::collections::b_tree::BTree; // containers = "0.9.10"
use default_allocator::Heap; // default_allocator = "0.3"
use rel::Core; // rel = "0.2"

fn main() {
    if let Some(mut btree) = BTree::<i32, Box<u64>, Core, Heap>::new(Core, 20) {
        if btree.insert(2, Box::new(1)).is_ok() {
            while btree
                .insert_with(2, |x| {
                    let ret = match x {
                        Some(str) => str,
                        None => Box::new(0),
                    };
                    None::<Option<u64>>.unwrap();
                    return ret;
                })
                .is_err()
            {}
        }
    }
}

Program Output

thread 'main' panicked at 'called `Option::unwrap()` on a `None` value', examples/containers.rs:15:41
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
free(): double free detected in tcache 2
Aborted (core dumped)

Thank you for checking out this issue 👍

@strake
Copy link
Owner

strake commented Jan 12, 2021

Thanks for the report! Should be unbroken since 7389a24 (in the new version 0.9.11).

By the way, what tools are you scanning with? I'd like to know more about this project.

@JOE1994
Copy link
Author

JOE1994 commented Jan 12, 2021

Thank you for your quick feedback!
We are working to submit a paper to a conference using the tool, and we'll probably release it once our work gets accepted :)

@JOE1994
Copy link
Author

JOE1994 commented Jan 19, 2021

I see you have published a new release containing the fix!
Thank you for your response, and I will close the issue now 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants