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

Audit flate2 #32

Closed
Shnatsel opened this issue Sep 3, 2019 · 5 comments
Closed

Audit flate2 #32

Shnatsel opened this issue Sep 3, 2019 · 5 comments

Comments

@Shnatsel
Copy link
Member

Shnatsel commented Sep 3, 2019

https://crates.io/crates/flate2

Frontend to a number of DEFLATE compression/decompression libraries: zlib, miniz and miniz_oxide. 11,000 downloads/day, exposed to untrusted data from the network through reqwest 😱

@Shnatsel
Copy link
Member Author

Shnatsel commented Sep 3, 2019

Following the work done by @oyvindln as part of #2, when using the Rust backend (miniz_oxide) it's already almost 100% safe. The only remaining unsafety in that path is passing a slice of uninitialized memory to be filled: https://github.com/alexcrichton/flate2-rs/blob/537fb77132a15b772fcc9c35a4c8c679d40aedf7/src/mem.rs#L317-L323

The outer function accepts the output buffer as &mut Vec<u8>, converts it to a slice internally and passes the slice as output buffer to the backend. If the backend does not overwrite the entire slice, this can become an exploitable security vulnerability.

Exposure of uninitialized memory can be easily avoided by passing the &mut Vec<u8> to miniz_oxide and decoding to it instead of decoding to a slice. This will likely require some additions to miniz_oxide APIs as well.

@Shnatsel
Copy link
Member Author

Shnatsel commented Nov 1, 2019

Opened an issue on flate2 project about the above: rust-lang/flate2-rs#220

@Shnatsel
Copy link
Member Author

Shnatsel commented Nov 1, 2019

MaybeUninit<T> aside, flate2 appears to be fully processed. The last two unsafe blocks cannot be killed off without losing performance - it would require a fixed-capacity Vec-like view of memory. I'll need to write an RFC for one at some point.

@Shnatsel Shnatsel closed this as completed Nov 1, 2019
@alex
Copy link
Member

alex commented Nov 1, 2019

@Shnatsel are we tracking the "if we had {X} we could remove unsafe" somewhere to have some ability to quantify relative value of different priorities?

@Shnatsel
Copy link
Member Author

Shnatsel commented Nov 1, 2019

Sort of? We have #34 for now, not sure if we need a more permanent place for collecting these.

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