Skip to content

Commit 1a8b5b5

Browse files
geoffw0CHMANG
geoffw0
authored and
CHMANG
committed
Rust: Add barrier guards to the query.
1 parent 1b3c870 commit 1a8b5b5

3 files changed

Lines changed: 61 additions & 12 deletions

File tree

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

Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -61,4 +61,62 @@ module CommandInjection {
6161

6262
private class FieldlessEnumTypeBarrier extends Barrier instanceof Barriers::FieldlessEnumTypeBarrier
6363
{ }
64+
65+
/**
66+
* A sanitizer guard for command injection vulnerabilities.
67+
*/
68+
class SanitizerGuard extends Barrier {
69+
SanitizerGuard() { this = DataFlow::BarrierGuard<sanitizerGuard/3>::getABarrierNode() }
70+
}
71+
}
72+
73+
private predicate sanitizerGuard(AstNode g, Expr e, boolean branch) {
74+
g.(SanitizerGuard::Range).checks(e, branch)
75+
}
76+
77+
/**
78+
* Provides a class for modeling new command injection safety checks.
79+
*/
80+
module SanitizerGuard {
81+
/**
82+
* A data-flow node that checks whether a command is safe.
83+
*/
84+
abstract class Range extends AstNode {
85+
/**
86+
* Holds if this guard validates `e` upon evaluating to `branch`.
87+
*/
88+
abstract predicate checks(Expr e, boolean branch);
89+
}
90+
}
91+
92+
/**
93+
* A successful membership check against a (presumed) command allowlist. For example:
94+
* ```
95+
* if allowlist.contains(&commmand) { ... }
96+
* ```
97+
*/
98+
private class AllowlistContainsCheck extends SanitizerGuard::Range, MethodCall {
99+
AllowlistContainsCheck() { this.getStaticTarget().getName().getText() = "contains" }
100+
101+
override predicate checks(Expr e, boolean branch) {
102+
e.getParentNode*() = this.getPositionalArgument(0) and
103+
branch = true
104+
}
105+
}
106+
107+
/**
108+
* An equality check against a (presumed) allowed command value. For example:
109+
* ```
110+
* if command == "ls" { ... }
111+
* ```
112+
*/
113+
private class AllowlistEqualityCheck extends SanitizerGuard::Range, EqualityOperation {
114+
override predicate checks(Expr e, boolean branch) {
115+
e = this.getAnOperand() and
116+
(
117+
this instanceof EqualsOperation and branch = true
118+
or
119+
this instanceof NotEqualsOperation and branch = false
120+
)
121+
}
64122
}

‎rust/ql/test/query-tests/security/CWE-078/CommandInjection.expected‎

