sciter-sdk / rust-sciter

Rust bindings for Sciter
https://sciter.com
MIT License
804 stars 75 forks source link

Unsound implementation of `video::AssetPtr::<T>::get` #143

Open shinmao opened 9 months ago

shinmao commented 9 months ago

Hi, I am the security research from SunLab. We are running our tools on open-source repositories and found the following function could be unsound.

The source of unsoundness

https://github.com/sciter-sdk/rust-sciter/blob/789013a5353826b681c896eef489a450ece84c9c/src/video.rs#L476-L479 Here, the raw pointer to T could be cast to the pointer of iasset.

pub struct AssetPtr<T> {
    ptr: *mut T,
}

Therefore, the attacker just needs to specify T aligned to lower bytes than iasset. Then, the call to adopt will trigger get with undefined behavior.

To reproduce the bug

use sciter::video::AssetPtr;

fn main() {
    let mut a: u8 = 1;
    let _ = AssetPtr::adopt(&mut a as *mut u8);
}

run with miri,

error: Undefined Behavior: dereferencing pointer failed: alloc820 has size 1, so pointer to 8 bytes starting at offset 0 is out-of-bounds
   --> /home/rafael/.cargo/registry/src/index.crates.io-6f17d22bba15001f/sciter-rs-0.5.58/src/video.rs:478:12
    |
478 |         unsafe { &mut *ptr }
    |                  ^^^^^^^^^ dereferencing pointer failed: alloc820 has size 1, so pointer to 8 bytes starting at offset 0 is out-of-bounds