Added advisory for undefined behavior in openssl

[alex]

No description provided.

[Shnatsel]

Is the length of the slice attacker-controlled? If not, I would rather file this as informational = unsound.

Also, this makes me wonder if there are any other similarly problematic calls to slice::from_raw_parts in the codebase that should be fixed before we publish an advisory, so that users could upgrade all at once instead of getting several different notifications.

[alex]

This API is used all over the place, there's at least a few places where the string length will be attacker controlled, yes.

And there may be others, I filed this before doing the variant analysis.

[Shnatsel]

There's about a dozen places where a slice is constructed from raw parts:

https://github.com/search?q=repo%3Asfackler%2Frust-openssl%20slice%3A%3Afrom_raw_parts&type=code

Perhaps we should just wrap all of them in a helper function that explicitly checks for the zero length, just in case?

[alex]

It's not a bad idea, a quick review gives me a few that I'm skeptical of (but which I don't have a PoC for), and most of them aren't problems or already have checks.

[alex]

Ok, on further review (and reading OpenSSL's code) I don't see any others that should be able to trigger UB.

It may still be worth it to have a utility function that handles this case as a best practice, but I don't think it needs to block this advisory.

[Shnatsel]

Could you open a PR to refactor them with a helper function with a zero check then? And after that I'd be happy to publish an advisory once that's released to crates.io. Since it's not clear how one would go about exploiting this, I wouldn't want to publish a non-actionable advisory, or several different advisories.

[alex]

sfackler/rust-openssl#2268 has the refactor, but I'm not going to do another release for this in the absence of any way to trigger this issue.