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

Double Free due to Ownership Duplication #97

Open
GeorgeAndrou opened this issue Nov 13, 2024 · 0 comments
Open

Double Free due to Ownership Duplication #97

GeorgeAndrou opened this issue Nov 13, 2024 · 0 comments

Comments

@GeorgeAndrou
Copy link

GeorgeAndrou commented Nov 13, 2024

Hello,

We are @purseclab, and we are fuzzing Rust crates to identify memory violation bugs. Although we are aware that this crate is unmaintained, we noticed on crates.io that it is still being downloaded. Therefore, we decided to report a memory violation bug we discovered.

The PoC below causes a double-free memory violation.

PoC:

fn main (){
    let mut vec = vec![String::from("0"), String::from("1"), String::from("2"), String::from("3"), String::from("4"), String::from("5")];
    let mut slice_deq = slice_deque::SliceDeque::from(&vec[..]);
    println!("slice_deq_old = {:#?}", slice_deq);
    slice_deq.insert(3, String::from("X"));
    println!("slice_deq_new = {:#?}", slice_deq);
}

Bug Description:

We believe this bug is caused by slice_deque::SliceDeque::insert:

slice_deque/src/lib.rs

Lines 1390 to 1398 in 045fb28

} else {
// Shift elements towards the front
self.move_head_unchecked(-1);
let p = self.as_mut_ptr().add(index);
ptr::copy(p, p.sub(1), index);
p
};
ptr::write(p, element); // Overwritte
}

More specifically, in line 1393 the pointer p is initialized to point to the location where the provided element should be inserted. However, the way that p is used in the subsequent call to ptr::copy does not correctly shift the elements with indices less than index towards the front of the deque.

Also, is possible for the element at the location index + 1 to get duplicated. If the duplicated element implements the Drop trait, the destructor of SliceDeque will attempt to free the element twice, resulting in a double free memory violation.

How to Build and Run the PoC:

RUSTFLAGS="-Zsanitizer=address" cargo run

Output:

slice_deque_old = ["0", "1", "2", "3", "4", "5"]
slice_deque_new = ["", "0", "2", "X", "4", "4", "5"]
free(): double free detected in tcache 2
Aborted (core dumped)

Proposed Fix:

Change the lines 1393-1395 as follows,

} else {
    // Shift elements towards the front
    self.move_head_unchecked(-1);
    let p = self.as_mut_ptr();
    ptr::copy(p.add(1), p, index);
    p.add(index)
};

Output:

slice_deq_old = ["0", "1", "2", "3", "4", "5"]
slice_deq_new = ["0", "1", "2", "X", "3", "4", "5"]

Details:

  • Compiler Version: rustc 1.77.0-stable (aedd173a2 2024-03-17)
  • Library Version: slice-deque-0.3.0
  • OS: Ubuntu 22.04.5 LTS
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