Skip to content

Rust: Update SqlxQuery, SqlxExecute to use getCanonicalPath #19802

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from
+20 −24
Draft

Rust: Update SqlxQuery, SqlxExecute to use getCanonicalPath #19802

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
dc08274
Rust: Update SqlxQuery, SqlxExecute from getResolvedPath -> getCanoni…
geoffw0 Jun 17, 2025
f3b5cc7
Merge branch 'main' into sqlx
geoffw0 Jul 8, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 9 additions & 5 deletions rust/ql/lib/codeql/rust/frameworks/Sqlx.qll
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@
private import rust
private import codeql.rust.Concepts
private import codeql.rust.dataflow.DataFlow
private import codeql.rust.internal.TypeInference
private import codeql.rust.internal.Type

/**
* A call to `sqlx::query` and variations.
Expand All @@ -14,11 +16,12 @@ private class SqlxQuery extends SqlConstruction::Range {

SqlxQuery() {
this.asExpr().getExpr() = call and
call.getFunction().(PathExpr).getResolvedPath() =
call.getStaticTarget().(Addressable).getCanonicalPath() =
[
"crate::query::query", "crate::query_as::query_as", "crate::query_with::query_with",
"crate::query_as_with::query_as_with", "crate::query_scalar::query_scalar",
"crate::query_scalar_with::query_scalar_with", "crate::raw_sql::raw_sql"
"sqlx_core::query::query", "sqlx_core::query_as::query_as",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This QLL file could be replaced by models as data, right?

"sqlx_core::query_with::query_with", "sqlx_core::query_as_with::query_as_with",
"sqlx_core::query_scalar::query_scalar", "sqlx_core::query_scalar_with::query_scalar_with",
"sqlx_core::raw_sql::raw_sql"
]
}

Expand All @@ -33,7 +36,8 @@ private class SqlxExecute extends SqlExecution::Range {

SqlxExecute() {
this.asExpr().getExpr() = call and
call.(Resolvable).getResolvedPath() = "crate::executor::Executor::execute"
call.getStaticTarget().(Addressable).getCanonicalPath() =
"sqlx_core::executor::Executor::execute"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess this should be something like

Suggested change
"sqlx_core::executor::Executor::execute"
"<sqlx_core::executor::Executor>::execute"

}

override DataFlow::Node getSql() { result.asExpr().getExpr() = call.getArgList().getArg(0) }
Expand Down
8 changes: 0 additions & 8 deletions rust/ql/test/query-tests/security/CWE-089/SqlInjection.expected
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,4 @@
#select
| sqlx.rs:67:26:67:48 | unsafe_query_1.as_str() | sqlx.rs:47:22:47:35 | ...::args | sqlx.rs:67:26:67:48 | unsafe_query_1.as_str() | This query depends on a $@. | sqlx.rs:47:22:47:35 | ...::args | user-provided value |
| sqlx.rs:69:30:69:52 | unsafe_query_2.as_str() | sqlx.rs:48:25:48:46 | ...::get | sqlx.rs:69:30:69:52 | unsafe_query_2.as_str() | This query depends on a $@. | sqlx.rs:48:25:48:46 | ...::get | user-provided value |
| sqlx.rs:78:25:78:47 | unsafe_query_1.as_str() | sqlx.rs:47:22:47:35 | ...::args | sqlx.rs:78:25:78:47 | unsafe_query_1.as_str() | This query depends on a $@. | sqlx.rs:47:22:47:35 | ...::args | user-provided value |
| sqlx.rs:80:29:80:51 | unsafe_query_2.as_str() | sqlx.rs:48:25:48:46 | ...::get | sqlx.rs:80:29:80:51 | unsafe_query_2.as_str() | This query depends on a $@. | sqlx.rs:48:25:48:46 | ...::get | user-provided value |
edges
Expand All @@ -15,14 +13,10 @@ edges
| sqlx.rs:48:25:48:78 | ... .unwrap() | sqlx.rs:48:25:48:85 | ... .text() [Ok] | provenance | MaD:9 |
| sqlx.rs:48:25:48:85 | ... .text() [Ok] | sqlx.rs:48:25:48:118 | ... .unwrap_or(...) | provenance | MaD:5 |
| sqlx.rs:48:25:48:118 | ... .unwrap_or(...) | sqlx.rs:48:9:48:21 | remote_string | provenance | |
| sqlx.rs:53:9:53:22 | unsafe_query_1 [&ref] | sqlx.rs:67:26:67:48 | unsafe_query_1.as_str() | provenance | MaD:7 |
| sqlx.rs:53:9:53:22 | unsafe_query_1 [&ref] | sqlx.rs:67:26:67:48 | unsafe_query_1.as_str() | provenance | MaD:6 |
| sqlx.rs:53:9:53:22 | unsafe_query_1 [&ref] | sqlx.rs:78:25:78:47 | unsafe_query_1.as_str() | provenance | MaD:7 |
| sqlx.rs:53:9:53:22 | unsafe_query_1 [&ref] | sqlx.rs:78:25:78:47 | unsafe_query_1.as_str() | provenance | MaD:6 |
| sqlx.rs:53:26:53:36 | &arg_string [&ref] | sqlx.rs:53:9:53:22 | unsafe_query_1 [&ref] | provenance | |
| sqlx.rs:53:27:53:36 | arg_string | sqlx.rs:53:26:53:36 | &arg_string [&ref] | provenance | |
| sqlx.rs:54:9:54:22 | unsafe_query_2 [&ref] | sqlx.rs:69:30:69:52 | unsafe_query_2.as_str() | provenance | MaD:7 |
| sqlx.rs:54:9:54:22 | unsafe_query_2 [&ref] | sqlx.rs:69:30:69:52 | unsafe_query_2.as_str() | provenance | MaD:6 |
| sqlx.rs:54:9:54:22 | unsafe_query_2 [&ref] | sqlx.rs:80:29:80:51 | unsafe_query_2.as_str() | provenance | MaD:7 |
| sqlx.rs:54:9:54:22 | unsafe_query_2 [&ref] | sqlx.rs:80:29:80:51 | unsafe_query_2.as_str() | provenance | MaD:6 |
| sqlx.rs:54:26:54:39 | &remote_string [&ref] | sqlx.rs:54:9:54:22 | unsafe_query_2 [&ref] | provenance | |
Expand Down Expand Up @@ -55,8 +49,6 @@ nodes
| sqlx.rs:54:9:54:22 | unsafe_query_2 [&ref] | semmle.label | unsafe_query_2 [&ref] |
| sqlx.rs:54:26:54:39 | &remote_string [&ref] | semmle.label | &remote_string [&ref] |
| sqlx.rs:54:27:54:39 | remote_string | semmle.label | remote_string |
| sqlx.rs:67:26:67:48 | unsafe_query_1.as_str() | semmle.label | unsafe_query_1.as_str() |
| sqlx.rs:69:30:69:52 | unsafe_query_2.as_str() | semmle.label | unsafe_query_2.as_str() |
| sqlx.rs:78:25:78:47 | unsafe_query_1.as_str() | semmle.label | unsafe_query_1.as_str() |
| sqlx.rs:80:29:80:51 | unsafe_query_2.as_str() | semmle.label | unsafe_query_2.as_str() |
subpaths
22 changes: 11 additions & 11 deletions rust/ql/test/query-tests/security/CWE-089/sqlx.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,14 +61,14 @@ async fn test_sqlx_mysql(url: &str, enable_remote: bool) -> Result<(), sqlx::Err
let prepared_query_1 = String::from("SELECT * FROM people WHERE firstname=?"); // (prepared arguments are safe)

// direct execution
let _ = conn.execute(safe_query_1.as_str()).await?; // $ sql-sink
let _ = conn.execute(safe_query_2.as_str()).await?; // $ sql-sink
let _ = conn.execute(safe_query_3.as_str()).await?; // $ sql-sink
let _ = conn.execute(unsafe_query_1.as_str()).await?; // $ sql-sink Alert[rust/sql-injection]=args1
let _ = conn.execute(safe_query_1.as_str()).await?; // $ MISSING: sql-sink
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

While we are able to infer that conn has type PoolConnection, we cannot currently infer that is in fact has type PoolConnection<MySql> (#19954 or similar is needed for that).

Once we are able to do that, we need implicit dereferencing via

impl<DB: Database> Deref for PoolConnection<DB> {
    type Target = DB::Connection;

    fn deref(&self) -> &Self::Target {
        &self.live.as_ref().expect(EXPECT_MSG).raw
    }
}

to get to the type &MySqlConnection, and then via impl<'c> Executor<'c> for &'c mut MySqlConnection we can find execute.

let _ = conn.execute(safe_query_2.as_str()).await?; // $ MISSING: sql-sink
let _ = conn.execute(safe_query_3.as_str()).await?; // $ MISSING: sql-sink
let _ = conn.execute(unsafe_query_1.as_str()).await?; // $ MISSING: sql-sink Alert[rust/sql-injection]=args1
if enable_remote {
let _ = conn.execute(unsafe_query_2.as_str()).await?; // $ sql-sink Alert[rust/sql-injection]=remote1
let _ = conn.execute(unsafe_query_3.as_str()).await?; // $ sql-sink MISSING: Alert[rust/sql-injection]=remote1
let _ = conn.execute(unsafe_query_4.as_str()).await?; // $ sql-sink MISSING: Alert[rust/sql-injection]=remote1
let _ = conn.execute(unsafe_query_2.as_str()).await?; // $ MISSING: sql-sink Alert[rust/sql-injection]=remote1
let _ = conn.execute(unsafe_query_3.as_str()).await?; // $ MISSING: sql-sink Alert[rust/sql-injection]=remote1
let _ = conn.execute(unsafe_query_4.as_str()).await?; // $ MISSING: sql-sink Alert[rust/sql-injection]=remote1
}

// prepared queries
Expand Down Expand Up @@ -103,9 +103,9 @@ async fn test_sqlx_sqlite(url: &str, enable_remote: bool) -> Result<(), sqlx::Er
let prepared_query_1 = String::from("SELECT * FROM people WHERE firstname=?"); // (prepared arguments are safe)

// direct execution (with extra variants)
let _ = conn.execute(safe_query_1.as_str()).await?; // $ sql-sink
let _ = conn.execute(safe_query_1.as_str()).await?; // $ MISSING: sql-sink
if enable_remote {
let _ = conn.execute(unsafe_query_1.as_str()).await?; // $ sql-sink MISSING: Alert[rust/sql-injection]=remote2
let _ = conn.execute(unsafe_query_1.as_str()).await?; // $ MISSING: sql-sink Alert[rust/sql-injection]=remote2
}
// ...
let _ = sqlx::raw_sql(safe_query_1.as_str()).execute(&mut conn).await?; // $ sql-sink
Expand Down Expand Up @@ -176,9 +176,9 @@ async fn test_sqlx_postgres(url: &str, enable_remote: bool) -> Result<(), sqlx::
let prepared_query_1 = String::from("SELECT * FROM people WHERE firstname=$1"); // (prepared arguments are safe)

// direct execution
let _ = conn.execute(safe_query_1.as_str()).await?; // $ sql-sink
let _ = conn.execute(safe_query_1.as_str()).await?; // $ MISSING: sql-sink
if enable_remote {
let _ = conn.execute(unsafe_query_1.as_str()).await?; // $ sql-sink MISSING: Alert[rust/sql-injection]=remote3
let _ = conn.execute(unsafe_query_1.as_str()).await?; // $ MISSING: sql-sink Alert[rust/sql-injection]=remote3
}

// prepared queries
Expand Down