diff --git a/rust/ql/lib/codeql/rust/frameworks/Sqlx.qll b/rust/ql/lib/codeql/rust/frameworks/Sqlx.qll index 5504993ab744..5b33f72fdf68 100644 --- a/rust/ql/lib/codeql/rust/frameworks/Sqlx.qll +++ b/rust/ql/lib/codeql/rust/frameworks/Sqlx.qll @@ -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. @@ -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", + "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" ] } @@ -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" } override DataFlow::Node getSql() { result.asExpr().getExpr() = call.getArgList().getArg(0) } diff --git a/rust/ql/test/query-tests/security/CWE-089/SqlInjection.expected b/rust/ql/test/query-tests/security/CWE-089/SqlInjection.expected index ab8e995be762..fcfa77cfda0f 100644 --- a/rust/ql/test/query-tests/security/CWE-089/SqlInjection.expected +++ b/rust/ql/test/query-tests/security/CWE-089/SqlInjection.expected @@ -1,8 +1,4 @@ #select -| sqlx.rs:66:26:66:46 | safe_query_3.as_str() | sqlx.rs:48:25:48:46 | ...::get | sqlx.rs:66:26:66:46 | safe_query_3.as_str() | This query depends on a $@. | sqlx.rs:48:25:48:46 | ...::get | user-provided value | -| 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:71:30:71:52 | unsafe_query_4.as_str() | sqlx.rs:48:25:48:46 | ...::get | sqlx.rs:71:30:71:52 | unsafe_query_4.as_str() | This query depends on a $@. | sqlx.rs:48:25:48:46 | ...::get | user-provided value | | sqlx.rs:77:25:77:45 | safe_query_3.as_str() | sqlx.rs:48:25:48:46 | ...::get | sqlx.rs:77:25:77:45 | safe_query_3.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 | @@ -24,22 +20,18 @@ edges | sqlx.rs:49:9:49:21 | remote_number | sqlx.rs:52:32:52:87 | MacroExpr | provenance | | | sqlx.rs:49:25:49:52 | remote_string.parse() [Ok] | sqlx.rs:49:25:49:65 | ... .unwrap_or(...) | provenance | MaD:7 | | sqlx.rs:49:25:49:65 | ... .unwrap_or(...) | sqlx.rs:49:9:49:21 | remote_number | provenance | | -| sqlx.rs:52:9:52:20 | safe_query_3 | sqlx.rs:66:26:66:46 | safe_query_3.as_str() | provenance | MaD:3 | | sqlx.rs:52:9:52:20 | safe_query_3 | sqlx.rs:77:25:77:45 | safe_query_3.as_str() | provenance | MaD:3 | | sqlx.rs:52:24:52:88 | res | sqlx.rs:52:32:52:87 | { ... } | provenance | | | sqlx.rs:52:32:52:87 | ...::format(...) | sqlx.rs:52:24:52:88 | res | provenance | | | sqlx.rs:52:32:52:87 | ...::must_use(...) | sqlx.rs:52:9:52:20 | safe_query_3 | provenance | | | sqlx.rs:52:32:52:87 | MacroExpr | sqlx.rs:52:32:52:87 | ...::format(...) | provenance | MaD:4 | | sqlx.rs:52:32:52:87 | { ... } | sqlx.rs:52:32:52:87 | ...::must_use(...) | provenance | MaD:9 | -| sqlx.rs:53:9:53:22 | unsafe_query_1 [&ref] | sqlx.rs:67:26:67:48 | unsafe_query_1.as_str() | provenance | MaD:3 | | sqlx.rs:53:9:53:22 | unsafe_query_1 [&ref] | sqlx.rs:78:25:78:47 | unsafe_query_1.as_str() | provenance | MaD:3 | | 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:3 | | sqlx.rs:54:9:54:22 | unsafe_query_2 [&ref] | sqlx.rs:80:29:80:51 | unsafe_query_2.as_str() | provenance | MaD:3 | | sqlx.rs:54:26:54:39 | &remote_string [&ref] | sqlx.rs:54:9:54:22 | unsafe_query_2 [&ref] | provenance | | | sqlx.rs:54:27:54:39 | remote_string | sqlx.rs:54:26:54:39 | &remote_string [&ref] | provenance | | -| sqlx.rs:56:9:56:22 | unsafe_query_4 | sqlx.rs:71:30:71:52 | unsafe_query_4.as_str() | provenance | MaD:3 | | sqlx.rs:56:9:56:22 | unsafe_query_4 | sqlx.rs:82:29:82:51 | unsafe_query_4.as_str() | provenance | MaD:3 | | sqlx.rs:59:9:59:73 | res | sqlx.rs:59:17:59:72 | { ... } | provenance | | | sqlx.rs:59:17:59:72 | ...::format(...) | sqlx.rs:59:9:59:73 | res | provenance | | @@ -91,10 +83,6 @@ nodes | sqlx.rs:59:17:59:72 | ...::must_use(...) | semmle.label | ...::must_use(...) | | sqlx.rs:59:17:59:72 | MacroExpr | semmle.label | MacroExpr | | sqlx.rs:59:17:59:72 | { ... } | semmle.label | { ... } | -| sqlx.rs:66:26:66:46 | safe_query_3.as_str() | semmle.label | safe_query_3.as_str() | -| 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:71:30:71:52 | unsafe_query_4.as_str() | semmle.label | unsafe_query_4.as_str() | | sqlx.rs:77:25:77:45 | safe_query_3.as_str() | semmle.label | safe_query_3.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() | diff --git a/rust/ql/test/query-tests/security/CWE-089/sqlx.rs b/rust/ql/test/query-tests/security/CWE-089/sqlx.rs index 3de58350f20c..291b04257d5a 100644 --- a/rust/ql/test/query-tests/security/CWE-089/sqlx.rs +++ b/rust/ql/test/query-tests/security/CWE-089/sqlx.rs @@ -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 SPURIOUS: Alert[rust/sql-injection]=remote1 - 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 + 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 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 @@ -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 @@ -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