Skip to content

Unsoundness in cmp #45

@lwz23

Description

@lwz23

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);
}

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions