sqlx icon indicating copy to clipboard operation
sqlx copied to clipboard

Transmute in postgres prepared queries

Open Ekleog opened this issue 6 months ago • 0 comments

Hey! I just spent today struggling with a very weird issue reported by my fuzzers. I finally managed to reproduce it as a bug in sqlx's prepared statement cache. Indeed, sqlx does not take into account the type of the binds when caching the statement.

This results in a free transmute on the wire between the sqlx client and postgresql. I think this deserves a RUSTSEC advisory, considering the potential impact (if the binds are attacker-controlled, then the attacker could inject arbitrary data in the binds by using collisions on the encoding; and the use case my fuzzers were fuzzing was exactly that). However, I'm sending this issue on the bugtracker directly, because I think such a situation would be unlikely at best — I definitely do have weird usage patterns of sqlx, I'll acknowledge it.

Here is the reproducer. Here, you can see the first query succeed as expected, but the second one fail because 1 gets encoded with null bytes, and thus postgresql rejects it as an invalid string. This can be used as a regression test.

Cargo.toml:

[package]
name = "sqlx-test"
version = "0.1.0"
edition = "2021"

[dependencies]
sqlx = { version = "0.7.3", features = ["postgres", "runtime-tokio"] }

lib.rs:

#[sqlx::test]
fn reproducer(db: sqlx::PgPool) {
    let mut conn = db.acquire().await.unwrap();
    sqlx::query("SELECT $1").bind("foo").execute(&mut *conn).await.unwrap();
    sqlx::query("SELECT $1").bind(0).execute(&mut *conn).await.unwrap();
}

You can also see the transmute happening with the following example, that will actually have an encoding conflict, and "foob" gets reinterpreted as an i32:

use sqlx::Row;

#[sqlx::test]
fn reproducer(db: sqlx::PgPool) {
    let mut conn = db.acquire().await.unwrap();
    sqlx::query("SELECT $1").bind(0i32).execute(&mut *conn).await.unwrap();
    panic!("{:?}", sqlx::query("SELECT $1").bind("foob").fetch_one(&mut *conn).await.unwrap().get::<i32, _>(0));
}

In this specific case because the type of the output is the same as the type of the input it's obvious what is happening, but if the bind were used inside a WHERE clause it could lead to issues.

This, in particular, can happen with binds on JSONB values or json_paths, which are my use cases.

So here it is. I think sqlx should probably also key the prepared statement cache with the SQL type of the binds, to avoid such an issue?

Anyway, thank you for all you do on sqlx, despite the issues my application-level fuzzers find there it's very helpful! :D (In case you're curious, the fuzzers and the source code I'm actually fuzzing are here, and I'm running them all on my laptop, they usually find at least one issue in my application code overnight each time, though the last few times were the issues on sqlx I reported to you)

Ekleog avatar Jan 11 '24 01:01 Ekleog