search

isaacholt100 / bnum

Arbitrary, fixed size numeric types that extend the functionality of primitive numeric types in Rust.
https://crates.io/crates/bnum
Other
67 stars 8 forks source link

Soundness issue: Undefined Behavior due to writing to immutable pointer #36

Closed RalfJung closed 7 months ago

RalfJung commented 7 months ago

I stumbled upon this code:

https://github.com/isaacholt100/bnum/blob/6e26ddb6bc33d2219e094aeb8bfb2681a043ef2d/src/bint/endian.rs#L85-L92

That code has UB: uninit.as_ptr() is a read-only pointer (it derives from a shared reference), and so it must not be written to. Just casting from const to mut does not help, you must use a proper mutable pointer here.

copy_to_nonoverlapping got stabilized accidentally; there isn't actually a way to use it in const on stable in a UB-free way.

isaacholt100 commented 7 months ago

Hi @RalfJung, thank you very much for alerting me to this, I guess I'll have to make these non-const for the time being, or hopefully I can find a way of keeping it const without UB that isn't much slower.

RalfJung commented 7 months ago

Thanks a lot! Is there any chance of getting a semver-compatible update for bnum 0.8 that does the same thing? According to our crater results, that version occurs most often as dependency, and it would be great if people just had to do cargo update instead of having to wait for the version bump to propagate through the dependency tree.

isaacholt100 commented 7 months ago

No worries! Sure, can do, although the only major changes from 0.8 to 0.10 are the change from #[repr(Rust)] to #[repr(transparent)] of some structs and the fixing of a small number of incorrect implementations of methods, so users upgrading to 0.8 to 0.10 shouldn't have to change any existing code. If you still think this is a good idea though, I can certainly create a compatible update for v0.8: should this version be published as v0.8.1?

RalfJung commented 7 months ago

It's hard for me to say, I don't know the bnum ecosystem. The thing is just, the update from 0.8 to 0.10 has to be done by the author of the crate(s) that depend on 0.8 (and then the author of the next crate that depends on that, and so on). In contrast, an update to 0.8.1 can be done by anyone by just doing cargo update.

isaacholt100 commented 7 months ago

Thanks for explaining, I have now published v0.8.1 with this fix.

RalfJung commented 7 months ago

Thanks :)