BurntSushi / byteorder

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

Unsound usages of unsafe implementation from `[T]` to `[u8]` #207

Closed llooFlashooll closed 6 months ago

llooFlashooll commented 6 months ago

Hi, I am scanning the byteorder in the latest version with my own static analyzer tool.

Unsafe conversion found at src/io.rs:1591:5: 1591:66; SliceFromRawPartsMut:

unsafe fn slice_to_u8_mut<T: Copy>(slice: &mut [T]) -> &mut [u8] {
    use std::mem::size_of;

    let len = size_of::<T>() * slice.len();
    slice::from_raw_parts_mut(slice.as_mut_ptr() as *mut u8, len)
}

This unsound implementation would create a misalignment issues.

The problematic value is further manipulated at: src/io.rs:565:13: 565:33, src/io.rs:600:13: 600:33, src/io.rs:638:13: 638:33, src/io.rs:679:13: 679:33, src/io.rs:718:9: 718:29, src/io.rs:750:13: 750:33, src/io.rs:785:13: 785:33, src/io.rs:823:13: 823:33, src/io.rs:864:13: 864:33, src/io.rs:905:13: 905:33, src/io.rs:990:13: 990:33

This would potentially cause undefined behaviors in Rust. If we further manipulate the problematic converted types, it would potentially lead to different consequences. I am reporting this issue for your attention.

BurntSushi commented 6 months ago

There is no misalignment issue because u8 has an alignment that is compatible with all other alignments.

This is the third false positive safety issue you've filed on my repos. I'm going to have to ask you to stop filing issues in the future on my repos unless you have a Miri stack trace. (It's possible to have UB without a Miri stack trace, but I don't know how else to increase the signal to noise ratio that you're producing.)

BurntSushi commented 6 months ago

Also, why is the activity on your GitHub private? I'd like to see whether you're filing these types of issues on other projects.

You also don't disclose what tool you're using.

Please increase your transparency or stop filing issues on my repos.

llooFlashooll commented 6 months ago

Ok, thank you. I do not intend to do this. That's my bad. I will carefully handle each reported issue. I am anonymous since I am undergoing research. Hope you can understand. Thanks again.

BurntSushi commented 6 months ago

Once upon a time, I was a PhD student too. And I wasn't anonymous.

Basically, I'm appreciate of issues, but if you're just some human front for a tool with a high false positive rate, then you're going to be wasting a lot of peoples' time. And that's annoying.

llooFlashooll commented 6 months ago

Thank you very much for your instructions. I would of course be willing and happy to open source and share all finally. I would only report when it's TP. Those are my bad.