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

Possible panic safety issues in prune and insert_item #1

Open
ammaraskar opened this issue Feb 24, 2021 · 0 comments
Open

Possible panic safety issues in prune and insert_item #1

ammaraskar opened this issue Feb 24, 2021 · 0 comments

Comments

@ammaraskar
Copy link

Hi there, we (Rust group @sslab-gatech) are scanning crates on crates.io for potential soundness bugs. We noticed some panic safety issues in the prune and insert_item functions:

topq/src/lib.rs

Lines 163 to 174 in 8cc1e75

if idx_good {
// No need to copy if we are already here
if good != idx {
// Drop the destination item
core::ptr::drop_in_place(good_ptr);
// Move from source to destination
core::ptr::copy_nonoverlapping(idx_ptr, good_ptr, 1);
}
// Move to the next good position
good += 1;

topq/src/lib.rs

Lines 106 to 114 in 8cc1e75

match result_idx {
Ok(idx) => {
unsafe {
// We have an exact priority match. Drop the old item and
// replace it with the new.
core::ptr::drop_in_place(start_ptr.add(idx));
core::ptr::write(start_ptr.add(idx), new_item);
}
}

This isn't too big of an issue right now because Topq currently leaks memory when it goes out of scope because the queue is wrapped in MaybeUninit. However, this can lead to double-frees if Topq was updated to free the memory or if someone called these methods indirectly through their drop code.

Namely, if the user provided type T panics during the drop_in_place operations, the Topq can be left in an inconsistent state and when it unwinds it can cause the same element to be dropped again.

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

1 participant