BurntSushi / byteorder

Rust library for reading/writing numbers in big-endian and little-endian.
The Unlicense
984 stars 144 forks source link

Bound `slice_to_u8_mut` by bytemuck::AnyBitPattern or bytemuck::Pod to ensure soundness #212

Closed danakj closed 2 months ago

danakj commented 2 months ago

The slice_to_u8_mut method docs say:

/// This function is wildly unsafe because it permits arbitrary modification of
/// the binary representation of any `Copy` type. Use with care. It's intended
/// to be called only where `T` is a numeric type.

While it's only used internally, this can be bounded to check for soundness at compile time by using the bytemuck trait AnyBitPattern or Pod (which implies the former). Pod comes defined for all numeric types.

BurntSushi commented 2 months ago

I'm aware. But I'm not adding a dependency for it. It is nowhere near worth doing.

BurntSushi commented 2 months ago

For anyone following along at home, this is the function in question:

https://github.com/BurntSushi/byteorder/blob/18f32ca3a41c9823138e782752bc439e99ef7ec8/src/io.rs#L1587

It is appropriately marked as unsafe. And while the docs could be better, they do outline its safety contract. So it is up to the caller to ensure sound usage. There are no known unsound uses of this function.

Adding a dependency on a "safe transmute" crate would decrease the amount of unsafe in byteorder proper. But... It adds a dependency. byteorder is not changing much if at all, so I'm personally not too worried about something being misused here.

If safe transmute helpers were added to std (which I am strongly in favor of as a member of libs-api), then I would absolutely use them in byteorder.

BurntSushi commented 2 months ago

Also, hi @danakj! I used Openbox for many years back in the day. And it (along with xmonad) inspired me to write Wingo. :-)

danakj commented 2 months ago

Hi :) Thanks for the comments, that all makes sense.