-
Notifications
You must be signed in to change notification settings - Fork 10
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 claxon crate #4
Comments
I just skimmed the unsafes in the master branch. There's a mem::uninitialized in a bench, and there's 3 instances of setting a vec's len and then making a slice from it in this file: https://github.com/ruuda/claxon/blob/cd82be35f413940ba446d2a19f10d74b86466487/src/metadata.rs#L459-L461 There's a few get_unchecked's but they seemed alright. I think that's technically UB but I'll leave it to someone more knowledgeable if it's worth repalcing/how. |
A while ago I've found one instance of such code to be exploitable: https://rustsec.org/advisories/RUSTSEC-2018-0004.html The solution is to either zero-initialize the vector which is essentially free in one-shot programs, or appending to a vector instead of coercing it to a slice and writing to the slice. The reason Claxon does this is so that they don't have to carry a separate length field everywhere; I've detailed it more here. This could potentially be solved by providing a fixed-capacity buffer that nevertheless doesn't expose uninitialized memory. The API of buffer crate is very close, although I have not vetted the implementation. This would also help with the problem that if you want to read a fixed number of elements, you need to zero-initialize the memory because |
Looks like auditing is done here, we just need to land the PR. Some of the |
https://crates.io/crates/claxon
FLAC decoder in Rust, within 10% performance margin compared to reference C implementation. Not terribly widely used (300 downloads/day) but high-risk due to being a binary format decoder. Uses unsafe code.
It is highly optimized, but on an older compiler version, so it should be possible to either rewrite it safely or identify missing safe abstractions.
The text was updated successfully, but these errors were encountered: