Skip to content

Commit 52397f0

Browse files
geoffw0geoffw0
committed
Rust: Add numeric type barrier for SQL injection.
1 parent 6433bec commit 52397f0

File tree

4 files changed

+44
-39
lines changed

4 files changed

+44
-39
lines changed

‎rust/ql/lib/codeql/rust/security/Barriers.qll‎

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
/**
2+
* Classes to represent barriers commonly used in dataflow and taint tracking
3+
* configurations.
4+
*/
5+
6+
import rust
7+
private import codeql.rust.dataflow.DataFlow
8+
private import codeql.rust.internal.TypeInference as TypeInference
9+
private import codeql.rust.internal.Type
10+
private import codeql.rust.frameworks.stdlib.Builtins
11+
12+
/**
13+
* A node whose type is a numeric or boolean type, which may be an appropriate
14+
* taint flow barrier for some queries.
15+
*/
16+
class NumericTypeBarrier extends DataFlow::Node {
17+
NumericTypeBarrier() {
18+
exists(TypeInference::Type t |
19+
t = TypeInference::inferType(this.asExpr().getExpr()) and
20+
(
21+
t.(StructType).getStruct() instanceof NumericType or
22+
t.(StructType).getStruct() instanceof Bool
23+
)
24+
)
25+
}
26+
}

‎rust/ql/lib/codeql/rust/security/SqlInjectionExtensions.qll‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ private import codeql.rust.dataflow.DataFlow
99
private import codeql.rust.dataflow.FlowSink
1010
private import codeql.rust.Concepts
1111
private import codeql.util.Unit
12+
private import codeql.rust.security.Barriers as Barriers
1213

1314
/**
1415
* Provides default sources, sinks and barriers for detecting SQL injection
@@ -57,4 +58,10 @@ module SqlInjection {
5758
private class ModelsAsDataSink extends Sink {
5859
ModelsAsDataSink() { sinkNode(this, "sql-injection") }
5960
}
61+
62+
/**
63+
* A barrier for SQL injection vulnerabilities for nodes whose type is a numeric or
64+
* boolean type, which is unlikely to expose any vulnerability.
65+
*/
66+
private class NumericTypeBarrier extends Barrier instanceof Barriers::NumericTypeBarrier { }
6067
}

‎rust/ql/test/query-tests/security/CWE-089/SqlInjection.expected‎

