As part of auditing rusqlite's unsafe, I noticed that Connection::from_handle is very difficult to use correctly, even privately to the crate. While checking the use-sites, I noticed that you can safely install a collation handler, which gives the user access to a &Connection which is non-owning. Most of the constraints are enforced by the fact that you gave the user a &Connection and it is being called by the C library (which means it's already on a thread that has the connection borrowed and non-transferrable), but the InterruptHandler abstraction only works because you null it out during destruction. In a non-owned connection, you have a different copy of InterruptHandler, so the owned connection nulling it out won't clean things up, and the unowned one explicitly doesn't null out its InterruptHandler when dropped.
I think the easiest fix might be to make get_interrupt_handler require a &mut Connection even though that's a bit of a funny interface. Alternatively, you could make get_interrupt_handler panic if !owned, or make the destruction of a non-owned Connection null out any InterruptHandler's acquired from it.
mmaurer@curtana:~/sample-rusqlite$ cat Cargo.toml
[package]
name = "sample-rusqlite"
version = "0.1.0"
edition = "2021"
# See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html
[dependencies]
rusqlite = { version = "0.32.0", features = ["collation"] }
mmaurer@curtana:~/sample-rusqlite$ cat src/main.rs
use rusqlite::{Connection, InterruptHandle};
use std::sync::Mutex;
static EXFIL: Mutex<Option<InterruptHandle>> = Mutex::new(None);
fn exfil<E>(c: &Connection, _: &str) -> Result<(), E> {
println!("Exfiltrating interrupt handle object from an unowned connection");
*EXFIL.lock().unwrap() = Some(c.get_interrupt_handle());
Ok(())
}
fn main() {
let db0 = Connection::open_in_memory().unwrap();
db0.collation_needed(exfil).unwrap();
// This statement will fail because the "bar" collation isn't real, so it
// will ask `exfil` if it'd like to install the bar collation. It'll refuse,
// but exfil the connection as an IH.
assert!(db0
.prepare("WITH cte(c0) AS (VALUES ('v1'), ('v2')) SELECT DISTINCT c0 COLLATE bar FROM cte")
.is_err());
db0.close().unwrap();
// 0x350 is the size of `struct sqlite3` on my system, you may have a different value.
let x: Box<[u8]> = Box::new([0; 0x350]);
// Most likely, we've now stolen the database pointer;
for byte in &*x {
assert_eq!(*byte, 0, "Precondition failed");
}
EXFIL.lock().unwrap().as_ref().unwrap().interrupt();
for byte in &*x {
assert_eq!(*byte, 0, "Postcondition failed");
}
}
mmaurer@curtana:~/sample-rusqlite$ cargo run
Finished `dev` profile [unoptimized + debuginfo] target(s) in 0.06s
Running `target/debug/sample-rusqlite`
Exfiltrating interrupt handle object from an unowned connection
thread 'main' panicked at src/main.rs:30:9:
assertion `left == right` failed: Postcondition failed
left: 1
right: 0
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
mmaurer@curtana:~/sample-rusqlite$
As part of auditing
rusqlite's unsafe, I noticed thatConnection::from_handleis very difficult to use correctly, even privately to the crate. While checking the use-sites, I noticed that you can safely install a collation handler, which gives the user access to a&Connectionwhich is non-owning. Most of the constraints are enforced by the fact that you gave the user a&Connectionand it is being called by the C library (which means it's already on a thread that has the connection borrowed and non-transferrable), but theInterruptHandlerabstraction only works because you null it out during destruction. In a non-owned connection, you have a different copy ofInterruptHandler, so the owned connection nulling it out won't clean things up, and the unowned one explicitly doesn't null out itsInterruptHandlerwhen dropped.I think the easiest fix might be to make
get_interrupt_handlerrequire a&mut Connectioneven though that's a bit of a funny interface. Alternatively, you could makeget_interrupt_handlerpanic if!owned, or make the destruction of a non-ownedConnectionnull out anyInterruptHandler's acquired from it.