Skip to content

Commit d3e70c1

Browse files
committed
JS: Add in-barrier to XSS query
This is a bit of a bandaid to cover issues with the push() method on next/router being treated as an array push, which causes it to flow into other taint sources.
1 parent 1b85feb commit d3e70c1

File tree

3 files changed

+8
-34
lines changed

3 files changed

+8
-34
lines changed

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

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -83,6 +83,8 @@ module DomBasedXssConfig implements DataFlow::StateConfigSig {
8383
node = DataFlow::MakeLabeledBarrierGuard<BarrierGuard>::getABarrierNode(lbl)
8484
}
8585

86+
predicate isBarrierIn(DataFlow::Node node, DataFlow::FlowLabel label) { isSource(node, label) }
87+
8688
predicate isAdditionalFlowStep(
8789
DataFlow::Node node1, DataFlow::FlowLabel state1, DataFlow::Node node2,
8890
DataFlow::FlowLabel state2

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

Lines changed: 3 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -258,10 +258,6 @@ nodes
258258
| react-use-router.js:8:21:8:39 | router.query.foobar | semmle.label | router.query.foobar |
259259
| react-use-router.js:11:24:11:35 | router.query | semmle.label | router.query |
260260
| react-use-router.js:11:24:11:42 | router.query.foobar | semmle.label | router.query.foobar |
261-
| react-use-router.js:23:31:23:36 | [post update] router | semmle.label | [post update] router |
262-
| react-use-router.js:23:31:23:36 | [post update] router [ArrayElement] | semmle.label | [post update] router [ArrayElement] |
263-
| react-use-router.js:23:43:23:48 | router | semmle.label | router |
264-
| react-use-router.js:23:43:23:48 | router [ArrayElement] | semmle.label | router [ArrayElement] |
265261
| react-use-router.js:23:43:23:54 | router.query | semmle.label | router.query |
266262
| react-use-router.js:23:43:23:61 | router.query.foobar | semmle.label | router.query.foobar |
267263
| react-use-router.js:33:21:33:32 | router.query | semmle.label | router.query |
@@ -501,8 +497,6 @@ nodes
501497
| tst.js:371:16:371:39 | documen ... .search | semmle.label | documen ... .search |
502498
| tst.js:374:18:374:23 | target | semmle.label | target |
503499
| tst.js:381:7:381:39 | target | semmle.label | target |
504-
| tst.js:381:7:381:39 | target [taint3] | semmle.label | target [taint3] |
505-
| tst.js:381:7:381:39 | target [taint8] | semmle.label | target [taint8] |
506500
| tst.js:381:16:381:39 | documen ... .search | semmle.label | documen ... .search |
507501
| tst.js:384:18:384:23 | target | semmle.label | target |
508502
| tst.js:386:18:386:23 | target | semmle.label | target |
@@ -824,13 +818,7 @@ edges
824818
| react-native.js:7:17:7:33 | req.param("code") | react-native.js:7:7:7:33 | tainted | provenance | |
825819
| react-use-router.js:8:21:8:32 | router.query | react-use-router.js:8:21:8:39 | router.query.foobar | provenance | |
826820
| react-use-router.js:11:24:11:35 | router.query | react-use-router.js:11:24:11:42 | router.query.foobar | provenance | |
827-
| react-use-router.js:23:31:23:36 | [post update] router | react-use-router.js:23:43:23:48 | router | provenance | |
828-
| react-use-router.js:23:31:23:36 | [post update] router [ArrayElement] | react-use-router.js:23:43:23:48 | router [ArrayElement] | provenance | |
829-
| react-use-router.js:23:43:23:48 | router | react-use-router.js:23:43:23:54 | router.query | provenance | |
830-
| react-use-router.js:23:43:23:48 | router [ArrayElement] | react-use-router.js:23:43:23:54 | router.query | provenance | |
831821
| react-use-router.js:23:43:23:54 | router.query | react-use-router.js:23:43:23:61 | router.query.foobar | provenance | |
832-
| react-use-router.js:23:43:23:61 | router.query.foobar | react-use-router.js:23:31:23:36 | [post update] router | provenance | |
833-
| react-use-router.js:23:43:23:61 | router.query.foobar | react-use-router.js:23:31:23:36 | [post update] router [ArrayElement] | provenance | |
834822
| react-use-router.js:33:21:33:32 | router.query | react-use-router.js:33:21:33:39 | router.query.foobar | provenance | |
835823
| react-use-state.js:4:9:4:49 | state | react-use-state.js:5:51:5:55 | state | provenance | |
836824
| react-use-state.js:4:38:4:48 | window.name | react-use-state.js:4:9:4:49 | state | provenance | |
@@ -1025,17 +1013,15 @@ edges
10251013
| tst.js:381:7:381:39 | target | tst.js:397:18:397:23 | target | provenance | |
10261014
| tst.js:381:7:381:39 | target | tst.js:406:18:406:23 | target | provenance | |
10271015
| tst.js:381:7:381:39 | target | tst.js:408:19:408:24 | target | provenance | |
1028-
| tst.js:381:7:381:39 | target [taint3] | tst.js:392:18:392:23 | target [taint3] | provenance | |
1029-
| tst.js:381:7:381:39 | target [taint8] | tst.js:408:19:408:24 | target [taint8] | provenance | |
1030-
| tst.js:381:7:381:39 | target [taint8] | tst.js:409:18:409:23 | target [taint8] | provenance | |
10311016
| tst.js:381:16:381:39 | documen ... .search | tst.js:381:7:381:39 | target | provenance | |
10321017
| tst.js:386:18:386:23 | target | tst.js:386:18:386:29 | target.taint | provenance | |
1033-
| tst.js:391:3:391:8 | [post update] target [taint3] | tst.js:381:7:381:39 | target [taint3] | provenance | |
1018+
| tst.js:391:3:391:8 | [post update] target [taint3] | tst.js:392:18:392:23 | target [taint3] | provenance | |
10341019
| tst.js:391:19:391:42 | documen ... .search | tst.js:391:3:391:8 | [post update] target [taint3] | provenance | |
10351020
| tst.js:392:18:392:23 | target [taint3] | tst.js:392:18:392:30 | target.taint3 | provenance | |
10361021
| tst.js:397:18:397:23 | target | tst.js:397:18:397:30 | target.taint5 | provenance | |
10371022
| tst.js:406:18:406:23 | target | tst.js:406:18:406:30 | target.taint7 | provenance | |
1038-
| tst.js:408:3:408:8 | [post update] target [taint8] | tst.js:381:7:381:39 | target [taint8] | provenance | |
1023+
| tst.js:408:3:408:8 | [post update] target [taint8] | tst.js:408:19:408:24 | target [taint8] | provenance | |
1024+
| tst.js:408:3:408:8 | [post update] target [taint8] | tst.js:409:18:409:23 | target [taint8] | provenance | |
10391025
| tst.js:408:19:408:24 | target | tst.js:408:19:408:31 | target.taint8 | provenance | |
10401026
| tst.js:408:19:408:24 | target [taint8] | tst.js:408:19:408:31 | target.taint8 | provenance | |
10411027
| tst.js:408:19:408:31 | target.taint8 | tst.js:408:3:408:8 | [post update] target [taint8] | provenance | |

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

Lines changed: 3 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -263,10 +263,6 @@ nodes
263263
| react-use-router.js:8:21:8:39 | router.query.foobar | semmle.label | router.query.foobar |
264264
| react-use-router.js:11:24:11:35 | router.query | semmle.label | router.query |
265265
| react-use-router.js:11:24:11:42 | router.query.foobar | semmle.label | router.query.foobar |
266-
| react-use-router.js:23:31:23:36 | [post update] router | semmle.label | [post update] router |
267-
| react-use-router.js:23:31:23:36 | [post update] router [ArrayElement] | semmle.label | [post update] router [ArrayElement] |
268-
| react-use-router.js:23:43:23:48 | router | semmle.label | router |
269-
| react-use-router.js:23:43:23:48 | router [ArrayElement] | semmle.label | router [ArrayElement] |
270266
| react-use-router.js:23:43:23:54 | router.query | semmle.label | router.query |
271267
| react-use-router.js:23:43:23:61 | router.query.foobar | semmle.label | router.query.foobar |
272268
| react-use-router.js:33:21:33:32 | router.query | semmle.label | router.query |
@@ -506,8 +502,6 @@ nodes
506502
| tst.js:371:16:371:39 | documen ... .search | semmle.label | documen ... .search |
507503
| tst.js:374:18:374:23 | target | semmle.label | target |
508504
| tst.js:381:7:381:39 | target | semmle.label | target |
509-
| tst.js:381:7:381:39 | target [taint3] | semmle.label | target [taint3] |
510-
| tst.js:381:7:381:39 | target [taint8] | semmle.label | target [taint8] |
511505
| tst.js:381:16:381:39 | documen ... .search | semmle.label | documen ... .search |
512506
| tst.js:384:18:384:23 | target | semmle.label | target |
513507
| tst.js:386:18:386:23 | target | semmle.label | target |
@@ -848,13 +842,7 @@ edges
848842
| react-native.js:7:17:7:33 | req.param("code") | react-native.js:7:7:7:33 | tainted | provenance | |
849843
| react-use-router.js:8:21:8:32 | router.query | react-use-router.js:8:21:8:39 | router.query.foobar | provenance | |
850844
| react-use-router.js:11:24:11:35 | router.query | react-use-router.js:11:24:11:42 | router.query.foobar | provenance | |
851-
| react-use-router.js:23:31:23:36 | [post update] router | react-use-router.js:23:43:23:48 | router | provenance | |
852-
| react-use-router.js:23:31:23:36 | [post update] router [ArrayElement] | react-use-router.js:23:43:23:48 | router [ArrayElement] | provenance | |
853-
| react-use-router.js:23:43:23:48 | router | react-use-router.js:23:43:23:54 | router.query | provenance | |
854-
| react-use-router.js:23:43:23:48 | router [ArrayElement] | react-use-router.js:23:43:23:54 | router.query | provenance | |
855845
| react-use-router.js:23:43:23:54 | router.query | react-use-router.js:23:43:23:61 | router.query.foobar | provenance | |
856-
| react-use-router.js:23:43:23:61 | router.query.foobar | react-use-router.js:23:31:23:36 | [post update] router | provenance | |
857-
| react-use-router.js:23:43:23:61 | router.query.foobar | react-use-router.js:23:31:23:36 | [post update] router [ArrayElement] | provenance | |
858846
| react-use-router.js:33:21:33:32 | router.query | react-use-router.js:33:21:33:39 | router.query.foobar | provenance | |
859847
| react-use-state.js:4:9:4:49 | state | react-use-state.js:5:51:5:55 | state | provenance | |
860848
| react-use-state.js:4:38:4:48 | window.name | react-use-state.js:4:9:4:49 | state | provenance | |
@@ -1049,17 +1037,15 @@ edges
10491037
| tst.js:381:7:381:39 | target | tst.js:397:18:397:23 | target | provenance | |
10501038
| tst.js:381:7:381:39 | target | tst.js:406:18:406:23 | target | provenance | |
10511039
| tst.js:381:7:381:39 | target | tst.js:408:19:408:24 | target | provenance | |
1052-
| tst.js:381:7:381:39 | target [taint3] | tst.js:392:18:392:23 | target [taint3] | provenance | |
1053-
| tst.js:381:7:381:39 | target [taint8] | tst.js:408:19:408:24 | target [taint8] | provenance | |
1054-
| tst.js:381:7:381:39 | target [taint8] | tst.js:409:18:409:23 | target [taint8] | provenance | |
10551040
| tst.js:381:16:381:39 | documen ... .search | tst.js:381:7:381:39 | target | provenance | |
10561041
| tst.js:386:18:386:23 | target | tst.js:386:18:386:29 | target.taint | provenance | |
1057-
| tst.js:391:3:391:8 | [post update] target [taint3] | tst.js:381:7:381:39 | target [taint3] | provenance | |
1042+
| tst.js:391:3:391:8 | [post update] target [taint3] | tst.js:392:18:392:23 | target [taint3] | provenance | |
10581043
| tst.js:391:19:391:42 | documen ... .search | tst.js:391:3:391:8 | [post update] target [taint3] | provenance | |
10591044
| tst.js:392:18:392:23 | target [taint3] | tst.js:392:18:392:30 | target.taint3 | provenance | |
10601045
| tst.js:397:18:397:23 | target | tst.js:397:18:397:30 | target.taint5 | provenance | |
10611046
| tst.js:406:18:406:23 | target | tst.js:406:18:406:30 | target.taint7 | provenance | |
1062-
| tst.js:408:3:408:8 | [post update] target [taint8] | tst.js:381:7:381:39 | target [taint8] | provenance | |
1047+
| tst.js:408:3:408:8 | [post update] target [taint8] | tst.js:408:19:408:24 | target [taint8] | provenance | |
1048+
| tst.js:408:3:408:8 | [post update] target [taint8] | tst.js:409:18:409:23 | target [taint8] | provenance | |
10631049
| tst.js:408:19:408:24 | target | tst.js:408:19:408:31 | target.taint8 | provenance | |
10641050
| tst.js:408:19:408:24 | target [taint8] | tst.js:408:19:408:31 | target.taint8 | provenance | |
10651051
| tst.js:408:19:408:31 | target.taint8 | tst.js:408:3:408:8 | [post update] target [taint8] | provenance | |

0 commit comments

Comments
 (0)