rust-sciter icon indicating copy to clipboard operation
rust-sciter copied to clipboard

Unsoundness in get

Open lwz23 opened this issue 7 months ago • 2 comments

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 struct AssetPtr<T> {
    ptr: *mut T,
}

impl<T> AssetPtr<T> {
    /// Attach to an existing iasset pointer without reference increment.
    fn attach(lp: *mut T) -> Self {
        assert!(!lp.is_null());
        Self {
            ptr: lp
        }
    }
    /// Attach to an iasset pointer and increment its reference count.
    pub fn adopt(lp: *mut T) -> Self {
        let mut me = Self::attach(lp);
        me.get().add_ref();
        me
    }
    /// Get as an iasset type.
    fn get(&mut self) -> &mut iasset {
        let ptr = self.ptr as *mut iasset;
        unsafe { &mut *ptr }
    }
}

The issue is that AssetPtr::adopt is a public function that accepts any type T, but internally it assumes T can be safely cast to an iasset. There's no trait bound or type checking to ensure this is valid. The problematic code path is:

adopt takes any *mut T pointer It calls get() which unsafely casts self.ptr to *mut iasset and dereferences it It then calls add_ref() on the supposed iasset

Since there's no guarantee that T is actually an iasset or has the same memory layout, this creates undefined behavior when given a pointer to an incompatible type. The function provides a safe interface (no unsafe required to call it) but can lead to memory safety violations. A valid path to call this fn: pub fn adopt -> fn get

POC

fn main() {
    // Create a struct that is NOT an iasset
    struct NotAnIAsset {
        some_field: u32,
    }
    
    // Create an instance of NotAnIAsset
    let not_iasset = Box::new(NotAnIAsset { some_field: 42 });
    
    // Convert the Box to a raw pointer
    let raw_ptr = Box::into_raw(not_iasset);
    
    // Call the public function with a pointer to something that is not an iasset
    let asset_ptr = AssetPtr::adopt(raw_ptr as *mut NotAnIAsset);
    
    // The above call will cause undefined behavior when it tries to:
    // 1. Cast the NotAnIAsset pointer to iasset in the get() method
    // 2. Call add_ref() on something that doesn't have that method
}

lwz23 avatar Mar 04 '25 11:03 lwz23