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

Custom Cell implementation is unsound #160

Closed
Tracked by #327
Shnatsel opened this issue Jul 19, 2020 · 0 comments · Fixed by #161
Closed
Tracked by #327

Custom Cell implementation is unsound #160

Shnatsel opened this issue Jul 19, 2020 · 0 comments · Fixed by #161

Comments

@Shnatsel
Copy link
Contributor

Shnatsel commented Jul 19, 2020

Right now there is no mechanism in Cell to track whether a mutable reference to the data is already acquired. Thus it is possible to obtain several mutable references to the same memory location by calling Cell::get_mut() repeatedly:

let mycell = Cell::new(vec![1,2,3]);
let ref1 = mysell.get_mut();
let ref2 = mysell.get_mut(); // obtained a second mutable reference; UB starts here

This may result in pretty much arbitrary memory corruption, most likely a use-after-free. Even though no code internal to Actix makes two obvious calls to get_mut() in a row, this behavior has been shown to be exploitable from the public API (see PoC that fails MIRI).

PR #158 has removed one instance of this Cell and all uses of it. However, there is another copy of it in the repository in actix-utils crate, and there are 23 calls to .get_mut().

The obvious fix is to replace all uses of custom Cell<T> with Rc<RefCell<T>> (the way it was before the introduction of a custom cell in 20b03a4), like it was done in #158.

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

Successfully merging a pull request may close this issue.

1 participant