Skip to content

Commit 87c089e

Browse files
mbgowen-mc
mbg
authored and
owen-mc
committed
Make CommandInjection.qll use new API
The new `edges` and `nodes` sections in the .expected files are because the PathGraph module was not imported in the tests before, and thus these query predicates were not in scope.
1 parent 957757c commit 87c089e

6 files changed

Lines changed: 112 additions & 11 deletions

File tree

‎go/ql/lib/semmle/go/security/CommandInjection.qll‎

Lines changed: 31 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,10 +17,12 @@ module CommandInjection {
1717
import CommandInjectionCustomizations::CommandInjection
1818

1919
/**
20+
* DEPRECATED: Use `Flow` instead.
21+
*
2022
* A taint-tracking configuration for reasoning about command-injection vulnerabilities
2123
* with sinks which are not sanitized by `--`.
2224
*/
23-
class Configuration extends TaintTracking::Configuration {
25+
deprecated class Configuration extends TaintTracking::Configuration {
2426
Configuration() { this = "CommandInjection" }
2527

2628
override predicate isSource(DataFlow::Node source) { source instanceof Source }
@@ -39,6 +41,18 @@ module CommandInjection {
3941
}
4042
}
4143

44+
private module Config implements DataFlow::ConfigSig {
45+
predicate isSource(DataFlow::Node source) { source instanceof Source }
46+
47+
predicate isSink(DataFlow::Node sink) {
48+
exists(Sink s | sink = s | not s.doubleDashIsSanitizing())
49+
}
50+
51+
predicate isBarrier(DataFlow::Node node) { node instanceof Sanitizer }
52+
}
53+
54+
module Flow = TaintTracking::Global<Config>;
55+
4256
private class ArgumentArrayWithDoubleDash extends DataFlow::Node {
4357
int doubleDashIndex;
4458

@@ -79,10 +93,12 @@ module CommandInjection {
7993
}
8094

8195
/**
96+
* DEPRECATED: Use `DoubleDashSanitizingFlow` instead.
97+
*
8298
* A taint-tracking configuration for reasoning about command-injection vulnerabilities
8399
* with sinks which are sanitized by `--`.
84100
*/
85-
class DoubleDashSanitizingConfiguration extends TaintTracking::Configuration {
101+
deprecated class DoubleDashSanitizingConfiguration extends TaintTracking::Configuration {
86102
DoubleDashSanitizingConfiguration() { this = "CommandInjectionWithDoubleDashSanitizer" }
87103

88104
override predicate isSource(DataFlow::Node source) { source instanceof Source }
@@ -101,4 +117,17 @@ module CommandInjection {
101117
guard instanceof SanitizerGuard
102118
}
103119
}
120+
121+
private module DoubleDashSanitizingConfig implements DataFlow::ConfigSig {
122+
predicate isSource(DataFlow::Node source) { source instanceof Source }
123+
124+
predicate isSink(DataFlow::Node sink) { exists(Sink s | sink = s | s.doubleDashIsSanitizing()) }
125+
126+
predicate isBarrier(DataFlow::Node node) {
127+
node instanceof Sanitizer or
128+
node = any(ArgumentArrayWithDoubleDash array).getASanitizedElement()
129+
}
130+
}
131+
132+
module DoubleDashSanitizingFlow = TaintTracking::Global<DoubleDashSanitizingConfig>;
104133
}

‎go/ql/src/Security/CWE-078/CommandInjection.ql‎

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -13,11 +13,17 @@
1313

1414
import go
1515
import semmle.go.security.CommandInjection
16-
import DataFlow::PathGraph
1716

18-
from
19-
CommandInjection::Configuration cfg, CommandInjection::DoubleDashSanitizingConfiguration cfg2,
20-
DataFlow::PathNode source, DataFlow::PathNode sink
21-
where cfg.hasFlowPath(source, sink) or cfg2.hasFlowPath(source, sink)
17+
module Flow =
18+
DataFlow::MergePathGraph<CommandInjection::Flow::PathNode,
19+
CommandInjection::DoubleDashSanitizingFlow::PathNode, CommandInjection::Flow::PathGraph,
20+
CommandInjection::DoubleDashSanitizingFlow::PathGraph>;
21+
22+
import Flow::PathGraph
23+
24+
from Flow::PathNode source, Flow::PathNode sink
25+
where
26+
CommandInjection::Flow::flowPath(source.asPathNode1(), sink.asPathNode1()) or
27+
CommandInjection::DoubleDashSanitizingFlow::flowPath(source.asPathNode2(), sink.asPathNode2())
2228
select sink.getNode(), source, sink, "This command depends on a $@.", source.getNode(),
2329
"user-provided value"

‎go/ql/test/library-tests/semmle/go/frameworks/Encoding/jsoniter.expected‎

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,33 @@
1+
edges
2+
| jsoniter.go:23:20:23:38 | call to getUntrustedBytes | jsoniter.go:27:17:27:30 | untrustedInput |
3+
| jsoniter.go:23:20:23:38 | call to getUntrustedBytes | jsoniter.go:31:21:31:34 | untrustedInput |
4+
| jsoniter.go:24:21:24:40 | call to getUntrustedString | jsoniter.go:35:27:35:41 | untrustedString |
5+
| jsoniter.go:24:21:24:40 | call to getUntrustedString | jsoniter.go:39:31:39:45 | untrustedString |
6+
| jsoniter.go:27:17:27:30 | untrustedInput | jsoniter.go:27:33:27:37 | &... |
7+
| jsoniter.go:27:33:27:37 | &... | jsoniter.go:28:15:28:24 | selection of field |
8+
| jsoniter.go:31:21:31:34 | untrustedInput | jsoniter.go:31:37:31:42 | &... |
9+
| jsoniter.go:31:37:31:42 | &... | jsoniter.go:32:15:32:25 | selection of field |
10+
| jsoniter.go:35:27:35:41 | untrustedString | jsoniter.go:35:44:35:49 | &... |
11+
| jsoniter.go:35:44:35:49 | &... | jsoniter.go:36:15:36:25 | selection of field |
12+
| jsoniter.go:39:31:39:45 | untrustedString | jsoniter.go:39:48:39:53 | &... |
13+
| jsoniter.go:39:48:39:53 | &... | jsoniter.go:40:15:40:25 | selection of field |
14+
nodes
15+
| jsoniter.go:23:20:23:38 | call to getUntrustedBytes | semmle.label | call to getUntrustedBytes |
16+
| jsoniter.go:24:21:24:40 | call to getUntrustedString | semmle.label | call to getUntrustedString |
17+
| jsoniter.go:27:17:27:30 | untrustedInput | semmle.label | untrustedInput |
18+
| jsoniter.go:27:33:27:37 | &... | semmle.label | &... |
19+
| jsoniter.go:28:15:28:24 | selection of field | semmle.label | selection of field |
20+
| jsoniter.go:31:21:31:34 | untrustedInput | semmle.label | untrustedInput |
21+
| jsoniter.go:31:37:31:42 | &... | semmle.label | &... |
22+
| jsoniter.go:32:15:32:25 | selection of field | semmle.label | selection of field |
23+
| jsoniter.go:35:27:35:41 | untrustedString | semmle.label | untrustedString |
24+
| jsoniter.go:35:44:35:49 | &... | semmle.label | &... |
25+
| jsoniter.go:36:15:36:25 | selection of field | semmle.label | selection of field |
26+
| jsoniter.go:39:31:39:45 | untrustedString | semmle.label | untrustedString |
27+
| jsoniter.go:39:48:39:53 | &... | semmle.label | &... |
28+
| jsoniter.go:40:15:40:25 | selection of field | semmle.label | selection of field |
29+
subpaths
30+
#select
131
| jsoniter.go:28:15:28:24 | selection of field | jsoniter.go:23:20:23:38 | call to getUntrustedBytes | jsoniter.go:28:15:28:24 | selection of field | This command depends on $@. | jsoniter.go:23:20:23:38 | call to getUntrustedBytes | a user-provided value |
232
| jsoniter.go:32:15:32:25 | selection of field | jsoniter.go:23:20:23:38 | call to getUntrustedBytes | jsoniter.go:32:15:32:25 | selection of field | This command depends on $@. | jsoniter.go:23:20:23:38 | call to getUntrustedBytes | a user-provided value |
333
| jsoniter.go:36:15:36:25 | selection of field | jsoniter.go:24:21:24:40 | call to getUntrustedString | jsoniter.go:36:15:36:25 | selection of field | This command depends on $@. | jsoniter.go:24:21:24:40 | call to getUntrustedString | a user-provided value |

‎go/ql/test/library-tests/semmle/go/frameworks/Encoding/jsoniter.ql‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import go
22
import semmle.go.security.CommandInjection
3+
import CommandInjection::Flow::PathGraph
34

45
class UntrustedFunction extends Function {
56
UntrustedFunction() { this.getName() = ["getUntrustedString", "getUntrustedBytes"] }
@@ -9,7 +10,7 @@ class UntrustedSource extends DataFlow::Node, UntrustedFlowSource::Range {
910
UntrustedSource() { this = any(UntrustedFunction f).getACall() }
1011
}
1112

12-
from CommandInjection::Configuration cfg, DataFlow::PathNode source, DataFlow::PathNode sink
13-
where cfg.hasFlowPath(source, sink)
13+
from CommandInjection::Flow::PathNode source, CommandInjection::Flow::PathNode sink
14+
where CommandInjection::Flow::flowPath(source, sink)
1415
select sink.getNode(), source, sink, "This command depends on $@.", source.getNode(),
1516
"a user-provided value"

‎go/ql/test/library-tests/semmle/go/frameworks/Gorestful/gorestful.expected‎

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,37 @@
1+
edges
2+
| gorestful.go:15:15:15:44 | call to QueryParameters | gorestful.go:15:15:15:47 | index expression |
3+
| gorestful.go:17:12:17:39 | call to BodyParameter | gorestful.go:18:15:18:17 | val |
4+
| gorestful.go:21:15:21:38 | call to PathParameters | gorestful.go:21:15:21:45 | index expression |
5+
| gorestful.go:23:21:23:24 | &... | gorestful.go:24:15:24:21 | selection of cmd |
6+
| gorestful_v2.go:15:15:15:44 | call to QueryParameters | gorestful_v2.go:15:15:15:47 | index expression |
7+
| gorestful_v2.go:17:12:17:39 | call to BodyParameter | gorestful_v2.go:18:15:18:17 | val |
8+
| gorestful_v2.go:21:15:21:38 | call to PathParameters | gorestful_v2.go:21:15:21:45 | index expression |
9+
| gorestful_v2.go:23:21:23:24 | &... | gorestful_v2.go:24:15:24:21 | selection of cmd |
10+
nodes
11+
| gorestful.go:15:15:15:44 | call to QueryParameters | semmle.label | call to QueryParameters |
12+
| gorestful.go:15:15:15:47 | index expression | semmle.label | index expression |
13+
| gorestful.go:16:15:16:43 | call to QueryParameter | semmle.label | call to QueryParameter |
14+
| gorestful.go:17:12:17:39 | call to BodyParameter | semmle.label | call to BodyParameter |
15+
| gorestful.go:18:15:18:17 | val | semmle.label | val |
16+
| gorestful.go:19:15:19:44 | call to HeaderParameter | semmle.label | call to HeaderParameter |
17+
| gorestful.go:20:15:20:42 | call to PathParameter | semmle.label | call to PathParameter |
18+
| gorestful.go:21:15:21:38 | call to PathParameters | semmle.label | call to PathParameters |
19+
| gorestful.go:21:15:21:45 | index expression | semmle.label | index expression |
20+
| gorestful.go:23:21:23:24 | &... | semmle.label | &... |
21+
| gorestful.go:24:15:24:21 | selection of cmd | semmle.label | selection of cmd |
22+
| gorestful_v2.go:15:15:15:44 | call to QueryParameters | semmle.label | call to QueryParameters |
23+
| gorestful_v2.go:15:15:15:47 | index expression | semmle.label | index expression |
24+
| gorestful_v2.go:16:15:16:43 | call to QueryParameter | semmle.label | call to QueryParameter |
25+
| gorestful_v2.go:17:12:17:39 | call to BodyParameter | semmle.label | call to BodyParameter |
26+
| gorestful_v2.go:18:15:18:17 | val | semmle.label | val |
27+
| gorestful_v2.go:19:15:19:44 | call to HeaderParameter | semmle.label | call to HeaderParameter |
28+
| gorestful_v2.go:20:15:20:42 | call to PathParameter | semmle.label | call to PathParameter |
29+
| gorestful_v2.go:21:15:21:38 | call to PathParameters | semmle.label | call to PathParameters |
30+
| gorestful_v2.go:21:15:21:45 | index expression | semmle.label | index expression |
31+
| gorestful_v2.go:23:21:23:24 | &... | semmle.label | &... |
32+
| gorestful_v2.go:24:15:24:21 | selection of cmd | semmle.label | selection of cmd |
33+
subpaths
34+
#select
135
| gorestful.go:15:15:15:47 | index expression | gorestful.go:15:15:15:44 | call to QueryParameters | gorestful.go:15:15:15:47 | index expression | This command depends on $@. | gorestful.go:15:15:15:44 | call to QueryParameters | a user-provided value |
236
| gorestful.go:16:15:16:43 | call to QueryParameter | gorestful.go:16:15:16:43 | call to QueryParameter | gorestful.go:16:15:16:43 | call to QueryParameter | This command depends on $@. | gorestful.go:16:15:16:43 | call to QueryParameter | a user-provided value |
337
| gorestful.go:18:15:18:17 | val | gorestful.go:17:12:17:39 | call to BodyParameter | gorestful.go:18:15:18:17 | val | This command depends on $@. | gorestful.go:17:12:17:39 | call to BodyParameter | a user-provided value |

‎go/ql/test/library-tests/semmle/go/frameworks/Gorestful/gorestful.ql‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,8 @@
11
import go
22
import semmle.go.security.CommandInjection
3+
import CommandInjection::Flow::PathGraph
34

4-
from CommandInjection::Configuration cfg, DataFlow::PathNode source, DataFlow::PathNode sink
5-
where cfg.hasFlowPath(source, sink)
5+
from CommandInjection::Flow::PathNode source, CommandInjection::Flow::PathNode sink
6+
where CommandInjection::Flow::flowPath(source, sink)
67
select sink.getNode(), source, sink, "This command depends on $@.", source.getNode(),
78
"a user-provided value"

0 commit comments

Comments
 (0)