Skip to content

Commit 7790f68

Browse files
asgerfasgerf
committed
JS: Make the TaintedUrlSuffix library use optional steps/barriers
1 parent 3b34cd7 commit 7790f68

File tree

5 files changed

+21
-20
lines changed

5 files changed

+21
-20
lines changed

‎javascript/ql/lib/semmle/javascript/security/TaintedUrlSuffix.qll‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
*/
55

66
import javascript
7+
private import semmle.javascript.dataflow.internal.DataFlowPrivate as DataFlowPrivate
78

89
/**
910
* Provides a flow label for reasoning about URLs with a tainted query and fragment part,
@@ -45,6 +46,11 @@ module TaintedUrlSuffix {
4546
]
4647
}
4748

49+
predicate isBarrier(Node node, FlowLabel label) {
50+
label = label() and
51+
DataFlowPrivate::optionalBarrier(node, "tainted-url-suffix")
52+
}
53+
4854
/**
4955
* Holds if there is a flow step `src -> dst` involving the URL suffix taint label.
5056
*
@@ -57,6 +63,10 @@ module TaintedUrlSuffix {
5763
TaintTracking::AdditionalTaintStep::step(src, dst) and
5864
not isSafeLocationProp(dst)
5965
or
66+
srclbl = label() and
67+
dstlbl.isTaint() and
68+
DataFlowPrivate::optionalStep(src, "tainted-url-suffix", dst)
69+
or
6070
// Transition from URL suffix to full taint when extracting the query/fragment part.
6171
srclbl = label() and
6272
dstlbl.isTaint() and

‎javascript/ql/lib/semmle/javascript/security/dataflow/DomBasedXssQuery.qll‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -76,6 +76,8 @@ module DomBasedXssConfig implements DataFlow::StateConfigSig {
7676
isOptionallySanitizedNode(node) and
7777
lbl = [DataFlow::FlowLabel::taint(), prefixLabel(), TaintedUrlSuffix::label()]
7878
or
79+
TaintedUrlSuffix::isBarrier(node, lbl)
80+
or
7981
node = DataFlow::MakeLabeledBarrierGuard<BarrierGuard>::getABarrierNode(lbl)
8082
}
8183

‎javascript/ql/test/query-tests/Security/CWE-079/DomBasedXss/Xss.expected‎

Lines changed: 4 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -514,9 +514,6 @@ nodes
514514
| tst.js:371:7:371:39 | target | semmle.label | target |
515515
| tst.js:371:16:371:39 | documen ... .search | semmle.label | documen ... .search |
516516
| tst.js:374:18:374:23 | target | semmle.label | target |
517-
| tst.js:377:18:377:39 | documen ... on.href | semmle.label | documen ... on.href |
518-
| tst.js:377:18:377:50 | documen ... it("?") [ArrayElement] | semmle.label | documen ... it("?") [ArrayElement] |
519-
| tst.js:377:18:377:53 | documen ... "?")[0] | semmle.label | documen ... "?")[0] |
520517
| tst.js:381:7:381:39 | target | semmle.label | target |
521518
| tst.js:381:7:381:39 | target [taint3] | semmle.label | target [taint3] |
522519
| tst.js:381:7:381:39 | target [taint8] | semmle.label | target [taint8] |
@@ -549,7 +546,7 @@ nodes
549546
| tst.js:421:20:421:27 | match[1] | semmle.label | match[1] |
550547
| tst.js:424:18:424:37 | window.location.hash | semmle.label | window.location.hash |
551548
| tst.js:424:18:424:48 | window. ... it('#') | semmle.label | window. ... it('#') |
552-
| tst.js:424:18:424:48 | window. ... it('#') [ArrayElement] | semmle.label | window. ... it('#') [ArrayElement] |
549+
| tst.js:424:18:424:48 | window. ... it('#') [1] | semmle.label | window. ... it('#') [1] |
553550
| tst.js:424:18:424:51 | window. ... '#')[1] | semmle.label | window. ... '#')[1] |
554551
| tst.js:428:7:428:39 | target | semmle.label | target |
555552
| tst.js:428:16:428:39 | documen ... .search | semmle.label | documen ... .search |
@@ -1115,8 +1112,6 @@ edges
11151112
| tst.js:355:19:355:42 | documen ... .search | tst.js:355:10:355:42 | target | provenance | |
11161113
| tst.js:371:7:371:39 | target | tst.js:374:18:374:23 | target | provenance | |
11171114
| tst.js:371:16:371:39 | documen ... .search | tst.js:371:7:371:39 | target | provenance | |
1118-
| tst.js:377:18:377:39 | documen ... on.href | tst.js:377:18:377:50 | documen ... it("?") [ArrayElement] | provenance | |
1119-
| tst.js:377:18:377:50 | documen ... it("?") [ArrayElement] | tst.js:377:18:377:53 | documen ... "?")[0] | provenance | |
11201115
| tst.js:381:7:381:39 | target | tst.js:384:18:384:23 | target | provenance | |
11211116
| tst.js:381:7:381:39 | target | tst.js:386:18:386:23 | target | provenance | |
11221117
| tst.js:381:7:381:39 | target | tst.js:397:18:397:23 | target | provenance | |
@@ -1153,11 +1148,11 @@ edges
11531148
| tst.js:421:20:421:24 | match | tst.js:421:20:421:27 | match[1] | provenance | Config |
11541149
| tst.js:424:18:424:37 | window.location.hash | tst.js:424:18:424:48 | window. ... it('#') | provenance | |
11551150
| tst.js:424:18:424:37 | window.location.hash | tst.js:424:18:424:48 | window. ... it('#') | provenance | Config |
1156-
| tst.js:424:18:424:37 | window.location.hash | tst.js:424:18:424:48 | window. ... it('#') [ArrayElement] | provenance | |
1151+
| tst.js:424:18:424:37 | window.location.hash | tst.js:424:18:424:48 | window. ... it('#') [1] | provenance | Config |
11571152
| tst.js:424:18:424:48 | window. ... it('#') | tst.js:424:18:424:51 | window. ... '#')[1] | provenance | |
11581153
| tst.js:424:18:424:48 | window. ... it('#') | tst.js:424:18:424:51 | window. ... '#')[1] | provenance | Config |
1159-
| tst.js:424:18:424:48 | window. ... it('#') [ArrayElement] | tst.js:424:18:424:51 | window. ... '#')[1] | provenance | |
1160-
| tst.js:424:18:424:48 | window. ... it('#') [ArrayElement] | tst.js:424:18:424:51 | window. ... '#')[1] | provenance | Config |
1154+
| tst.js:424:18:424:48 | window. ... it('#') [1] | tst.js:424:18:424:51 | window. ... '#')[1] | provenance | |
1155+
| tst.js:424:18:424:48 | window. ... it('#') [1] | tst.js:424:18:424:51 | window. ... '#')[1] | provenance | Config |
11611156
| tst.js:428:7:428:39 | target | tst.js:430:18:430:23 | target | provenance | |
11621157
| tst.js:428:16:428:39 | documen ... .search | tst.js:428:7:428:39 | target | provenance | |
11631158
| tst.js:430:18:430:23 | target | tst.js:430:18:430:89 | target. ... data>') | provenance | |
@@ -1460,7 +1455,6 @@ subpaths
14601455
| tst.js:360:21:360:26 | target | tst.js:355:19:355:42 | documen ... .search | tst.js:360:21:360:26 | target | Cross-site scripting vulnerability due to $@. | tst.js:355:19:355:42 | documen ... .search | user-provided value |
14611456
| tst.js:363:18:363:23 | target | tst.js:355:19:355:42 | documen ... .search | tst.js:363:18:363:23 | target | Cross-site scripting vulnerability due to $@. | tst.js:355:19:355:42 | documen ... .search | user-provided value |
14621457
| tst.js:374:18:374:23 | target | tst.js:371:16:371:39 | documen ... .search | tst.js:374:18:374:23 | target | Cross-site scripting vulnerability due to $@. | tst.js:371:16:371:39 | documen ... .search | user-provided value |
1463-
| tst.js:377:18:377:53 | documen ... "?")[0] | tst.js:377:18:377:39 | documen ... on.href | tst.js:377:18:377:53 | documen ... "?")[0] | Cross-site scripting vulnerability due to $@. | tst.js:377:18:377:39 | documen ... on.href | user-provided value |
14641458
| tst.js:384:18:384:23 | target | tst.js:381:16:381:39 | documen ... .search | tst.js:384:18:384:23 | target | Cross-site scripting vulnerability due to $@. | tst.js:381:16:381:39 | documen ... .search | user-provided value |
14651459
| tst.js:386:18:386:29 | target.taint | tst.js:381:16:381:39 | documen ... .search | tst.js:386:18:386:29 | target.taint | Cross-site scripting vulnerability due to $@. | tst.js:381:16:381:39 | documen ... .search | user-provided value |
14661460
| tst.js:392:18:392:30 | target.taint3 | tst.js:391:19:391:42 | documen ... .search | tst.js:392:18:392:30 | target.taint3 | Cross-site scripting vulnerability due to $@. | tst.js:391:19:391:42 | documen ... .search | user-provided value |

‎javascript/ql/test/query-tests/Security/CWE-079/DomBasedXss/XssWithAdditionalSources.expected‎

Lines changed: 4 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -519,9 +519,6 @@ nodes
519519
| tst.js:371:7:371:39 | target | semmle.label | target |
520520
| tst.js:371:16:371:39 | documen ... .search | semmle.label | documen ... .search |
521521
| tst.js:374:18:374:23 | target | semmle.label | target |
522-
| tst.js:377:18:377:39 | documen ... on.href | semmle.label | documen ... on.href |
523-
| tst.js:377:18:377:50 | documen ... it("?") [ArrayElement] | semmle.label | documen ... it("?") [ArrayElement] |
524-
| tst.js:377:18:377:53 | documen ... "?")[0] | semmle.label | documen ... "?")[0] |
525522
| tst.js:381:7:381:39 | target | semmle.label | target |
526523
| tst.js:381:7:381:39 | target [taint3] | semmle.label | target [taint3] |
527524
| tst.js:381:7:381:39 | target [taint8] | semmle.label | target [taint8] |
@@ -554,7 +551,7 @@ nodes
554551
| tst.js:421:20:421:27 | match[1] | semmle.label | match[1] |
555552
| tst.js:424:18:424:37 | window.location.hash | semmle.label | window.location.hash |
556553
| tst.js:424:18:424:48 | window. ... it('#') | semmle.label | window. ... it('#') |
557-
| tst.js:424:18:424:48 | window. ... it('#') [ArrayElement] | semmle.label | window. ... it('#') [ArrayElement] |
554+
| tst.js:424:18:424:48 | window. ... it('#') [1] | semmle.label | window. ... it('#') [1] |
558555
| tst.js:424:18:424:51 | window. ... '#')[1] | semmle.label | window. ... '#')[1] |
559556
| tst.js:428:7:428:39 | target | semmle.label | target |
560557
| tst.js:428:16:428:39 | documen ... .search | semmle.label | documen ... .search |
@@ -1140,8 +1137,6 @@ edges
11401137
| tst.js:355:19:355:42 | documen ... .search | tst.js:355:10:355:42 | target | provenance | |
11411138
| tst.js:371:7:371:39 | target | tst.js:374:18:374:23 | target | provenance | |
11421139
| tst.js:371:16:371:39 | documen ... .search | tst.js:371:7:371:39 | target | provenance | |
1143-
| tst.js:377:18:377:39 | documen ... on.href | tst.js:377:18:377:50 | documen ... it("?") [ArrayElement] | provenance | |
1144-
| tst.js:377:18:377:50 | documen ... it("?") [ArrayElement] | tst.js:377:18:377:53 | documen ... "?")[0] | provenance | |
11451140
| tst.js:381:7:381:39 | target | tst.js:384:18:384:23 | target | provenance | |
11461141
| tst.js:381:7:381:39 | target | tst.js:386:18:386:23 | target | provenance | |
11471142
| tst.js:381:7:381:39 | target | tst.js:397:18:397:23 | target | provenance | |
@@ -1178,11 +1173,11 @@ edges
11781173
| tst.js:421:20:421:24 | match | tst.js:421:20:421:27 | match[1] | provenance | Config |
11791174
| tst.js:424:18:424:37 | window.location.hash | tst.js:424:18:424:48 | window. ... it('#') | provenance | |
11801175
| tst.js:424:18:424:37 | window.location.hash | tst.js:424:18:424:48 | window. ... it('#') | provenance | Config |
1181-
| tst.js:424:18:424:37 | window.location.hash | tst.js:424:18:424:48 | window. ... it('#') [ArrayElement] | provenance | |
1176+
| tst.js:424:18:424:37 | window.location.hash | tst.js:424:18:424:48 | window. ... it('#') [1] | provenance | Config |
11821177
| tst.js:424:18:424:48 | window. ... it('#') | tst.js:424:18:424:51 | window. ... '#')[1] | provenance | |
11831178
| tst.js:424:18:424:48 | window. ... it('#') | tst.js:424:18:424:51 | window. ... '#')[1] | provenance | Config |
1184-
| tst.js:424:18:424:48 | window. ... it('#') [ArrayElement] | tst.js:424:18:424:51 | window. ... '#')[1] | provenance | |
1185-
| tst.js:424:18:424:48 | window. ... it('#') [ArrayElement] | tst.js:424:18:424:51 | window. ... '#')[1] | provenance | Config |
1179+
| tst.js:424:18:424:48 | window. ... it('#') [1] | tst.js:424:18:424:51 | window. ... '#')[1] | provenance | |
1180+
| tst.js:424:18:424:48 | window. ... it('#') [1] | tst.js:424:18:424:51 | window. ... '#')[1] | provenance | Config |
11861181
| tst.js:428:7:428:39 | target | tst.js:430:18:430:23 | target | provenance | |
11871182
| tst.js:428:16:428:39 | documen ... .search | tst.js:428:7:428:39 | target | provenance | |
11881183
| tst.js:430:18:430:23 | target | tst.js:430:18:430:89 | target. ... data>') | provenance | |

‎javascript/ql/test/query-tests/Security/CWE-079/DomBasedXss/tst.js‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -373,7 +373,7 @@ function test() {
373373
// NOT OK
374374
$('myId').html(target)
375375

376-
// OK [INCONSISTENCY] (TODO: fix)
376+
// OK
377377
$('myid').html(document.location.href.split("?")[0]);
378378
}
379379

0 commit comments

Comments
 (0)