meilisearch / heed

A fully typed LMDB wrapper with minimum overhead 🐦
https://docs.rs/heed
MIT License
519 stars 52 forks source link

`ReservedSpace` is unsound due to not being bound to its origin transaction. #258

Closed nolanderc closed 2 months ago

nolanderc commented 2 months ago

ReservedSpace is currently defined without lifetimes: https://github.com/meilisearch/heed/blob/34063834019b41740b684a5aa3ebef8f872579f4/heed/src/reserved_space.rs#L8-L12

However, the callback functions in which it is used expose it as FnOnce(&mut ReservedSpace). This means it is possible to write something like the following to modify the reserved space in safe Rust:

let env_foo = heed::Env::open(...)?;
txn_foo = env_foo.write_txn()?;
let db_foo = env1.create_database(&mut txn_foo, None);  

let env_bar = heed::Env::open(...)?;
txn_bar = env_bar.write_txn()?;
let db_bar = env1.create_database(&mut txn_bar, None);  

// reserve an outer space
db_foo.put_reserved(&mut txn_foo, key_foo, 3, |space_foo| {

    // fill the outer space
    space_bar.write_all(b"baz");

    // reserve an inner space
    db_bar.put_reserved(&mut txn_bar, key_bar, 6, |space_bar| {

        // swap them:
        std::mem::swap(space_foo, space_bar);

        // `space_foo` now points into `db_bar`
        // `space_bar` now points into `db_foo`, which is fully written

    })?; // returns Ok(())

    // `space_foo` still points into `db_bar`.

    db_bar.delete(&mut txn_bar, key_bar)?;

    // `space_foo` points to deleted value.

    space_foo.write_all(b"foobar")?; // UNDEFINED BEHAVIOUR!

})?;

This could be fixed by attaching a lifetime to ReservedSpace<'a> which is only valid for the duration of the closure.