Skip to content

Commit 33efed9

Browse files
geoffw0geoffw0
committed
Rust: Add integral type barrier for Regex injection.
1 parent 2d4369a commit 33efed9

File tree

4 files changed

+32
-34
lines changed

4 files changed

+32
-34
lines changed

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

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,3 +24,19 @@ class NumericTypeBarrier extends DataFlow::Node {
2424
)
2525
}
2626
}
27+
28+
/**
29+
* A node whose type is an integral (integer) or boolean type, which may be an
30+
* appropriate taint flow barrier for some queries.
31+
*/
32+
class IntegralOrBooleanTypeBarrier extends DataFlow::Node {
33+
IntegralOrBooleanTypeBarrier() {
34+
exists(TypeInference::Type t |
35+
t = TypeInference::inferType(this.asExpr().getExpr()) and
36+
(
37+
t.(StructType).getStruct() instanceof IntegralType or
38+
t.(StructType).getStruct() instanceof Bool
39+
)
40+
)
41+
}
42+
}

‎rust/ql/lib/codeql/rust/security/regex/RegexInjectionExtensions.qll‎

Lines changed: 10 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.controlflow.CfgNodes
1010
private import codeql.rust.dataflow.FlowSink
1111
private import codeql.rust.Concepts
12+
private import codeql.rust.security.Barriers as Barriers
1213

1314
/**
1415
* Provides default sources, sinks and barriers for detecting regular expression
@@ -87,4 +88,13 @@ module RegexInjection {
8788
.getText() = "escape"
8889
}
8990
}
91+
92+
/**
93+
* A barrier for regular expression injection vulnerabilities for nodes whose
94+
* type is an integral or boolean type, which is unlikely to expose any vulnerability.
95+
*
96+
* We don't include floating point types in this barrier, as `.` is a special character
97+
* in regular expressions.
98+
*/
99+
private class IntegralOrBooleanTypeBarrier extends Barrier instanceof Barriers::IntegralOrBooleanTypeBarrier { }
90100
}

‎rust/ql/test/query-tests/security/CWE-020/RegexInjection.expected‎

Lines changed: 4 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,5 @@
11
#select
22
| main.rs:6:25:6:30 | &regex | main.rs:4:20:4:32 | ...::var | main.rs:6:25:6:30 | &regex | This regular expression is constructed from a $@. | main.rs:4:20:4:32 | ...::var | user-provided value |
3-
| main.rs:21:25:21:30 | &regex | main.rs:19:23:19:35 | ...::var | main.rs:21:25:21:30 | &regex | This regular expression is constructed from a $@. | main.rs:19:23:19:35 | ...::var | user-provided value |
43
edges
54
| main.rs:4:9:4:16 | username | main.rs:5:25:5:44 | MacroExpr | provenance | |
65
| main.rs:4:20:4:32 | ...::var | main.rs:4:20:4:40 | ...::var(...) [Ok] | provenance | Src:MaD:1 |
@@ -9,28 +8,14 @@ edges
98
| main.rs:5:9:5:13 | regex | main.rs:6:26:6:30 | regex | provenance | |
109
| main.rs:5:25:5:44 | ...::format(...) | main.rs:5:25:5:44 | { ... } | provenance | |
1110
| main.rs:5:25:5:44 | ...::must_use(...) | main.rs:5:9:5:13 | regex | provenance | |
12-
| main.rs:5:25:5:44 | MacroExpr | main.rs:5:25:5:44 | ...::format(...) | provenance | MaD:4 |
13-
| main.rs:5:25:5:44 | { ... } | main.rs:5:25:5:44 | ...::must_use(...) | provenance | MaD:5 |
11+
| main.rs:5:25:5:44 | MacroExpr | main.rs:5:25:5:44 | ...::format(...) | provenance | MaD:3 |
12+
| main.rs:5:25:5:44 | { ... } | main.rs:5:25:5:44 | ...::must_use(...) | provenance | MaD:4 |
1413
| main.rs:6:26:6:30 | regex | main.rs:6:25:6:30 | &regex | provenance | |
15-
| main.rs:19:9:19:19 | user_number | main.rs:20:25:20:47 | MacroExpr | provenance | |
16-
| main.rs:19:23:19:35 | ...::var | main.rs:19:23:19:43 | ...::var(...) [Ok] | provenance | Src:MaD:1 |
17-
| main.rs:19:23:19:43 | ...::var(...) [Ok] | main.rs:19:23:19:70 | ... .unwrap_or(...) | provenance | MaD:2 |
18-
| main.rs:19:23:19:70 | ... .unwrap_or(...) | main.rs:19:23:19:85 | ... .parse() [Ok] | provenance | MaD:3 |
19-
| main.rs:19:23:19:70 | ... .unwrap_or(...) | main.rs:19:23:19:85 | ... .parse() [Ok] | provenance | MaD:3 |
20-
| main.rs:19:23:19:85 | ... .parse() [Ok] | main.rs:19:23:19:98 | ... .unwrap_or(...) | provenance | MaD:2 |
21-
| main.rs:19:23:19:98 | ... .unwrap_or(...) | main.rs:19:9:19:19 | user_number | provenance | |
22-
| main.rs:20:9:20:13 | regex | main.rs:21:26:21:30 | regex | provenance | |
23-
| main.rs:20:25:20:47 | ...::format(...) | main.rs:20:25:20:47 | { ... } | provenance | |
24-
| main.rs:20:25:20:47 | ...::must_use(...) | main.rs:20:9:20:13 | regex | provenance | |
25-
| main.rs:20:25:20:47 | MacroExpr | main.rs:20:25:20:47 | ...::format(...) | provenance | MaD:4 |
26-
| main.rs:20:25:20:47 | { ... } | main.rs:20:25:20:47 | ...::must_use(...) | provenance | MaD:5 |
27-
| main.rs:21:26:21:30 | regex | main.rs:21:25:21:30 | &regex | provenance | |
2814
models
2915
| 1 | Source: std::env::var; ReturnValue.Field[core::result::Result::Ok(0)]; environment |
3016
| 2 | Summary: <core::result::Result>::unwrap_or; Argument[self].Field[core::result::Result::Ok(0)]; ReturnValue; value |
31-
| 3 | Summary: <core::str>::parse; Argument[self]; ReturnValue.Field[core::result::Result::Ok(0)]; taint |
32-
| 4 | Summary: alloc::fmt::format; Argument[0]; ReturnValue; taint |
33-
| 5 | Summary: core::hint::must_use; Argument[0]; ReturnValue; value |
17+
| 3 | Summary: alloc::fmt::format; Argument[0]; ReturnValue; taint |
18+
| 4 | Summary: core::hint::must_use; Argument[0]; ReturnValue; value |
3419
nodes
3520
| main.rs:4:9:4:16 | username | semmle.label | username |
3621
| main.rs:4:20:4:32 | ...::var | semmle.label | ...::var |
@@ -43,17 +28,4 @@ nodes
4328
| main.rs:5:25:5:44 | { ... } | semmle.label | { ... } |
4429
| main.rs:6:25:6:30 | &regex | semmle.label | &regex |
4530
| main.rs:6:26:6:30 | regex | semmle.label | regex |
46-
| main.rs:19:9:19:19 | user_number | semmle.label | user_number |
47-
| main.rs:19:23:19:35 | ...::var | semmle.label | ...::var |
48-
| main.rs:19:23:19:43 | ...::var(...) [Ok] | semmle.label | ...::var(...) [Ok] |
49-
| main.rs:19:23:19:70 | ... .unwrap_or(...) | semmle.label | ... .unwrap_or(...) |
50-
| main.rs:19:23:19:85 | ... .parse() [Ok] | semmle.label | ... .parse() [Ok] |
51-
| main.rs:19:23:19:98 | ... .unwrap_or(...) | semmle.label | ... .unwrap_or(...) |
52-
| main.rs:20:9:20:13 | regex | semmle.label | regex |
53-
| main.rs:20:25:20:47 | ...::format(...) | semmle.label | ...::format(...) |
54-
| main.rs:20:25:20:47 | ...::must_use(...) | semmle.label | ...::must_use(...) |
55-
| main.rs:20:25:20:47 | MacroExpr | semmle.label | MacroExpr |
56-
| main.rs:20:25:20:47 | { ... } | semmle.label | { ... } |
57-
| main.rs:21:25:21:30 | &regex | semmle.label | &regex |
58-
| main.rs:21:26:21:30 | regex | semmle.label | regex |
5931
subpaths

‎rust/ql/test/query-tests/security/CWE-020/main.rs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,9 +16,9 @@ fn simple_good_escaped(hay: &str) -> Option<bool> {
1616
}
1717

1818
fn simple_good_numeric(hay: &str) -> Option<bool> {
19-
let user_number = std::env::var("USER").unwrap_or("0".to_string()).parse::<u64>().unwrap_or(0); // $ Source=env
19+
let user_number = std::env::var("USER").unwrap_or("0".to_string()).parse::<u64>().unwrap_or(0);
2020
let regex = format!("foo{}bar", user_number);
21-
let re = Regex::new(&regex).unwrap(); // $ SPURIOUS: Alert[rust/regex-injection]=env
21+
let re = Regex::new(&regex).unwrap();
2222
Some(re.is_match(hay))
2323
}
2424

0 commit comments

Comments
 (0)