Undefined Behaviour in uint::from_big_endian

[vakaras]

It seems that for a completely safe Rust client code it is possible to trigger undefined behaviour by calling the uint::from_big_endian function. Miri reports undefined behaviour for the following two examples:

Example 1

Code

use uint::construct_uint;

construct_uint! {
    pub struct U1024(1);
}

fn main() {    
    let _ = U1024::from_big_endian(&[]);
}

Miri command and output

cargo +nightly-2020-04-27 miri 
    Checking uint-test v0.1.0 (/tmp/uint-test)
error: Undefined Behavior: overflowing in-bounds pointer arithmetic
   --> /home/ANT.AMAZON.COM/astrauv/.rustup/toolchains/nightly-2020-04-27-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/src/libcore/ptr/const_ptr.rs:160:9
    |
160 |         intrinsics::offset(self, count)
    |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ overflowing in-bounds pointer arithmetic
    |
    = help: this indicates a bug in the program: it performed an invalid operation, and caused Undefined Behavior
    = help: see https://doc.rust-lang.org/nightly/reference/behavior-considered-undefined.html for further information
            
    = note: inside `std::ptr::const_ptr::<impl *const u8>::offset` at /home/ANT.AMAZON.COM/astrauv/.rustup/toolchains/nightly-2020-04-27-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/src/libcore/ptr/const_ptr.rs:160:9
note: inside `U1024::from_big_endian` at /home/ANT.AMAZON.COM/astrauv/.cargo/git/checkouts/parity-common-b7e6dd3e48c6ce77/692aa9d/uint/src/uint.rs:1124:26

Problem

When the slice size is 0, the initial value for slice_ptr points outside of the allocated range. This is undefined behaviour according to Rust semantics.

Suggested Fix

Add an early return:

if slice.is_empty() {
    return ret;
}

Example 2

Code

use uint::construct_uint;

construct_uint! {
    pub struct U1024(1);
}

fn main() {    
    let _ = U1024::from_big_endian(&[0]);
}

Miri command and output

cargo +nightly-2020-04-27 miri 
    Checking uint-test v0.1.0 (/tmp/uint-test)
error: Undefined Behavior: overflowing in-bounds pointer arithmetic
   --> /home/ANT.AMAZON.COM/astrauv/.rustup/toolchains/nightly-2020-04-27-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/src/libcore/ptr/const_ptr.rs:160:9
    |
160 |         intrinsics::offset(self, count)
    |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ overflowing in-bounds pointer arithmetic
    |
    = help: this indicates a bug in the program: it performed an invalid operation, and caused Undefined Behavior
    = help: see https://doc.rust-lang.org/nightly/reference/behavior-considered-undefined.html for further information
            
    = note: inside `std::ptr::const_ptr::<impl *const u8>::offset` at /home/ANT.AMAZON.COM/astrauv/.rustup/toolchains/nightly-2020-04-27-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/src/libcore/ptr/const_ptr.rs:160:9
note: inside `U1024::from_big_endian` at /home/ANT.AMAZON.COM/astrauv/.cargo/git/checkouts/parity-common-b7e6dd3e48c6ce77/692aa9d/uint/src/uint.rs:1128:19

Problem

During the last iteration of the for loop, slice_ptr is assigned a value that points outside of the allocated range. This is undefined behaviour according to Rust semantics.

Suggested Fix

Either add a guard that ensures that slice_ptr is not assigned in the last iteration, or unroll the loop once to avoid the unnecessary assignment to slice_ptr

[vakaras]

Thank you for fixing it so fast! Are you planning to release the fixed version on crates.io any time soon?

[ordian]

@vakaras Thanks for the report! Yes, we're planning to release it today #382