Lines changed: 0 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -10,12 +10,9 @@
1010
| main.rs:100:22:100:29 | filename | main.rs:138:22:138:37 | ...::args(...) | main.rs:100:22:100:29 | filename | This command line depends on a $@. | main.rs:138:22:138:37 | ...::args(...) | user-provided value |
1111
| main.rs:111:22:111:28 | command | main.rs:138:22:138:37 | ...::args(...) | main.rs:111:22:111:28 | command | This command line depends on a $@. | main.rs:138:22:138:37 | ...::args(...) | user-provided value |
1212
| main.rs:113:22:113:28 | command | main.rs:138:22:138:37 | ...::args(...) | main.rs:113:22:113:28 | command | This command line depends on a $@. | main.rs:138:22:138:37 | ...::args(...) | user-provided value |
13-
| main.rs:119:22:119:28 | command | main.rs:138:22:138:37 | ...::args(...) | main.rs:119:22:119:28 | command | This command line depends on a $@. | main.rs:138:22:138:37 | ...::args(...) | user-provided value |
1413
| main.rs:121:22:121:28 | command | main.rs:138:22:138:37 | ...::args(...) | main.rs:121:22:121:28 | command | This command line depends on a $@. | main.rs:138:22:138:37 | ...::args(...) | user-provided value |
15-
| main.rs:125:22:125:28 | command | main.rs:138:22:138:37 | ...::args(...) | main.rs:125:22:125:28 | command | This command line depends on a $@. | main.rs:138:22:138:37 | ...::args(...) | user-provided value |
1614
| main.rs:127:22:127:28 | command | main.rs:138:22:138:37 | ...::args(...) | main.rs:127:22:127:28 | command | This command line depends on a $@. | main.rs:138:22:138:37 | ...::args(...) | user-provided value |
1715
| main.rs:131:22:131:28 | command | main.rs:138:22:138:37 | ...::args(...) | main.rs:131:22:131:28 | command | This command line depends on a $@. | main.rs:138:22:138:37 | ...::args(...) | user-provided value |
18-
| main.rs:133:22:133:28 | command | main.rs:138:22:138:37 | ...::args(...) | main.rs:133:22:133:28 | command | This command line depends on a $@. | main.rs:138:22:138:37 | ...::args(...) | user-provided value |
1916
edges
2017
| main.rs:4:9:4:18 | arg_string | main.rs:27:18:27:27 | arg_string | provenance | |
2118
| main.rs:4:9:4:18 | arg_string | main.rs:40:14:40:23 | arg_string | provenance | |
@@ -64,12 +61,9 @@ edges
6461
| main.rs:95:27:95:40 | ...: ... [&ref] | main.rs:100:22:100:29 | filename | provenance | Sink:MaD:1 |
6562
| main.rs:107:30:107:42 | ...: ... [&ref] | main.rs:111:22:111:28 | command | provenance | Sink:MaD:3 |
6663
| main.rs:107:30:107:42 | ...: ... [&ref] | main.rs:113:22:113:28 | command | provenance | Sink:MaD:3 |
67-
| main.rs:107:30:107:42 | ...: ... [&ref] | main.rs:119:22:119:28 | command | provenance | Sink:MaD:3 |
6864
| main.rs:107:30:107:42 | ...: ... [&ref] | main.rs:121:22:121:28 | command | provenance | Sink:MaD:3 |
69-
| main.rs:107:30:107:42 | ...: ... [&ref] | main.rs:125:22:125:28 | command | provenance | Sink:MaD:3 |
7065
| main.rs:107:30:107:42 | ...: ... [&ref] | main.rs:127:22:127:28 | command | provenance | Sink:MaD:3 |
7166
| main.rs:107:30:107:42 | ...: ... [&ref] | main.rs:131:22:131:28 | command | provenance | Sink:MaD:3 |
72-
| main.rs:107:30:107:42 | ...: ... [&ref] | main.rs:133:22:133:28 | command | provenance | Sink:MaD:3 |
7367
| main.rs:138:9:138:18 | arg_string | main.rs:142:39:142:48 | arg_string | provenance | |
7468
| main.rs:138:9:138:18 | arg_string | main.rs:143:40:143:49 | arg_string | provenance | |
7569
| main.rs:138:9:138:18 | arg_string | main.rs:144:31:144:40 | arg_string | provenance | |
@@ -152,12 +146,9 @@ nodes
152146
| main.rs:107:30:107:42 | ...: ... [&ref] | semmle.label | ...: ... [&ref] |
153147
| main.rs:111:22:111:28 | command | semmle.label | command |
154148
| main.rs:113:22:113:28 | command | semmle.label | command |
155-
| main.rs:119:22:119:28 | command | semmle.label | command |
156149
| main.rs:121:22:121:28 | command | semmle.label | command |
157-
| main.rs:125:22:125:28 | command | semmle.label | command |
158150
| main.rs:127:22:127:28 | command | semmle.label | command |
159151
| main.rs:131:22:131:28 | command | semmle.label | command |
160-
| main.rs:133:22:133:28 | command | semmle.label | command |
161152
| main.rs:138:9:138:18 | arg_string | semmle.label | arg_string |
162153
| main.rs:138:22:138:37 | ...::args(...) | semmle.label | ...::args(...) |
163154
| main.rs:138:22:138:37 | ...::args(...) [element] | semmle.label | ...::args(...) [element] |

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

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -116,21 +116,21 @@ fn test_allowlist_sanitizers(command: &str) {
116116
let allowed_commands_vec = vec!["cat", "git", "ls"];
117117

118118
if allowed_commands_vec.contains(&command) {
119-
Command::new(command).output().expect("failed"); // $ SPURIOUS: Alert[rust/command-line-injection]=args2
119+
Command::new(command).output().expect("failed");
120120
} else {
121121
Command::new(command).output().expect("failed"); // $ Alert[rust/command-line-injection]=args2
122122
}
123123

124124
if command == "ls" {
125-
Command::new(command).output().expect("failed"); // $ SPURIOUS: Alert[rust/command-line-injection]=args2
125+
Command::new(command).output().expect("failed");
126126
} else {
127127
Command::new(command).output().expect("failed"); // $ Alert[rust/command-line-injection]=args2
128128
}
129129

130130
if command != "ls" {
131131
Command::new(command).output().expect("failed"); // $ Alert[rust/command-line-injection]=args2
132132
} else {
133-
Command::new(command).output().expect("failed"); // $ SPURIOUS: Alert[rust/command-line-injection]=args2
133+
Command::new(command).output().expect("failed");
134134
}
135135
}
136136

0 commit comments

Comments
 (0)