Implementation of NotNan::cmp is unsound #150
Description
The current implementation of Ord for NotNan<T> is as follows:
impl<T: FloatCore> Ord for NotNan<T> {
fn cmp(&self, other: &NotNan<T>) -> Ordering {
match self.partial_cmp(other) {
Some(ord) => ord,
None => unsafe { unreachable_unchecked() },
}
}
}and the implementation of NotNan::<T>::new is:
pub fn new(val: T) -> Result<Self, FloatIsNan> {
match val {
ref val if val.is_nan() => Err(FloatIsNan),
val => Ok(NotNan(val)),
}
}this makes it possible to invoke undefined behaviour without using unsafe with the following code:
#[derive(PartialOrd, PartialEq)]
struct EvilFloat(f32);
impl FloatCore for EvilFloat {
fn is_nan(self) -> bool {
false
}
// ...
}
fn main() {
let evil_value = NotNan::new(EvilFloat(f32::NAN)).unwrap();
let x = NotNan::new(EvilFloat(0.0)).unwrap();
evil_value.cmp(&x);
}Full test case: https://gist.github.com/Earthcomputer/27d11c9c70c1335ae29cdf187362c4dd
Miri run:
error: Undefined Behavior: entering unreachable code
--> /home/joe/.cargo/registry/src/index.crates.io-6f17d22bba15001f/ordered-float-4.2.0/src/lib.rs:1168:30
|
1168 | None => unsafe { unreachable_unchecked() },
| ^^^^^^^^^^^^^^^^^^^^^^^ entering unreachable code
|
= 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: BACKTRACE:
= note: inside `<ordered_float::NotNan<EvilFloat> as std::cmp::Ord>::cmp` at /home/joe/.cargo/registry/src/index.crates.io-6f17d22bba15001f/ordered-float-4.2.0/src/lib.rs:1168:30: 1168:53
note: inside `main`
--> src/main.rs:128:5
|
128 | evil_value.cmp(&x);
| ^^^^^^^^^^^^^^^^^^
note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace