Skip to content

Commit 651cc04

Browse files
chanel-ychanel-y
authored
Merge pull request #301 from microsoft/actually-require-string-concat
PS: Actually require string concatenation in SQL injection query
2 parents 92e83f9 + c163416 commit 651cc04

File tree

2 files changed

+50
-11
lines changed

2 files changed

+50
-11
lines changed

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

Lines changed: 24 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -12,13 +12,33 @@ import semmle.code.powershell.dataflow.TaintTracking
1212
import SqlInjectionCustomizations::SqlInjection
1313
import semmle.code.powershell.dataflow.DataFlow
1414

15-
private module Config implements DataFlow::ConfigSig {
16-
predicate isSource(DataFlow::Node source) { source instanceof Source }
15+
private module Config implements DataFlow::StateConfigSig {
16+
newtype FlowState =
17+
additional BeforeConcat() or
18+
additional AfterConcat()
1719

18-
predicate isSink(DataFlow::Node sink) { sink instanceof Sink }
20+
predicate isSource(DataFlow::Node source, FlowState state) {
21+
source instanceof Source and state = BeforeConcat()
22+
}
23+
24+
predicate isSink(DataFlow::Node sink, FlowState state) {
25+
sink instanceof Sink and state = AfterConcat()
26+
}
1927

2028
predicate isBarrier(DataFlow::Node node) { node instanceof Sanitizer }
2129

30+
predicate isAdditionalFlowStep(
31+
DataFlow::Node node1, FlowState state1, DataFlow::Node node2, FlowState state2
32+
) {
33+
state1 = BeforeConcat() and
34+
state2 = AfterConcat() and
35+
(
36+
TaintTracking::stringInterpolationTaintStep(node1, node2)
37+
or
38+
TaintTracking::operationTaintStep(node1, node2)
39+
)
40+
}
41+
2242
predicate allowImplicitRead(DataFlow::Node node, DataFlow::ContentSet cs) {
2343
node.(Sink).allowImplicitRead(cs)
2444
}
@@ -27,4 +47,4 @@ private module Config implements DataFlow::ConfigSig {
2747
/**
2848
* Taint-tracking for reasoning about SQL-injection vulnerabilities.
2949
*/
30-
module SqlInjectionFlow = TaintTracking::Global<Config>;
50+
module SqlInjectionFlow = TaintTracking::GlobalWithState<Config>;

‎powershell/ql/test/query-tests/security/cwe-089/SqlInjection.expected‎

Lines changed: 26 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,37 +1,56 @@
11
edges
2-
| test.ps1:1:1:1:10 | userinput | test.ps1:4:10:4:62 | SELECT * FROM MyTable WHERE MyColumn = '$userinput' | provenance | |
3-
| test.ps1:1:1:1:10 | userinput | test.ps1:8:1:8:6 | query | provenance | |
4-
| test.ps1:1:1:1:10 | userinput | test.ps1:17:24:17:76 | SELECT * FROM MyTable WHERE MyColumn = '$userinput' | provenance | |
5-
| test.ps1:1:1:1:10 | userinput | test.ps1:28:24:28:76 | SELECT * FROM MyTable WHERE MyColumn = '$userinput' | provenance | |
6-
| test.ps1:1:1:1:10 | userinput | test.ps1:78:13:78:59 | SELECT * FROM Customers WHERE id = $userinput | provenance | |
7-
| test.ps1:1:1:1:10 | userinput | test.ps1:128:28:128:37 | userinput | provenance | |
2+
| test.ps1:1:1:1:10 | userinput | test.ps1:4:51:4:60 | userinput | provenance | |
83
| test.ps1:1:14:1:45 | Call to read-host | test.ps1:1:1:1:10 | userinput | provenance | Src:MaD:0 |
94
| test.ps1:4:1:4:6 | query | test.ps1:5:72:5:77 | query | provenance | |
105
| test.ps1:4:10:4:62 | SELECT * FROM MyTable WHERE MyColumn = '$userinput' | test.ps1:4:1:4:6 | query | provenance | |
6+
| test.ps1:4:51:4:60 | userinput | test.ps1:4:10:4:62 | SELECT * FROM MyTable WHERE MyColumn = '$userinput' | provenance | Config |
7+
| test.ps1:4:51:4:60 | userinput | test.ps1:8:43:8:52 | userinput | provenance | |
118
| test.ps1:8:1:8:6 | query | test.ps1:9:72:9:77 | query | provenance | |
9+
| test.ps1:8:10:8:52 | ...+... | test.ps1:8:1:8:6 | query | provenance | |
10+
| test.ps1:8:43:8:52 | userinput | test.ps1:8:10:8:52 | ...+... | provenance | Config |
11+
| test.ps1:8:43:8:52 | userinput | test.ps1:17:65:17:74 | userinput | provenance | |
12+
| test.ps1:17:65:17:74 | userinput | test.ps1:17:24:17:76 | SELECT * FROM MyTable WHERE MyColumn = '$userinput' | provenance | Config |
13+
| test.ps1:17:65:17:74 | userinput | test.ps1:28:65:28:74 | userinput | provenance | |
14+
| test.ps1:28:65:28:74 | userinput | test.ps1:28:24:28:76 | SELECT * FROM MyTable WHERE MyColumn = '$userinput' | provenance | Config |
15+
| test.ps1:28:65:28:74 | userinput | test.ps1:78:49:78:58 | userinput | provenance | |
1216
| test.ps1:72:1:72:11 | QueryConn2 [element Query] | test.ps1:81:15:81:25 | QueryConn2 | provenance | |
1317
| test.ps1:72:15:79:1 | ${...} [element Query] | test.ps1:72:1:72:11 | QueryConn2 [element Query] | provenance | |
1418
| test.ps1:78:13:78:59 | SELECT * FROM Customers WHERE id = $userinput | test.ps1:72:15:79:1 | ${...} [element Query] | provenance | |
15-
| test.ps1:121:9:121:56 | unvalidated | test.ps1:125:128:125:142 | $(...) | provenance | |
19+
| test.ps1:78:49:78:58 | userinput | test.ps1:78:13:78:59 | SELECT * FROM Customers WHERE id = $userinput | provenance | Config |
20+
| test.ps1:78:49:78:58 | userinput | test.ps1:111:51:111:60 | userinput | provenance | |
21+
| test.ps1:111:51:111:60 | userinput | test.ps1:128:28:128:37 | userinput | provenance | |
22+
| test.ps1:121:9:121:56 | unvalidated | test.ps1:125:130:125:141 | unvalidated | provenance | |
1623
| test.ps1:125:128:125:142 | $(...) | test.ps1:125:92:125:143 | SELECT * FROM Customers where id = $($unvalidated) | provenance | |
24+
| test.ps1:125:128:125:142 | $(...) | test.ps1:125:92:125:143 | SELECT * FROM Customers where id = $($unvalidated) | provenance | Config |
25+
| test.ps1:125:130:125:141 | unvalidated | test.ps1:125:128:125:142 | $(...) | provenance | |
26+
| test.ps1:125:130:125:141 | unvalidated | test.ps1:125:128:125:142 | $(...) | provenance | Config |
1727
| test.ps1:128:28:128:37 | userinput | test.ps1:121:9:121:56 | unvalidated | provenance | |
1828
nodes
1929
| test.ps1:1:1:1:10 | userinput | semmle.label | userinput |
2030
| test.ps1:1:14:1:45 | Call to read-host | semmle.label | Call to read-host |
2131
| test.ps1:4:1:4:6 | query | semmle.label | query |
2232
| test.ps1:4:10:4:62 | SELECT * FROM MyTable WHERE MyColumn = '$userinput' | semmle.label | SELECT * FROM MyTable WHERE MyColumn = '$userinput' |
33+
| test.ps1:4:51:4:60 | userinput | semmle.label | userinput |
2334
| test.ps1:5:72:5:77 | query | semmle.label | query |
2435
| test.ps1:8:1:8:6 | query | semmle.label | query |
36+
| test.ps1:8:10:8:52 | ...+... | semmle.label | ...+... |
37+
| test.ps1:8:43:8:52 | userinput | semmle.label | userinput |
2538
| test.ps1:9:72:9:77 | query | semmle.label | query |
2639
| test.ps1:17:24:17:76 | SELECT * FROM MyTable WHERE MyColumn = '$userinput' | semmle.label | SELECT * FROM MyTable WHERE MyColumn = '$userinput' |
40+
| test.ps1:17:65:17:74 | userinput | semmle.label | userinput |
2741
| test.ps1:28:24:28:76 | SELECT * FROM MyTable WHERE MyColumn = '$userinput' | semmle.label | SELECT * FROM MyTable WHERE MyColumn = '$userinput' |
42+
| test.ps1:28:65:28:74 | userinput | semmle.label | userinput |
2843
| test.ps1:72:1:72:11 | QueryConn2 [element Query] | semmle.label | QueryConn2 [element Query] |
2944
| test.ps1:72:15:79:1 | ${...} [element Query] | semmle.label | ${...} [element Query] |
3045
| test.ps1:78:13:78:59 | SELECT * FROM Customers WHERE id = $userinput | semmle.label | SELECT * FROM Customers WHERE id = $userinput |
46+
| test.ps1:78:49:78:58 | userinput | semmle.label | userinput |
3147
| test.ps1:81:15:81:25 | QueryConn2 | semmle.label | QueryConn2 |
48+
| test.ps1:111:51:111:60 | userinput | semmle.label | userinput |
3249
| test.ps1:121:9:121:56 | unvalidated | semmle.label | unvalidated |
3350
| test.ps1:125:92:125:143 | SELECT * FROM Customers where id = $($unvalidated) | semmle.label | SELECT * FROM Customers where id = $($unvalidated) |
3451
| test.ps1:125:128:125:142 | $(...) | semmle.label | $(...) |
52+
| test.ps1:125:128:125:142 | $(...) | semmle.label | $(...) |
53+
| test.ps1:125:130:125:141 | unvalidated | semmle.label | unvalidated |
3554
| test.ps1:128:28:128:37 | userinput | semmle.label | userinput |
3655
subpaths
3756
#select

0 commit comments

Comments
 (0)