Skip to content

Commit ae4b3e8

Browse files
MathiasVPMathiasVP
authored
Merge pull request #263 from microsoft/remove-env-reads-from-sql-injection
PS: Remove environment variables from `powershell/microsoft/public/sql-injection`
2 parents 7c83d9d + 7991eb4 commit ae4b3e8

File tree

11 files changed

+254
-144
lines changed

11 files changed

+254
-144
lines changed

‎powershell/ql/lib/semmle/code/powershell/dataflow/internal/DataFlowPrivate.qll

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -428,7 +428,11 @@ predicate nodeIsHidden(Node n) { n.(NodeImpl).nodeIsHidden() }
428428
* Holds if `n` should never be skipped over in the `PathGraph` and in path
429429
* explanations.
430430
*/
431-
predicate neverSkipInPathGraph(Node n) { isReturned(n.(AstNode).getCfgNode()) }
431+
predicate neverSkipInPathGraph(Node n) {
432+
isReturned(n.(AstNode).getCfgNode())
433+
or
434+
n = any(SsaDefinitionNodeImpl def | not def.nodeIsHidden())
435+
}
432436

433437
/** An SSA node. */
434438
class SsaNode extends NodeImpl, TSsaNode {
@@ -439,9 +443,6 @@ class SsaNode extends NodeImpl, TSsaNode {
439443
/** Gets the underlying variable. */
440444
Variable getVariable() { result = node.getSourceVariable() }
441445

442-
/** Holds if this node should be hidden from path explanations. */
443-
predicate isHidden() { any() }
444-
445446
override CfgScope getCfgScope() { result = node.getBasicBlock().getScope() }
446447

447448
override Location getLocationImpl() { result = node.getLocation() }
@@ -454,7 +455,7 @@ class SsaDefinitionNodeImpl extends SsaNode {
454455

455456
Ssa::Definition getDefinition() { result = node.getDefinition() }
456457

457-
override predicate isHidden() {
458+
override predicate nodeIsHidden() {
458459
exists(SsaImpl::Definition def | def = this.getDefinition() |
459460
not def instanceof Ssa::WriteDefinition
460461
or

‎powershell/ql/lib/semmle/code/powershell/security/SqlInjectionCustomizations.qll

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -38,8 +38,13 @@ module SqlInjection {
3838
abstract class Sanitizer extends DataFlow::Node { }
3939

4040
/** A source of user input, considered as a flow source for command injection. */
41-
class FlowSourceAsSource extends Source instanceof SourceNode {
42-
override string getSourceType() { result = SourceNode.super.getSourceType() }
41+
class FlowSourceAsSource extends Source {
42+
FlowSourceAsSource() {
43+
this instanceof SourceNode and
44+
not this instanceof EnvironmentVariableSource
45+
}
46+
47+
override string getSourceType() { result = this.(SourceNode).getSourceType() }
4348
}
4449

4550
class InvokeSqlCmdSink extends Sink {

‎powershell/ql/test/library-tests/dataflow/fields/test.expected

Lines changed: 38 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -33,15 +33,19 @@ edges
3333
| test.ps1:32:6:32:13 | ...[...] [unknown] | test.ps1:32:6:32:16 | ...[...] | provenance | |
3434
| test.ps1:33:6:33:10 | arr7 [unknown, unknown] | test.ps1:33:6:33:21 | ...[...] [unknown] | provenance | |
3535
| test.ps1:33:6:33:21 | ...[...] [unknown] | test.ps1:33:6:33:32 | ...[...] | provenance | |
36-
| test.ps1:35:6:35:16 | Call to source | test.ps1:37:15:37:16 | x | provenance | |
37-
| test.ps1:37:9:37:16 | ...,... [element 2] | test.ps1:40:6:40:10 | arr8 [element 2] | provenance | |
38-
| test.ps1:37:9:37:16 | ...,... [element 2] | test.ps1:41:6:41:10 | arr8 [element 2] | provenance | |
36+
| test.ps1:35:1:35:2 | x | test.ps1:37:15:37:16 | x | provenance | |
37+
| test.ps1:35:6:35:16 | Call to source | test.ps1:35:1:35:2 | x | provenance | |
38+
| test.ps1:37:1:37:5 | arr8 [element 2] | test.ps1:40:6:40:10 | arr8 [element 2] | provenance | |
39+
| test.ps1:37:1:37:5 | arr8 [element 2] | test.ps1:41:6:41:10 | arr8 [element 2] | provenance | |
40+
| test.ps1:37:9:37:16 | ...,... [element 2] | test.ps1:37:1:37:5 | arr8 [element 2] | provenance | |
3941
| test.ps1:37:15:37:16 | x | test.ps1:37:9:37:16 | ...,... [element 2] | provenance | |
4042
| test.ps1:40:6:40:10 | arr8 [element 2] | test.ps1:40:6:40:13 | ...[...] | provenance | |
4143
| test.ps1:41:6:41:10 | arr8 [element 2] | test.ps1:41:6:41:20 | ...[...] | provenance | |
42-
| test.ps1:43:6:43:16 | Call to source | test.ps1:45:17:45:18 | y | provenance | |
43-
| test.ps1:45:9:45:19 | @(...) [element 2] | test.ps1:48:6:48:10 | arr9 [element 2] | provenance | |
44-
| test.ps1:45:9:45:19 | @(...) [element 2] | test.ps1:49:6:49:10 | arr9 [element 2] | provenance | |
44+
| test.ps1:43:1:43:2 | y | test.ps1:45:17:45:18 | y | provenance | |
45+
| test.ps1:43:6:43:16 | Call to source | test.ps1:43:1:43:2 | y | provenance | |
46+
| test.ps1:45:1:45:5 | arr9 [element 2] | test.ps1:48:6:48:10 | arr9 [element 2] | provenance | |
47+
| test.ps1:45:1:45:5 | arr9 [element 2] | test.ps1:49:6:49:10 | arr9 [element 2] | provenance | |
48+
| test.ps1:45:9:45:19 | @(...) [element 2] | test.ps1:45:1:45:5 | arr9 [element 2] | provenance | |
4549
| test.ps1:45:17:45:18 | y | test.ps1:45:9:45:19 | @(...) [element 2] | provenance | |
4650
| test.ps1:48:6:48:10 | arr9 [element 2] | test.ps1:48:6:48:13 | ...[...] | provenance | |
4751
| test.ps1:49:6:49:10 | arr9 [element 2] | test.ps1:49:6:49:20 | ...[...] | provenance | |
@@ -50,20 +54,29 @@ edges
5054
| test.ps1:61:1:61:8 | [post] myClass [field] | test.ps1:63:1:63:8 | myClass [field] | provenance | |
5155
| test.ps1:61:18:61:28 | Call to source | test.ps1:61:1:61:8 | [post] myClass [field] | provenance | |
5256
| test.ps1:63:1:63:8 | myClass [field] | test.ps1:54:5:56:5 | this [field] | provenance | |
53-
| test.ps1:66:10:66:20 | Call to source | test.ps1:69:5:69:6 | x | provenance | |
54-
| test.ps1:67:10:67:20 | Call to source | test.ps1:70:5:70:6 | y | provenance | |
55-
| test.ps1:68:10:68:20 | Call to source | test.ps1:70:9:70:10 | z | provenance | |
57+
| test.ps1:66:5:66:6 | x | test.ps1:69:5:69:6 | x | provenance | |
58+
| test.ps1:66:5:66:6 | x | test.ps1:69:5:69:6 | x | provenance | |
59+
| test.ps1:66:10:66:20 | Call to source | test.ps1:66:5:66:6 | x | provenance | |
60+
| test.ps1:66:10:66:20 | Call to source | test.ps1:66:5:66:6 | x | provenance | |
61+
| test.ps1:67:5:67:6 | y | test.ps1:70:5:70:6 | y | provenance | |
62+
| test.ps1:67:5:67:6 | y | test.ps1:70:5:70:6 | y | provenance | |
63+
| test.ps1:67:10:67:20 | Call to source | test.ps1:67:5:67:6 | y | provenance | |
64+
| test.ps1:67:10:67:20 | Call to source | test.ps1:67:5:67:6 | y | provenance | |
65+
| test.ps1:68:5:68:6 | z | test.ps1:70:9:70:10 | z | provenance | |
66+
| test.ps1:68:10:68:20 | Call to source | test.ps1:68:5:68:6 | z | provenance | |
5667
| test.ps1:69:5:69:6 | x | test.ps1:73:6:73:12 | Call to produce [unknown index] | provenance | |
5768
| test.ps1:70:5:70:6 | y | test.ps1:73:6:73:12 | Call to produce [unknown index] | provenance | |
5869
| test.ps1:70:9:70:10 | z | test.ps1:73:6:73:12 | Call to produce [unknown index] | provenance | |
59-
| test.ps1:73:6:73:12 | Call to produce [unknown index] | test.ps1:74:6:74:7 | x [unknown index] | provenance | |
60-
| test.ps1:73:6:73:12 | Call to produce [unknown index] | test.ps1:75:6:75:7 | x [unknown index] | provenance | |
61-
| test.ps1:73:6:73:12 | Call to produce [unknown index] | test.ps1:76:6:76:7 | x [unknown index] | provenance | |
70+
| test.ps1:73:1:73:2 | x [unknown index] | test.ps1:74:6:74:7 | x [unknown index] | provenance | |
71+
| test.ps1:73:1:73:2 | x [unknown index] | test.ps1:75:6:75:7 | x [unknown index] | provenance | |
72+
| test.ps1:73:1:73:2 | x [unknown index] | test.ps1:76:6:76:7 | x [unknown index] | provenance | |
73+
| test.ps1:73:6:73:12 | Call to produce [unknown index] | test.ps1:73:1:73:2 | x [unknown index] | provenance | |
6274
| test.ps1:74:6:74:7 | x [unknown index] | test.ps1:74:6:74:10 | ...[...] | provenance | |
6375
| test.ps1:75:6:75:7 | x [unknown index] | test.ps1:75:6:75:10 | ...[...] | provenance | |
6476
| test.ps1:76:6:76:7 | x [unknown index] | test.ps1:76:6:76:10 | ...[...] | provenance | |
65-
| test.ps1:78:9:81:1 | ${...} [element a] | test.ps1:83:6:83:10 | hash [element a] | provenance | |
66-
| test.ps1:78:9:81:1 | ${...} [element a] | test.ps1:87:6:87:10 | hash [element a] | provenance | |
77+
| test.ps1:78:1:78:5 | hash [element a] | test.ps1:83:6:83:10 | hash [element a] | provenance | |
78+
| test.ps1:78:1:78:5 | hash [element a] | test.ps1:87:6:87:10 | hash [element a] | provenance | |
79+
| test.ps1:78:9:81:1 | ${...} [element a] | test.ps1:78:1:78:5 | hash [element a] | provenance | |
6780
| test.ps1:79:7:79:17 | Call to source | test.ps1:78:9:81:1 | ${...} [element a] | provenance | |
6881
| test.ps1:83:6:83:10 | hash [element a] | test.ps1:83:6:83:15 | ...[...] | provenance | |
6982
| test.ps1:87:6:87:10 | hash [element a] | test.ps1:87:6:87:15 | ...[...] | provenance | |
@@ -112,14 +125,18 @@ nodes
112125
| test.ps1:33:6:33:10 | arr7 [unknown, unknown] | semmle.label | arr7 [unknown, unknown] |
113126
| test.ps1:33:6:33:21 | ...[...] [unknown] | semmle.label | ...[...] [unknown] |
114127
| test.ps1:33:6:33:32 | ...[...] | semmle.label | ...[...] |
128+
| test.ps1:35:1:35:2 | x | semmle.label | x |
115129
| test.ps1:35:6:35:16 | Call to source | semmle.label | Call to source |
130+
| test.ps1:37:1:37:5 | arr8 [element 2] | semmle.label | arr8 [element 2] |
116131
| test.ps1:37:9:37:16 | ...,... [element 2] | semmle.label | ...,... [element 2] |
117132
| test.ps1:37:15:37:16 | x | semmle.label | x |
118133
| test.ps1:40:6:40:10 | arr8 [element 2] | semmle.label | arr8 [element 2] |
119134
| test.ps1:40:6:40:13 | ...[...] | semmle.label | ...[...] |
120135
| test.ps1:41:6:41:10 | arr8 [element 2] | semmle.label | arr8 [element 2] |
121136
| test.ps1:41:6:41:20 | ...[...] | semmle.label | ...[...] |
137+
| test.ps1:43:1:43:2 | y | semmle.label | y |
122138
| test.ps1:43:6:43:16 | Call to source | semmle.label | Call to source |
139+
| test.ps1:45:1:45:5 | arr9 [element 2] | semmle.label | arr9 [element 2] |
123140
| test.ps1:45:9:45:19 | @(...) [element 2] | semmle.label | @(...) [element 2] |
124141
| test.ps1:45:17:45:18 | y | semmle.label | y |
125142
| test.ps1:48:6:48:10 | arr9 [element 2] | semmle.label | arr9 [element 2] |
@@ -132,19 +149,26 @@ nodes
132149
| test.ps1:61:1:61:8 | [post] myClass [field] | semmle.label | [post] myClass [field] |
133150
| test.ps1:61:18:61:28 | Call to source | semmle.label | Call to source |
134151
| test.ps1:63:1:63:8 | myClass [field] | semmle.label | myClass [field] |
152+
| test.ps1:66:5:66:6 | x | semmle.label | x |
153+
| test.ps1:66:5:66:6 | x | semmle.label | x |
135154
| test.ps1:66:10:66:20 | Call to source | semmle.label | Call to source |
155+
| test.ps1:67:5:67:6 | y | semmle.label | y |
156+
| test.ps1:67:5:67:6 | y | semmle.label | y |
136157
| test.ps1:67:10:67:20 | Call to source | semmle.label | Call to source |
158+
| test.ps1:68:5:68:6 | z | semmle.label | z |
137159
| test.ps1:68:10:68:20 | Call to source | semmle.label | Call to source |
138160
| test.ps1:69:5:69:6 | x | semmle.label | x |
139161
| test.ps1:70:5:70:6 | y | semmle.label | y |
140162
| test.ps1:70:9:70:10 | z | semmle.label | z |
163+
| test.ps1:73:1:73:2 | x [unknown index] | semmle.label | x [unknown index] |
141164
| test.ps1:73:6:73:12 | Call to produce [unknown index] | semmle.label | Call to produce [unknown index] |
142165
| test.ps1:74:6:74:7 | x [unknown index] | semmle.label | x [unknown index] |
143166
| test.ps1:74:6:74:10 | ...[...] | semmle.label | ...[...] |
144167
| test.ps1:75:6:75:7 | x [unknown index] | semmle.label | x [unknown index] |
145168
| test.ps1:75:6:75:10 | ...[...] | semmle.label | ...[...] |
146169
| test.ps1:76:6:76:7 | x [unknown index] | semmle.label | x [unknown index] |
147170
| test.ps1:76:6:76:10 | ...[...] | semmle.label | ...[...] |
171+
| test.ps1:78:1:78:5 | hash [element a] | semmle.label | hash [element a] |
148172
| test.ps1:78:9:81:1 | ${...} [element a] | semmle.label | ${...} [element a] |
149173
| test.ps1:79:7:79:17 | Call to source | semmle.label | Call to source |
150174
| test.ps1:83:6:83:10 | hash [element a] | semmle.label | hash [element a] |

‎powershell/ql/test/library-tests/dataflow/global/test.expected

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,14 @@
11
models
22
edges
3-
| test.ps1:2:14:2:23 | Call to source | test.ps1:5:25:7:1 | <initial env var> env:x | provenance | |
4-
| test.ps1:5:25:7:1 | <initial env var> env:x | test.ps1:6:5:6:15 | env:x | provenance | |
5-
| test.ps1:16:18:16:27 | Call to source | test.ps1:5:25:7:1 | <initial env var> env:x | provenance | |
3+
| test.ps1:2:5:2:23 | env:x | test.ps1:6:5:6:15 | env:x | provenance | |
4+
| test.ps1:2:14:2:23 | Call to source | test.ps1:2:5:2:23 | env:x | provenance | |
5+
| test.ps1:16:9:16:27 | env:x | test.ps1:6:5:6:15 | env:x | provenance | |
6+
| test.ps1:16:18:16:27 | Call to source | test.ps1:16:9:16:27 | env:x | provenance | |
67
nodes
8+
| test.ps1:2:5:2:23 | env:x | semmle.label | env:x |
79
| test.ps1:2:14:2:23 | Call to source | semmle.label | Call to source |
8-
| test.ps1:5:25:7:1 | <initial env var> env:x | semmle.label | <initial env var> env:x |
910
| test.ps1:6:5:6:15 | env:x | semmle.label | env:x |
11+
| test.ps1:16:9:16:27 | env:x | semmle.label | env:x |
1012
| test.ps1:16:18:16:27 | Call to source | semmle.label | Call to source |
1113
subpaths
1214
testFailures

‎powershell/ql/test/library-tests/dataflow/mad/flow.expected

Lines changed: 15 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -5,19 +5,24 @@ edges
55
| file://:0:0:0:0 | [summary param] pos(0, {}) in system.management.automation.language.codegeneration!;Method[escapesinglequotedstringcontent] | file://:0:0:0:0 | [summary] to write: ReturnValue in system.management.automation.language.codegeneration!;Method[escapesinglequotedstringcontent] | provenance | |
66
| file://:0:0:0:0 | [summary] read: Argument[pipeline].Element[?] in microsoft.powershell.utility!;Method[join-string] | file://:0:0:0:0 | [summary] to write: ReturnValue in microsoft.powershell.utility!;Method[join-string] | provenance | |
77
| file://:0:0:0:0 | [summary] read: Argument[pipeline].Element[?] in microsoft.powershell.utility!;Method[join-string] | file://:0:0:0:0 | [summary] to write: ReturnValue in microsoft.powershell.utility!;Method[join-string] | provenance | |
8-
| test.ps1:1:6:1:15 | Call to source | test.ps1:2:94:2:95 | x | provenance | |
9-
| test.ps1:2:6:2:96 | Call to escapesinglequotedstringcontent | test.ps1:3:6:3:7 | y | provenance | |
8+
| test.ps1:1:1:1:2 | x | test.ps1:2:94:2:95 | x | provenance | |
9+
| test.ps1:1:6:1:15 | Call to source | test.ps1:1:1:1:2 | x | provenance | |
10+
| test.ps1:2:1:2:2 | y | test.ps1:3:6:3:7 | y | provenance | |
11+
| test.ps1:2:6:2:96 | Call to escapesinglequotedstringcontent | test.ps1:2:1:2:2 | y | provenance | |
1012
| test.ps1:2:94:2:95 | x | file://:0:0:0:0 | [summary param] pos(0, {}) in system.management.automation.language.codegeneration!;Method[escapesinglequotedstringcontent] | provenance | |
1113
| test.ps1:2:94:2:95 | x | test.ps1:2:6:2:96 | Call to escapesinglequotedstringcontent | provenance | |
12-
| test.ps1:5:6:5:15 | Call to source | test.ps1:7:6:7:7 | x | provenance | |
13-
| test.ps1:6:6:6:15 | Call to source | test.ps1:7:10:7:11 | y | provenance | |
14+
| test.ps1:5:1:5:2 | x | test.ps1:7:6:7:7 | x | provenance | |
15+
| test.ps1:5:6:5:15 | Call to source | test.ps1:5:1:5:2 | x | provenance | |
16+
| test.ps1:6:1:6:2 | y | test.ps1:7:10:7:11 | y | provenance | |
17+
| test.ps1:6:6:6:15 | Call to source | test.ps1:6:1:6:2 | y | provenance | |
18+
| test.ps1:7:1:7:2 | z | test.ps1:8:6:8:7 | z | provenance | |
1419
| test.ps1:7:6:7:7 | x | test.ps1:7:6:7:11 | ...,... [element 0] | provenance | |
1520
| test.ps1:7:6:7:11 | ...,... [element 0] | file://:0:0:0:0 | [summary param] pipeline in microsoft.powershell.utility!;Method[join-string] [element 0] | provenance | |
1621
| test.ps1:7:6:7:11 | ...,... [element 0] | test.ps1:7:15:7:25 | Call to join-string | provenance | |
1722
| test.ps1:7:6:7:11 | ...,... [element 1] | file://:0:0:0:0 | [summary param] pipeline in microsoft.powershell.utility!;Method[join-string] [element 1] | provenance | |
1823
| test.ps1:7:6:7:11 | ...,... [element 1] | test.ps1:7:15:7:25 | Call to join-string | provenance | |
1924
| test.ps1:7:10:7:11 | y | test.ps1:7:6:7:11 | ...,... [element 1] | provenance | |
20-
| test.ps1:7:15:7:25 | Call to join-string | test.ps1:8:6:8:7 | z | provenance | |
25+
| test.ps1:7:15:7:25 | Call to join-string | test.ps1:7:1:7:2 | z | provenance | |
2126
nodes
2227
| file://:0:0:0:0 | [summary param] pipeline in microsoft.powershell.utility!;Method[join-string] [element 0] | semmle.label | [summary param] pipeline in microsoft.powershell.utility!;Method[join-string] [element 0] |
2328
| file://:0:0:0:0 | [summary param] pipeline in microsoft.powershell.utility!;Method[join-string] [element 1] | semmle.label | [summary param] pipeline in microsoft.powershell.utility!;Method[join-string] [element 1] |
@@ -27,12 +32,17 @@ nodes
2732
| file://:0:0:0:0 | [summary] to write: ReturnValue in microsoft.powershell.utility!;Method[join-string] | semmle.label | [summary] to write: ReturnValue in microsoft.powershell.utility!;Method[join-string] |
2833
| file://:0:0:0:0 | [summary] to write: ReturnValue in microsoft.powershell.utility!;Method[join-string] | semmle.label | [summary] to write: ReturnValue in microsoft.powershell.utility!;Method[join-string] |
2934
| file://:0:0:0:0 | [summary] to write: ReturnValue in system.management.automation.language.codegeneration!;Method[escapesinglequotedstringcontent] | semmle.label | [summary] to write: ReturnValue in system.management.automation.language.codegeneration!;Method[escapesinglequotedstringcontent] |
35+
| test.ps1:1:1:1:2 | x | semmle.label | x |
3036
| test.ps1:1:6:1:15 | Call to source | semmle.label | Call to source |
37+
| test.ps1:2:1:2:2 | y | semmle.label | y |
3138
| test.ps1:2:6:2:96 | Call to escapesinglequotedstringcontent | semmle.label | Call to escapesinglequotedstringcontent |
3239
| test.ps1:2:94:2:95 | x | semmle.label | x |
3340
| test.ps1:3:6:3:7 | y | semmle.label | y |
41+
| test.ps1:5:1:5:2 | x | semmle.label | x |
3442
| test.ps1:5:6:5:15 | Call to source | semmle.label | Call to source |
43+
| test.ps1:6:1:6:2 | y | semmle.label | y |
3544
| test.ps1:6:6:6:15 | Call to source | semmle.label | Call to source |
45+
| test.ps1:7:1:7:2 | z | semmle.label | z |
3646
| test.ps1:7:6:7:7 | x | semmle.label | x |
3747
| test.ps1:7:6:7:11 | ...,... [element 0] | semmle.label | ...,... [element 0] |
3848
| test.ps1:7:6:7:11 | ...,... [element 1] | semmle.label | ...,... [element 1] |

0 commit comments

Comments
 (0)