Lines changed: 10 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,6 @@
2121
| 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 |
2222
| 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 |
2323
| 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 |
24-
| 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 |
2524
| 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 |
2625
| 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 |
2726
| 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
3736
| mysql.rs:12:13:12:29 | mut remote_string | mysql.rs:18:71:18:83 | remote_string | provenance | |
3837
| mysql.rs:12:33:12:54 | ...::get | mysql.rs:12:33:12:77 | ...::get(...) [Ok] | provenance | Src:MaD:23 |
3938
| mysql.rs:12:33:12:77 | ...::get(...) [Ok] | mysql.rs:12:33:13:21 | ... .unwrap() | provenance | MaD:31 |
40-
| mysql.rs:12:33:13:21 | ... .unwrap() | mysql.rs:12:33:14:19 | ... .text() [Ok] | provenance | MaD:35 |
39+
| mysql.rs:12:33:13:21 | ... .unwrap() | mysql.rs:12:33:14:19 | ... .text() [Ok] | provenance | MaD:34 |
4140
| mysql.rs:12:33:14:19 | ... .text() [Ok] | mysql.rs:12:33:15:40 | ... .unwrap_or(...) | provenance | MaD:32 |
4241
| mysql.rs:12:33:15:40 | ... .unwrap_or(...) | mysql.rs:12:13:12:29 | mut remote_string | provenance | |
4342
| mysql.rs:17:13:17:24 | unsafe_query | mysql.rs:25:38:25:49 | unsafe_query | provenance | |
@@ -114,7 +113,7 @@ edges
114113
| mysql.rs:97:13:97:29 | mut remote_string | mysql.rs:103:71:103:83 | remote_string | provenance | |
115114
| mysql.rs:97:33:97:54 | ...::get | mysql.rs:97:33:97:77 | ...::get(...) [Ok] | provenance | Src:MaD:23 |
116115
| mysql.rs:97:33:97:77 | ...::get(...) [Ok] | mysql.rs:97:33:98:21 | ... .unwrap() | provenance | MaD:31 |
117-
| mysql.rs:97:33:98:21 | ... .unwrap() | mysql.rs:97:33:99:19 | ... .text() [Ok] | provenance | MaD:35 |
116+
| mysql.rs:97:33:98:21 | ... .unwrap() | mysql.rs:97:33:99:19 | ... .text() [Ok] | provenance | MaD:34 |
118117
| mysql.rs:97:33:99:19 | ... .text() [Ok] | mysql.rs:97:33:100:40 | ... .unwrap_or(...) | provenance | MaD:32 |
119118
| mysql.rs:97:33:100:40 | ... .unwrap_or(...) | mysql.rs:97:13:97:29 | mut remote_string | provenance | |
120119
| mysql.rs:102:13:102:24 | unsafe_query | mysql.rs:110:38:110:49 | unsafe_query | provenance | |
@@ -173,25 +172,14 @@ edges
173172
| sqlx.rs:47:22:47:37 | ...::args(...) [element] | sqlx.rs:47:22:47:44 | ... .nth(...) [Some] | provenance | MaD:25 |
174173
| sqlx.rs:47:22:47:44 | ... .nth(...) [Some] | sqlx.rs:47:22:47:77 | ... .unwrap_or(...) | provenance | MaD:30 |
175174
| sqlx.rs:47:22:47:77 | ... .unwrap_or(...) | sqlx.rs:47:9:47:18 | arg_string | provenance | |
176-
| sqlx.rs:48:9:48:21 | remote_string | sqlx.rs:49:25:49:52 | remote_string.parse() [Ok] | provenance | MaD:34 |
177-
| sqlx.rs:48:9:48:21 | remote_string | sqlx.rs:49:25:49:52 | remote_string.parse() [Ok] | provenance | MaD:34 |
178175
| sqlx.rs:48:9:48:21 | remote_string | sqlx.rs:54:27:54:39 | remote_string | provenance | |
179176
| sqlx.rs:48:9:48:21 | remote_string | sqlx.rs:55:84:55:96 | remote_string | provenance | |
180177
| sqlx.rs:48:9:48:21 | remote_string | sqlx.rs:59:17:59:72 | MacroExpr | provenance | |
181178
| sqlx.rs:48:25:48:46 | ...::get | sqlx.rs:48:25:48:69 | ...::get(...) [Ok] | provenance | Src:MaD:23 |
182179
| sqlx.rs:48:25:48:69 | ...::get(...) [Ok] | sqlx.rs:48:25:48:78 | ... .unwrap() | provenance | MaD:31 |
183-
| sqlx.rs:48:25:48:78 | ... .unwrap() | sqlx.rs:48:25:48:85 | ... .text() [Ok] | provenance | MaD:35 |
180+
| sqlx.rs:48:25:48:78 | ... .unwrap() | sqlx.rs:48:25:48:85 | ... .text() [Ok] | provenance | MaD:34 |
184181
| sqlx.rs:48:25:48:85 | ... .text() [Ok] | sqlx.rs:48:25:48:118 | ... .unwrap_or(...) | provenance | MaD:32 |
185182
| sqlx.rs:48:25:48:118 | ... .unwrap_or(...) | sqlx.rs:48:9:48:21 | remote_string | provenance | |
186-
| sqlx.rs:49:9:49:21 | remote_number | sqlx.rs:52:32:52:87 | MacroExpr | provenance | |
187-
| sqlx.rs:49:25:49:52 | remote_string.parse() [Ok] | sqlx.rs:49:25:49:65 | ... .unwrap_or(...) | provenance | MaD:32 |
188-
| sqlx.rs:49:25:49:65 | ... .unwrap_or(...) | sqlx.rs:49:9:49:21 | remote_number | provenance | |
189-
| sqlx.rs:52:9:52:20 | safe_query_3 | sqlx.rs:77:25:77:36 | safe_query_3 | provenance | |
190-
| sqlx.rs:52:9:52:20 | safe_query_3 | sqlx.rs:77:25:77:45 | safe_query_3.as_str() | provenance | MaD:29 |
191-
| sqlx.rs:52:32:52:87 | ...::format(...) | sqlx.rs:52:32:52:87 | { ... } | provenance | |
192-
| sqlx.rs:52:32:52:87 | ...::must_use(...) | sqlx.rs:52:9:52:20 | safe_query_3 | provenance | |
193-
| sqlx.rs:52:32:52:87 | MacroExpr | sqlx.rs:52:32:52:87 | ...::format(...) | provenance | MaD:36 |
194-
| sqlx.rs:52:32:52:87 | { ... } | sqlx.rs:52:32:52:87 | ...::must_use(...) | provenance | MaD:37 |
195183
| 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 |
196184
| sqlx.rs:53:26:53:36 | &arg_string [&ref] | sqlx.rs:53:9:53:22 | unsafe_query_1 [&ref] | provenance | |
197185
| sqlx.rs:53:27:53:36 | arg_string | sqlx.rs:53:26:53:36 | &arg_string [&ref] | provenance | |
@@ -212,11 +200,8 @@ edges
212200
| sqlx.rs:56:9:56:22 | unsafe_query_4 | sqlx.rs:82:29:82:51 | unsafe_query_4.as_str() | provenance | MaD:29 |
213201
| sqlx.rs:59:17:59:72 | ...::format(...) | sqlx.rs:59:17:59:72 | { ... } | provenance | |
214202
| sqlx.rs:59:17:59:72 | ...::must_use(...) | sqlx.rs:56:9:56:22 | unsafe_query_4 | provenance | |
215-
| sqlx.rs:59:17:59:72 | MacroExpr | sqlx.rs:59:17:59:72 | ...::format(...) | provenance | MaD:36 |
216-
| sqlx.rs:59:17:59:72 | { ... } | sqlx.rs:59:17:59:72 | ...::must_use(...) | provenance | MaD:37 |
217-
| sqlx.rs:77:25:77:36 | safe_query_3 | sqlx.rs:77:25:77:45 | safe_query_3.as_str() [&ref] | provenance | MaD:29 |
218-
| sqlx.rs:77:25:77:45 | safe_query_3.as_str() | sqlx.rs:77:13:77:23 | ...::query | provenance | MaD:20 Sink:MaD:20 |
219-
| 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 |
203+
| sqlx.rs:59:17:59:72 | MacroExpr | sqlx.rs:59:17:59:72 | ...::format(...) | provenance | MaD:35 |
204+
| sqlx.rs:59:17:59:72 | { ... } | sqlx.rs:59:17:59:72 | ...::must_use(...) | provenance | MaD:36 |
220205
| 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 |
221206
| 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 |
222207
| 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
228213
| sqlx.rs:100:9:100:21 | remote_string | sqlx.rs:102:84:102:96 | remote_string | provenance | |
229214
| sqlx.rs:100:25:100:46 | ...::get | sqlx.rs:100:25:100:69 | ...::get(...) [Ok] | provenance | Src:MaD:23 |
230215
| sqlx.rs:100:25:100:69 | ...::get(...) [Ok] | sqlx.rs:100:25:100:78 | ... .unwrap() | provenance | MaD:31 |
231-
| sqlx.rs:100:25:100:78 | ... .unwrap() | sqlx.rs:100:25:100:85 | ... .text() [Ok] | provenance | MaD:35 |
216+
| sqlx.rs:100:25:100:78 | ... .unwrap() | sqlx.rs:100:25:100:85 | ... .text() [Ok] | provenance | MaD:34 |
232217
| sqlx.rs:100:25:100:85 | ... .text() [Ok] | sqlx.rs:100:25:100:118 | ... .unwrap_or(...) | provenance | MaD:32 |
233218
| sqlx.rs:100:25:100:118 | ... .unwrap_or(...) | sqlx.rs:100:9:100:21 | remote_string | provenance | |
234219
| sqlx.rs:102:9:102:22 | unsafe_query_1 | sqlx.rs:113:31:113:44 | unsafe_query_1 | provenance | |
@@ -270,7 +255,7 @@ edges
270255
| sqlx.rs:173:9:173:21 | remote_string | sqlx.rs:175:84:175:96 | remote_string | provenance | |
271256
| sqlx.rs:173:25:173:46 | ...::get | sqlx.rs:173:25:173:69 | ...::get(...) [Ok] | provenance | Src:MaD:23 |
272257
| sqlx.rs:173:25:173:69 | ...::get(...) [Ok] | sqlx.rs:173:25:173:78 | ... .unwrap() | provenance | MaD:31 |
273-
| sqlx.rs:173:25:173:78 | ... .unwrap() | sqlx.rs:173:25:173:85 | ... .text() [Ok] | provenance | MaD:35 |
258+
| sqlx.rs:173:25:173:78 | ... .unwrap() | sqlx.rs:173:25:173:85 | ... .text() [Ok] | provenance | MaD:34 |
274259
| sqlx.rs:173:25:173:85 | ... .text() [Ok] | sqlx.rs:173:25:173:118 | ... .unwrap_or(...) | provenance | MaD:32 |
275260
| sqlx.rs:173:25:173:118 | ... .unwrap_or(...) | sqlx.rs:173:9:173:21 | remote_string | provenance | |
276261
| sqlx.rs:175:9:175:22 | unsafe_query_1 | sqlx.rs:188:29:188:42 | unsafe_query_1 | provenance | |
@@ -318,10 +303,9 @@ models
318303
| 31 | Summary: <core::result::Result>::unwrap; Argument[self].Field[core::result::Result::Ok(0)]; ReturnValue; value |
319304
| 32 | Summary: <core::result::Result>::unwrap_or; Argument[self].Field[core::result::Result::Ok(0)]; ReturnValue; value |
320305
| 33 | Summary: <core::str>::as_str; Argument[self]; ReturnValue; value |
321-
| 34 | Summary: <core::str>::parse; Argument[self]; ReturnValue.Field[core::result::Result::Ok(0)]; taint |
322-
| 35 | Summary: <reqwest::blocking::response::Response>::text; Argument[self]; ReturnValue.Field[core::result::Result::Ok(0)]; taint |
323-
| 36 | Summary: alloc::fmt::format; Argument[0]; ReturnValue; taint |
324-
| 37 | Summary: core::hint::must_use; Argument[0]; ReturnValue; value |
306+
| 34 | Summary: <reqwest::blocking::response::Response>::text; Argument[self]; ReturnValue.Field[core::result::Result::Ok(0)]; taint |
307+
| 35 | Summary: alloc::fmt::format; Argument[0]; ReturnValue; taint |
308+
| 36 | Summary: core::hint::must_use; Argument[0]; ReturnValue; value |
325309
nodes
326310
| mysql.rs:12:13:12:29 | mut remote_string | semmle.label | mut remote_string |
327311
| mysql.rs:12:33:12:54 | ...::get | semmle.label | ...::get |
@@ -444,14 +428,6 @@ nodes
444428
| sqlx.rs:48:25:48:78 | ... .unwrap() | semmle.label | ... .unwrap() |
445429
| sqlx.rs:48:25:48:85 | ... .text() [Ok] | semmle.label | ... .text() [Ok] |
446430
| sqlx.rs:48:25:48:118 | ... .unwrap_or(...) | semmle.label | ... .unwrap_or(...) |
447-
| sqlx.rs:49:9:49:21 | remote_number | semmle.label | remote_number |
448-
| sqlx.rs:49:25:49:52 | remote_string.parse() [Ok] | semmle.label | remote_string.parse() [Ok] |
449-
| sqlx.rs:49:25:49:65 | ... .unwrap_or(...) | semmle.label | ... .unwrap_or(...) |
450-
| sqlx.rs:52:9:52:20 | safe_query_3 | semmle.label | safe_query_3 |
451-
| sqlx.rs:52:32:52:87 | ...::format(...) | semmle.label | ...::format(...) |
452-
| sqlx.rs:52:32:52:87 | ...::must_use(...) | semmle.label | ...::must_use(...) |
453-
| sqlx.rs:52:32:52:87 | MacroExpr | semmle.label | MacroExpr |
454-
| sqlx.rs:52:32:52:87 | { ... } | semmle.label | { ... } |
455431
| sqlx.rs:53:9:53:22 | unsafe_query_1 [&ref] | semmle.label | unsafe_query_1 [&ref] |
456432
| sqlx.rs:53:26:53:36 | &arg_string [&ref] | semmle.label | &arg_string [&ref] |
457433
| sqlx.rs:53:27:53:36 | arg_string | semmle.label | arg_string |
@@ -468,10 +444,6 @@ nodes
468444
| sqlx.rs:59:17:59:72 | ...::must_use(...) | semmle.label | ...::must_use(...) |
469445
| sqlx.rs:59:17:59:72 | MacroExpr | semmle.label | MacroExpr |
470446
| sqlx.rs:59:17:59:72 | { ... } | semmle.label | { ... } |
471-
| sqlx.rs:77:13:77:23 | ...::query | semmle.label | ...::query |
472-
| sqlx.rs:77:25:77:36 | safe_query_3 | semmle.label | safe_query_3 |
473-
| sqlx.rs:77:25:77:45 | safe_query_3.as_str() | semmle.label | safe_query_3.as_str() |
474-
| sqlx.rs:77:25:77:45 | safe_query_3.as_str() [&ref] | semmle.label | safe_query_3.as_str() [&ref] |
475447
| sqlx.rs:78:13:78:23 | ...::query | semmle.label | ...::query |
476448
| sqlx.rs:78:25:78:47 | unsafe_query_1.as_str() [&ref] | semmle.label | unsafe_query_1.as_str() [&ref] |
477449
| sqlx.rs:80:17:80:27 | ...::query | semmle.label | ...::query |

‎rust/ql/test/query-tests/security/CWE-089/sqlx.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -74,7 +74,7 @@ async fn test_sqlx_mysql(url: &str, enable_remote: bool) -> Result<(), sqlx::Err
7474
// prepared queries
7575
let _ = sqlx::query(safe_query_1.as_str()).execute(&pool).await?; // $ sql-sink
7676
let _ = sqlx::query(safe_query_2.as_str()).execute(&pool).await?; // $ sql-sink
77-
let _ = sqlx::query(safe_query_3.as_str()).execute(&pool).await?; // $ sql-sink $ SPURIOUS: Alert[rust/sql-injection]=remote1
77+
let _ = sqlx::query(safe_query_3.as_str()).execute(&pool).await?; // $ sql-sink
7878
let _ = sqlx::query(unsafe_query_1.as_str()).execute(&pool).await?; // $ sql-sink Alert[rust/sql-injection]=args1
7979
if enable_remote {
8080
let _ = sqlx::query(unsafe_query_2.as_str()).execute(&pool).await?; // $ sql-sink Alert[rust/sql-injection]=remote1

0 commit comments

Comments
 (0)