utils icon indicating copy to clipboard operation
utils copied to clipboard

zeroize: possible UB in `Zeroize` implementation for `Option<T>`

Open tarcieri opened this issue 4 years ago • 3 comments

Originally filed by @jessa0 as https://github.com/iqlusioninc/crates/issues/782:

It seems the Zeroize implementation for Option<T> where T: Zeroize has language-level UB here:

https://github.com/RustCrypto/utils/blob/00b569b52b6dece2a8ae881f8e5a150a45ccc674/zeroize/src/lib.rs#L330

I believe, as a repr(Rust) enum, the memory layout and set of valid bit-patterns for Option is not defined, and that setting an enum's storage to an invalid bit-pattern while a reference to it exists, even if the value is never read, is instant language-level UB. The documentation for Option does mention guarantees for several special cases, but the None case still isn't defined for many of those cases, and the Zeroize implementation is more generic than that. Here's an example of a miri error in such a situation, that scottmcm came up with on URLO here.

tarcieri avatar Nov 05 '21 01:11 tarcieri

The write_bytes docs seem fairly clear that writing invalid bit-patterns without reading should not be UB? (added from https://github.com/rust-lang/unsafe-code-guidelines/issues/330#issuecomment-1178725979).

Interestingly that miri playground example now works - I bisected it to nightly-2021-09-15, but can't see what changed then. It might be OK for other reasons though.

~As an aside, I wonder if the Z: Zeroize bound could be removed from Option, allowing Option<Z> to zeroize any uncooperative types...?~ (no, that won't work for types with heap allocations)

mkj avatar Dec 02 '22 13:12 mkj

Reads are still possible after Zeroize::zeroize since the API doesn't require dropping the value, although the question is whether we need to worry about reads after None has been written to the value

tarcieri avatar Dec 08 '22 00:12 tarcieri