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

Use-after-free on panic in client code #35

Closed
Shnatsel opened this issue Jun 26, 2019 · 4 comments
Closed

Use-after-free on panic in client code #35

Shnatsel opened this issue Jun 26, 2019 · 4 comments

Comments

@Shnatsel
Copy link
Contributor

If the code that uses libflate panics, it may trigger a use-after-free in libflate code. Since use-after-free usually poses an arbitrary code execution vulnerability, I will relay further details privately to the maintainer.

Code compiled with panic=abort is not affected. This can be used as a mitigation in the interim.

@Shnatsel
Copy link
Contributor Author

Shnatsel commented Jul 4, 2019

Fixed by #37. The problem is in this code:

libflate/src/gzip.rs

Lines 1122 to 1129 in cbf1289

let mut reader = mem::replace(&mut self.decoder, Err(unsafe { mem::uninitialized() }))
.ok()
.take()
.expect("Never fails")
.into_inner();
match Header::read_from(&mut reader) {
Err(e) => {
mem::forget(mem::replace(&mut self.decoder, Err(reader)));

The mere existence of mem::uninitialized() of a generic type is already undefined behavior. It's not clear how bad it is in practice exactly if this memory is never read. However, the definitely practical issue comes from the fact that a panic may happen on line 1127, before reaching mem::forget(), because the underlying reader provided by the API client may panic. It the panic occurs, drop() is called on the uninitialized memory, which is equivalent to a use-after-free bug, and use-after-free bugs are often exploitable.

@Shnatsel
Copy link
Contributor Author

Shnatsel commented Jul 7, 2019

The issue was introduced in commit deadd75 and affects releases 0.1.14 and onwards until the fix in 0.1.25.

@sile please yank versions 0.1.14 to 0.1.24 inclusive from crates.io. I'll handle the security advisory.

@sile
Copy link
Owner

sile commented Jul 8, 2019

@Shnatsel

please yank versions 0.1.14 to 0.1.24 inclusive from crates.io.

Done. Thank you for your contribution and advice.

dbrgn added a commit to dbrgn/sekursranko that referenced this issue Jul 8, 2019
This includes two security updates in indirectly used libraries:

- servo/rust-smallvec#148
- sile/libflate#35
@Shnatsel
Copy link
Contributor Author

Closing now that the fix is released, vulnerable versions are yanked and a security advisory is filed.

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