From 52397f0ce0a7f86aeceea0e891a3a4fc8841f02c Mon Sep 17 00:00:00 2001 From: Geoffrey White <40627776+geoffw0@users.noreply.github.com> Date: Fri, 31 Oct 2025 11:15:52 +0000 Subject: [PATCH] Rust: Add numeric type barrier for SQL injection. --- rust/ql/lib/codeql/rust/security/Barriers.qll | 26 ++++++++++ .../rust/security/SqlInjectionExtensions.qll | 7 +++ .../security/CWE-089/SqlInjection.expected | 48 ++++--------------- .../test/query-tests/security/CWE-089/sqlx.rs | 2 +- 4 files changed, 44 insertions(+), 39 deletions(-) create mode 100644 rust/ql/lib/codeql/rust/security/Barriers.qll diff --git a/rust/ql/lib/codeql/rust/security/Barriers.qll b/rust/ql/lib/codeql/rust/security/Barriers.qll new file mode 100644 index 00000000000..66d47146d30 --- /dev/null +++ b/rust/ql/lib/codeql/rust/security/Barriers.qll @@ -0,0 +1,26 @@ +/** + * Classes to represent barriers commonly used in dataflow and taint tracking + * configurations. + */ + +import rust +private import codeql.rust.dataflow.DataFlow +private import codeql.rust.internal.TypeInference as TypeInference +private import codeql.rust.internal.Type +private import codeql.rust.frameworks.stdlib.Builtins + +/** + * A node whose type is a numeric or boolean type, which may be an appropriate + * taint flow barrier for some queries. + */ +class NumericTypeBarrier extends DataFlow::Node { + NumericTypeBarrier() { + exists(TypeInference::Type t | + t = TypeInference::inferType(this.asExpr().getExpr()) and + ( + t.(StructType).getStruct() instanceof NumericType or + t.(StructType).getStruct() instanceof Bool + ) + ) + } +} diff --git a/rust/ql/lib/codeql/rust/security/SqlInjectionExtensions.qll b/rust/ql/lib/codeql/rust/security/SqlInjectionExtensions.qll index f2921ef0cc1..ff81df37e40 100644 --- a/rust/ql/lib/codeql/rust/security/SqlInjectionExtensions.qll +++ b/rust/ql/lib/codeql/rust/security/SqlInjectionExtensions.qll @@ -9,6 +9,7 @@ private import codeql.rust.dataflow.DataFlow private import codeql.rust.dataflow.FlowSink private import codeql.rust.Concepts private import codeql.util.Unit +private import codeql.rust.security.Barriers as Barriers /** * Provides default sources, sinks and barriers for detecting SQL injection @@ -57,4 +58,10 @@ module SqlInjection { private class ModelsAsDataSink extends Sink { ModelsAsDataSink() { sinkNode(this, "sql-injection") } } + + /** + * A barrier for SQL injection vulnerabilities for nodes whose type is a numeric or + * boolean type, which is unlikely to expose any vulnerability. + */ + private class NumericTypeBarrier extends Barrier instanceof Barriers::NumericTypeBarrier { } } 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 ecd8cfa7937..fafde42ff65 100644 --- a/rust/ql/test/query-tests/security/CWE-089/SqlInjection.expected +++ b/rust/ql/test/query-tests/security/CWE-089/SqlInjection.expected @@ -21,7 +21,6 @@ | mysql.rs:121:14:121:22 | query_map | mysql.rs:97:33:97:54 | ...::get | mysql.rs:121:14:121:22 | query_map | This query depends on a $@. | mysql.rs:97:33:97:54 | ...::get | user-provided value | | mysql.rs:149:26:149:29 | prep | mysql.rs:97:33:97:54 | ...::get | mysql.rs:149:26:149:29 | prep | This query depends on a $@. | mysql.rs:97:33:97:54 | ...::get | user-provided value | | mysql.rs:154:15:154:24 | query_drop | mysql.rs:97:33:97:54 | ...::get | mysql.rs:154:15:154:24 | query_drop | This query depends on a $@. | mysql.rs:97:33:97:54 | ...::get | user-provided value | -| sqlx.rs:77:13:77:23 | ...::query | sqlx.rs:48:25:48:46 | ...::get | sqlx.rs:77:13:77:23 | ...::query | This query depends on a $@. | sqlx.rs:48:25:48:46 | ...::get | user-provided value | | sqlx.rs:78:13:78:23 | ...::query | sqlx.rs:47:22:47:35 | ...::args | sqlx.rs:78:13:78:23 | ...::query | This query depends on a $@. | sqlx.rs:47:22:47:35 | ...::args | user-provided value | | sqlx.rs:80:17:80:27 | ...::query | sqlx.rs:48:25:48:46 | ...::get | sqlx.rs:80:17:80:27 | ...::query | This query depends on a $@. | sqlx.rs:48:25:48:46 | ...::get | user-provided value | | sqlx.rs:81:17:81:27 | ...::query | sqlx.rs:48:25:48:46 | ...::get | sqlx.rs:81:17:81:27 | ...::query | This query depends on a $@. | sqlx.rs:48:25:48:46 | ...::get | user-provided value | @@ -37,7 +36,7 @@ edges | mysql.rs:12:13:12:29 | mut remote_string | mysql.rs:18:71:18:83 | remote_string | provenance | | | mysql.rs:12:33:12:54 | ...::get | mysql.rs:12:33:12:77 | ...::get(...) [Ok] | provenance | Src:MaD:23 | | mysql.rs:12:33:12:77 | ...::get(...) [Ok] | mysql.rs:12:33:13:21 | ... .unwrap() | provenance | MaD:31 | -| mysql.rs:12:33:13:21 | ... .unwrap() | mysql.rs:12:33:14:19 | ... .text() [Ok] | provenance | MaD:35 | +| mysql.rs:12:33:13:21 | ... .unwrap() | mysql.rs:12:33:14:19 | ... .text() [Ok] | provenance | MaD:34 | | mysql.rs:12:33:14:19 | ... .text() [Ok] | mysql.rs:12:33:15:40 | ... .unwrap_or(...) | provenance | MaD:32 | | mysql.rs:12:33:15:40 | ... .unwrap_or(...) | mysql.rs:12:13:12:29 | mut remote_string | provenance | | | mysql.rs:17:13:17:24 | unsafe_query | mysql.rs:25:38:25:49 | unsafe_query | provenance | | @@ -114,7 +113,7 @@ edges | mysql.rs:97:13:97:29 | mut remote_string | mysql.rs:103:71:103:83 | remote_string | provenance | | | mysql.rs:97:33:97:54 | ...::get | mysql.rs:97:33:97:77 | ...::get(...) [Ok] | provenance | Src:MaD:23 | | mysql.rs:97:33:97:77 | ...::get(...) [Ok] | mysql.rs:97:33:98:21 | ... .unwrap() | provenance | MaD:31 | -| mysql.rs:97:33:98:21 | ... .unwrap() | mysql.rs:97:33:99:19 | ... .text() [Ok] | provenance | MaD:35 | +| mysql.rs:97:33:98:21 | ... .unwrap() | mysql.rs:97:33:99:19 | ... .text() [Ok] | provenance | MaD:34 | | mysql.rs:97:33:99:19 | ... .text() [Ok] | mysql.rs:97:33:100:40 | ... .unwrap_or(...) | provenance | MaD:32 | | mysql.rs:97:33:100:40 | ... .unwrap_or(...) | mysql.rs:97:13:97:29 | mut remote_string | provenance | | | mysql.rs:102:13:102:24 | unsafe_query | mysql.rs:110:38:110:49 | unsafe_query | provenance | | @@ -173,25 +172,14 @@ edges | sqlx.rs:47:22:47:37 | ...::args(...) [element] | sqlx.rs:47:22:47:44 | ... .nth(...) [Some] | provenance | MaD:25 | | sqlx.rs:47:22:47:44 | ... .nth(...) [Some] | sqlx.rs:47:22:47:77 | ... .unwrap_or(...) | provenance | MaD:30 | | sqlx.rs:47:22:47:77 | ... .unwrap_or(...) | sqlx.rs:47:9:47:18 | arg_string | provenance | | -| sqlx.rs:48:9:48:21 | remote_string | sqlx.rs:49:25:49:52 | remote_string.parse() [Ok] | provenance | MaD:34 | -| sqlx.rs:48:9:48:21 | remote_string | sqlx.rs:49:25:49:52 | remote_string.parse() [Ok] | provenance | MaD:34 | | sqlx.rs:48:9:48:21 | remote_string | sqlx.rs:54:27:54:39 | remote_string | provenance | | | sqlx.rs:48:9:48:21 | remote_string | sqlx.rs:55:84:55:96 | remote_string | provenance | | | sqlx.rs:48:9:48:21 | remote_string | sqlx.rs:59:17:59:72 | MacroExpr | provenance | | | sqlx.rs:48:25:48:46 | ...::get | sqlx.rs:48:25:48:69 | ...::get(...) [Ok] | provenance | Src:MaD:23 | | sqlx.rs:48:25:48:69 | ...::get(...) [Ok] | sqlx.rs:48:25:48:78 | ... .unwrap() | provenance | MaD:31 | -| sqlx.rs:48:25:48:78 | ... .unwrap() | sqlx.rs:48:25:48:85 | ... .text() [Ok] | provenance | MaD:35 | +| sqlx.rs:48:25:48:78 | ... .unwrap() | sqlx.rs:48:25:48:85 | ... .text() [Ok] | provenance | MaD:34 | | sqlx.rs:48:25:48:85 | ... .text() [Ok] | sqlx.rs:48:25:48:118 | ... .unwrap_or(...) | provenance | MaD:32 | | sqlx.rs:48:25:48:118 | ... .unwrap_or(...) | sqlx.rs:48:9:48:21 | remote_string | provenance | | -| 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:32 | -| 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:77:25:77:36 | safe_query_3 | provenance | | -| sqlx.rs:52:9:52:20 | safe_query_3 | sqlx.rs:77:25:77:45 | safe_query_3.as_str() | provenance | MaD:29 | -| sqlx.rs:52:32:52:87 | ...::format(...) | sqlx.rs:52:32:52:87 | { ... } | 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:36 | -| sqlx.rs:52:32:52:87 | { ... } | sqlx.rs:52:32:52:87 | ...::must_use(...) | provenance | MaD:37 | | sqlx.rs:53:9:53:22 | unsafe_query_1 [&ref] | sqlx.rs:78:25:78:47 | unsafe_query_1.as_str() [&ref] | provenance | MaD:29 | | 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 | | @@ -212,11 +200,8 @@ edges | sqlx.rs:56:9:56:22 | unsafe_query_4 | sqlx.rs:82:29:82:51 | unsafe_query_4.as_str() | provenance | MaD:29 | | sqlx.rs:59:17:59:72 | ...::format(...) | sqlx.rs:59:17:59:72 | { ... } | provenance | | | sqlx.rs:59:17:59:72 | ...::must_use(...) | sqlx.rs:56:9:56:22 | unsafe_query_4 | provenance | | -| sqlx.rs:59:17:59:72 | MacroExpr | sqlx.rs:59:17:59:72 | ...::format(...) | provenance | MaD:36 | -| sqlx.rs:59:17:59:72 | { ... } | sqlx.rs:59:17:59:72 | ...::must_use(...) | provenance | MaD:37 | -| sqlx.rs:77:25:77:36 | safe_query_3 | sqlx.rs:77:25:77:45 | safe_query_3.as_str() [&ref] | provenance | MaD:29 | -| sqlx.rs:77:25:77:45 | safe_query_3.as_str() | sqlx.rs:77:13:77:23 | ...::query | provenance | MaD:20 Sink:MaD:20 | -| sqlx.rs:77:25:77:45 | safe_query_3.as_str() [&ref] | sqlx.rs:77:13:77:23 | ...::query | provenance | MaD:20 Sink:MaD:20 | +| sqlx.rs:59:17:59:72 | MacroExpr | sqlx.rs:59:17:59:72 | ...::format(...) | provenance | MaD:35 | +| sqlx.rs:59:17:59:72 | { ... } | sqlx.rs:59:17:59:72 | ...::must_use(...) | provenance | MaD:36 | | sqlx.rs:78:25:78:47 | unsafe_query_1.as_str() [&ref] | sqlx.rs:78:13:78:23 | ...::query | provenance | MaD:20 Sink:MaD:20 | | sqlx.rs:80:29:80:51 | unsafe_query_2.as_str() [&ref] | sqlx.rs:80:17:80:27 | ...::query | provenance | MaD:20 Sink:MaD:20 | | sqlx.rs:81:29:81:42 | unsafe_query_3 | sqlx.rs:81:29:81:51 | unsafe_query_3.as_str() [&ref] | provenance | MaD:29 | @@ -228,7 +213,7 @@ edges | sqlx.rs:100:9:100:21 | remote_string | sqlx.rs:102:84:102:96 | remote_string | provenance | | | sqlx.rs:100:25:100:46 | ...::get | sqlx.rs:100:25:100:69 | ...::get(...) [Ok] | provenance | Src:MaD:23 | | sqlx.rs:100:25:100:69 | ...::get(...) [Ok] | sqlx.rs:100:25:100:78 | ... .unwrap() | provenance | MaD:31 | -| sqlx.rs:100:25:100:78 | ... .unwrap() | sqlx.rs:100:25:100:85 | ... .text() [Ok] | provenance | MaD:35 | +| sqlx.rs:100:25:100:78 | ... .unwrap() | sqlx.rs:100:25:100:85 | ... .text() [Ok] | provenance | MaD:34 | | sqlx.rs:100:25:100:85 | ... .text() [Ok] | sqlx.rs:100:25:100:118 | ... .unwrap_or(...) | provenance | MaD:32 | | sqlx.rs:100:25:100:118 | ... .unwrap_or(...) | sqlx.rs:100:9:100:21 | remote_string | provenance | | | sqlx.rs:102:9:102:22 | unsafe_query_1 | sqlx.rs:113:31:113:44 | unsafe_query_1 | provenance | | @@ -270,7 +255,7 @@ edges | sqlx.rs:173:9:173:21 | remote_string | sqlx.rs:175:84:175:96 | remote_string | provenance | | | sqlx.rs:173:25:173:46 | ...::get | sqlx.rs:173:25:173:69 | ...::get(...) [Ok] | provenance | Src:MaD:23 | | sqlx.rs:173:25:173:69 | ...::get(...) [Ok] | sqlx.rs:173:25:173:78 | ... .unwrap() | provenance | MaD:31 | -| sqlx.rs:173:25:173:78 | ... .unwrap() | sqlx.rs:173:25:173:85 | ... .text() [Ok] | provenance | MaD:35 | +| sqlx.rs:173:25:173:78 | ... .unwrap() | sqlx.rs:173:25:173:85 | ... .text() [Ok] | provenance | MaD:34 | | sqlx.rs:173:25:173:85 | ... .text() [Ok] | sqlx.rs:173:25:173:118 | ... .unwrap_or(...) | provenance | MaD:32 | | sqlx.rs:173:25:173:118 | ... .unwrap_or(...) | sqlx.rs:173:9:173:21 | remote_string | provenance | | | sqlx.rs:175:9:175:22 | unsafe_query_1 | sqlx.rs:188:29:188:42 | unsafe_query_1 | provenance | | @@ -318,10 +303,9 @@ models | 31 | Summary: ::unwrap; Argument[self].Field[core::result::Result::Ok(0)]; ReturnValue; value | | 32 | Summary: ::unwrap_or; Argument[self].Field[core::result::Result::Ok(0)]; ReturnValue; value | | 33 | Summary: ::as_str; Argument[self]; ReturnValue; value | -| 34 | Summary: ::parse; Argument[self]; ReturnValue.Field[core::result::Result::Ok(0)]; taint | -| 35 | Summary: ::text; Argument[self]; ReturnValue.Field[core::result::Result::Ok(0)]; taint | -| 36 | Summary: alloc::fmt::format; Argument[0]; ReturnValue; taint | -| 37 | Summary: core::hint::must_use; Argument[0]; ReturnValue; value | +| 34 | Summary: ::text; Argument[self]; ReturnValue.Field[core::result::Result::Ok(0)]; taint | +| 35 | Summary: alloc::fmt::format; Argument[0]; ReturnValue; taint | +| 36 | Summary: core::hint::must_use; Argument[0]; ReturnValue; value | nodes | mysql.rs:12:13:12:29 | mut remote_string | semmle.label | mut remote_string | | mysql.rs:12:33:12:54 | ...::get | semmle.label | ...::get | @@ -444,14 +428,6 @@ nodes | sqlx.rs:48:25:48:78 | ... .unwrap() | semmle.label | ... .unwrap() | | sqlx.rs:48:25:48:85 | ... .text() [Ok] | semmle.label | ... .text() [Ok] | | sqlx.rs:48:25:48:118 | ... .unwrap_or(...) | semmle.label | ... .unwrap_or(...) | -| sqlx.rs:49:9:49:21 | remote_number | semmle.label | remote_number | -| sqlx.rs:49:25:49:52 | remote_string.parse() [Ok] | semmle.label | remote_string.parse() [Ok] | -| sqlx.rs:49:25:49:65 | ... .unwrap_or(...) | semmle.label | ... .unwrap_or(...) | -| sqlx.rs:52:9:52:20 | safe_query_3 | semmle.label | safe_query_3 | -| sqlx.rs:52:32:52:87 | ...::format(...) | semmle.label | ...::format(...) | -| sqlx.rs:52:32:52:87 | ...::must_use(...) | semmle.label | ...::must_use(...) | -| sqlx.rs:52:32:52:87 | MacroExpr | semmle.label | MacroExpr | -| sqlx.rs:52:32:52:87 | { ... } | semmle.label | { ... } | | sqlx.rs:53:9:53:22 | unsafe_query_1 [&ref] | semmle.label | unsafe_query_1 [&ref] | | sqlx.rs:53:26:53:36 | &arg_string [&ref] | semmle.label | &arg_string [&ref] | | sqlx.rs:53:27:53:36 | arg_string | semmle.label | arg_string | @@ -468,10 +444,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:77:13:77:23 | ...::query | semmle.label | ...::query | -| sqlx.rs:77:25:77:36 | safe_query_3 | semmle.label | safe_query_3 | -| sqlx.rs:77:25:77:45 | safe_query_3.as_str() | semmle.label | safe_query_3.as_str() | -| sqlx.rs:77:25:77:45 | safe_query_3.as_str() [&ref] | semmle.label | safe_query_3.as_str() [&ref] | | sqlx.rs:78:13:78:23 | ...::query | semmle.label | ...::query | | sqlx.rs:78:25:78:47 | unsafe_query_1.as_str() [&ref] | semmle.label | unsafe_query_1.as_str() [&ref] | | sqlx.rs:80:17:80:27 | ...::query | semmle.label | ...::query | 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 151f9fa7c82..915625f7634 100644 --- a/rust/ql/test/query-tests/security/CWE-089/sqlx.rs +++ b/rust/ql/test/query-tests/security/CWE-089/sqlx.rs @@ -74,7 +74,7 @@ async fn test_sqlx_mysql(url: &str, enable_remote: bool) -> Result<(), sqlx::Err // prepared queries let _ = sqlx::query(safe_query_1.as_str()).execute(&pool).await?; // $ sql-sink let _ = sqlx::query(safe_query_2.as_str()).execute(&pool).await?; // $ sql-sink - let _ = sqlx::query(safe_query_3.as_str()).execute(&pool).await?; // $ sql-sink $ SPURIOUS: Alert[rust/sql-injection]=remote1 + let _ = sqlx::query(safe_query_3.as_str()).execute(&pool).await?; // $ sql-sink let _ = sqlx::query(unsafe_query_1.as_str()).execute(&pool).await?; // $ sql-sink Alert[rust/sql-injection]=args1 if enable_remote { let _ = sqlx::query(unsafe_query_2.as_str()).execute(&pool).await?; // $ sql-sink Alert[rust/sql-injection]=remote1