vhbit / lmdb-rs

Rust bindings for LMDB
MIT License
114 stars 46 forks source link

impl `from_mdb_value` for bool is unsound #67

Open shinmao opened 1 year ago

shinmao commented 1 year ago

The source of unsoundness

https://github.com/vhbit/lmdb-rs/blob/3a4bd66eb92716e5115568bc101246255dececf7/src/traits.rs#L135-L142 The function from_mdb_value is implemented through macro on several primitive types including integers, float numbers, and bool. However, it is unsound to transmute data with arbitrary bit patterns to bool.

To reproduce the bug

use lmdb_rs_m::core::MdbValue;
use lmdb_rs_m::FromMdbValue;

fn main() {
    let a: i32 = 3;
    let mdbval = MdbValue::new_from_sized(&a);
    let res = bool::from_mdb_value(&mdbval);
    println!("{:?}", res);
}

run with Miri would show that UB happen.

    |
157 | mdb_for_primitive!(bool);
    | ^^^^^^^^^^^^^^^^^^^^^^^^ constructing invalid value: encountered 0x03, but expected a boolean
    |

UB should not happen with safe function in Rust.

shinmao commented 1 year ago

Misaligned pointer dereference could also happens here:

fn main() {
    let a: i32 = 3;
    let mdbval = MdbValue::new_from_sized(&a);
    let res = i64::from_mdb_value(&mdbval);
    println!("{:?}", res);
}

run the code and panic

thread 'main' panicked at 'misaligned pointer dereference: address must be a multiple of 0x8 but is 0x7ffeedce1fbc', /${HOME}/.cargo/registry/src/index.crates.io-6f17d22bba15001f/lmdb-rs-m-0.7.7/src/traits.rs:154:1

At line 138, *i32 behind *c_void is indirectly transmuted to *mut i64 leading to misaligned pointer and dereference at line 139.

shinmao commented 1 year ago

[UPDATED] The implementation here could also cause to another undefined behavior. To reproduce the bug, please see the following code:

#[repr(align(2))]
#[derive(Copy, Clone, Debug)]
struct Padding {
    a: u8,
    b: u16,
    c: u8,
}

fn main() {
    let la = Padding { a: 10, b: 11, c: 12 };
    let mdbval = MdbValue::new_from_sized(&la);
    let res = i32::from_mdb_value(&mdbval);
    println!("{:?}", res);
}

to run with miri,

error: Undefined Behavior: using uninitialized data, but this operation requires initialized memory
   --> /home/rafael/.cargo/registry/src/index.crates.io-6f17d22bba15001f/lmdb-rs-m-0.7.7/src/traits.rs:152:1
    |
152 | mdb_for_primitive!(i32);
    | ^^^^^^^^^^^^^^^^^^^^^^^ using uninitialized data, but this operation requires initialized memory
    |
    = help: this indicates a bug in the program: it performed an invalid operation, and caused Undefined Behavior