Repository navigation
Unsoundness in cmp #45
Description
Hello, thank you for your contribution in this project, I an testing our static analysis tool in github's Rust project and I notice the following code:
pub fn erl_partial_cmp(&self, other: &Self) -> Option<Ordering> {
Some(self.cmp(other))
}
fn cmp(&self, other: &Variant) -> Ordering {
match (self, other) {
(Variant::Nil(..), Variant::Nil(..)) => Ordering::Equal,
(Variant::Integer(i1), Variant::Integer(i2)) => i1.cmp(i2),
(Variant::Float(f1), Variant::Float(f2)) => f1.partial_cmp(f2).unwrap(),
(Variant::Atom(a1), Variant::Atom(a2)) => atom::cmp(*a1, *a2),
(Variant::Pid(p1), Variant::Pid(p2)) => p1.cmp(p2),
(Variant::Port(p1), Variant::Port(p2)) => p1.cmp(p2),
(Variant::Cons(l1), Variant::Cons(l2)) => unsafe { (**l1).cmp(&(**l2)) }
(Variant::Pointer(p1), Variant::Pointer(p2)) => {
unsafe {
let h1 = **p1;
let h2 = **p2;
if h1 == h2 {
match h1 {
BOXED_TUPLE => {
let t1 = &*(*p1 as *const Tuple);
let t2 = &*(*p2 as *const Tuple);
t1.cmp(t2)
}
BOXED_REF => {
let r1 = &(*(*p1 as *const Boxed<process::Ref>)).value;
let r2 = &(*(*p2 as *const Boxed<process::Ref>)).value;
r1.cmp(r2)
}
BOXED_MAP => {
let m1 = &*(*p1 as *const Boxed<Map>);
let m2 = &*(*p2 as *const Boxed<Map>);
m1.value.0.cmp(&m2.value.0)
}
BOXED_CLOSURE => unreachable!(),
BOXED_BIGINT => {
let i1 = &(*(*p1 as *const Boxed<BigInt>)).value;
let i2 = &(*(*p2 as *const Boxed<BigInt>)).value;
i1.cmp(i2)
}
BOXED_BINARY => {
let b1 = &*(*p1 as *const Boxed<bitstring::RcBinary>);
let b2 = &*(*p2 as *const Boxed<bitstring::RcBinary>);
b1.value.data.cmp(&b2.value.data)
}
BOXED_SUBBINARY => {
let b1 = &(*(*p1 as *const Boxed<bitstring::SubBinary>))
.value;
let b2 = &(*(*p2 as *const Boxed<bitstring::SubBinary>))
.value;
let cmp = (b1.size * 8 + b1.bitsize)
.cmp(&(b2.size * 8 + b2.bitsize));
if cmp == std::cmp::Ordering::Equal {
bitstring::cmp_bits(
b1.original.data.as_ptr(),
(b1.offset * 8) + b1.bit_offset as usize,
b2.original.data.as_ptr(),
(b2.offset * 8) + b2.bit_offset as usize,
b2.bitsize + (b2.size * 8),
)
} else {
cmp
}
}
_ => unimplemented!("cmp for {}", h1),
}
} else {
match (h1, h2) {
(BOXED_BINARY, BOXED_SUBBINARY) => unimplemented!(),
(BOXED_SUBBINARY, BOXED_BINARY) => unimplemented!(),
_ => unimplemented!(),
}
}
}
}
(Variant::Integer(i1), Variant::Pointer(p2)) => {
unsafe {
if **p2 != BOXED_BIGINT {
unreachable!()
}
let i2 = &(*(*p2 as *const Boxed<BigInt>)).value;
BigInt::from(*i1).cmp(i2)
}
}
(Variant::Pointer(p1), Variant::Integer(i2)) => {
unsafe {
if **p1 != BOXED_BIGINT {
unreachable!()
}
let i1 = &(*(*p1 as *const Boxed<BigInt>)).value;
i1.cmp(&BigInt::from(*i2))
}
}
_ => unimplemented!("cmp for {:?} and {:?}", self, other),
}
}
The unsoundness occurs because:
The public function erl_partial_cmp directly calls cmp without any validation
The cmp function uses unsafe code to dereference pointers without checking if they're valid
When comparing a Variant::Pointer with a Variant::Integer, it performs **p1 without verifying that the pointer is valid
The code then performs a check if **p1 != BOXED_BIGINT, but by this point, undefined behavior has already occurred if the pointer was invalid
The absence of pointer validation before dereferencing creates a direct path from user input to memory safety violation.
pub fn erl_partial_cmp -> fn cmp
POC
fn main() {
// Create a Variant with a null or dangling pointer
let variant1 = Variant::Pointer(Box::new(std::ptr::null_mut()));
// Create a second variant to compare against
let variant2 = Variant::Integer(42);
// Call the public function that will lead to undefined behavior
let ordering = variant1.erl_partial_cmp(&variant2);
println!("Result: {:?}", ordering);
}
Activity