Skip to content

Commit 451f6f0

Browse files
erik-krogherik-krogh
authored
Merge pull request github#12633 from erik-krogh/more-global-flow
JS: better callgraph support for global variables
2 parents 7c74fd0 + 0462e2a commit 451f6f0

File tree

7 files changed

+45
-0
lines changed

7 files changed

+45
-0
lines changed

‎javascript/ql/lib/semmle/javascript/GlobalAccessPaths.qll

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -243,6 +243,11 @@ module AccessPath {
243243
root.isGlobal()
244244
)
245245
or
246+
exists(Assignment assign |
247+
fromReference(assign.getLhs().flow(), root) = result and
248+
node = assign.getRhs().flow()
249+
)
250+
or
246251
exists(FunctionDeclStmt fun |
247252
node = DataFlow::valueNode(fun) and
248253
result = fun.getIdentifier().(GlobalVarDecl).getName() and

‎javascript/ql/test/library-tests/CallGraphs/FullTest/tests.expected

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -85,6 +85,7 @@ test_getAFunctionValue
8585
| es2015.js:35:1:35:3 | sum | es2015.js:31:1:33:1 | functio ... +y+z;\\n} |
8686
| es2015.js:36:1:36:3 | sum | es2015.js:31:1:33:1 | functio ... +y+z;\\n} |
8787
| m2.js:2:6:2:18 | function() {} | m2.js:2:6:2:18 | function() {} |
88+
| m.js:1:1:1:9 | exports.f | m.js:1:13:1:25 | function() {} |
8889
| m.js:1:1:1:25 | exports ... on() {} | m.js:1:13:1:25 | function() {} |
8990
| m.js:1:13:1:25 | function() {} | m.js:1:13:1:25 | function() {} |
9091
| m.js:2:1:2:9 | exports.f | m.js:1:13:1:25 | function() {} |
@@ -100,16 +101,19 @@ test_getAFunctionValue
100101
| protoclass.js:3:10:3:10 | F | protoclass.js:3:1:5:1 | functio ... it();\\n} |
101102
| protoclass.js:4:3:4:11 | this.init | protoclass.js:7:20:11:1 | functio ... m();\\n} |
102103
| protoclass.js:7:1:7:1 | F | protoclass.js:3:1:5:1 | functio ... it();\\n} |
104+
| protoclass.js:7:1:7:16 | F.prototype.init | protoclass.js:7:20:11:1 | functio ... m();\\n} |
103105
| protoclass.js:7:1:11:1 | F.proto ... m();\\n} | protoclass.js:7:20:11:1 | functio ... m();\\n} |
104106
| protoclass.js:7:20:11:1 | functio ... m();\\n} | protoclass.js:7:20:11:1 | functio ... m();\\n} |
105107
| protoclass.js:8:3:8:13 | this.method | protoclass.js:13:22:13:34 | function() {} |
106108
| protoclass.js:9:11:9:21 | this.method | protoclass.js:13:22:13:34 | function() {} |
107109
| protoclass.js:13:1:13:1 | F | protoclass.js:3:1:5:1 | functio ... it();\\n} |
110+
| protoclass.js:13:1:13:18 | F.prototype.method | protoclass.js:13:22:13:34 | function() {} |
108111
| protoclass.js:13:1:13:34 | F.proto ... on() {} | protoclass.js:13:22:13:34 | function() {} |
109112
| protoclass.js:13:22:13:34 | function() {} | protoclass.js:13:22:13:34 | function() {} |
110113
| protoclass.js:15:16:15:16 | F | protoclass.js:3:1:5:1 | functio ... it();\\n} |
111114
| reflection.js:1:1:3:1 | functio ... x+y;\\n} | reflection.js:1:1:3:1 | functio ... x+y;\\n} |
112115
| reflection.js:5:3:5:5 | add | reflection.js:1:1:3:1 | functio ... x+y;\\n} |
116+
| reflection.js:5:3:5:11 | add.apply | reflection.js:5:15:5:39 | functio ... n 56; } |
113117
| reflection.js:5:3:5:39 | add.app ... n 56; } | reflection.js:5:15:5:39 | functio ... n 56; } |
114118
| reflection.js:5:15:5:14 | this | reflection.js:1:1:3:1 | functio ... x+y;\\n} |
115119
| reflection.js:5:15:5:39 | functio ... n 56; } | reflection.js:5:15:5:39 | functio ... n 56; } |
@@ -163,11 +167,13 @@ test_getAFunctionValue
163167
| tst.js:42:2:42:26 | functio ... rn x; } | tst.js:42:2:42:26 | functio ... rn x; } |
164168
| tst.js:44:1:44:15 | function A() {} | tst.js:44:1:44:15 | function A() {} |
165169
| tst.js:45:1:45:1 | A | tst.js:44:1:44:15 | function A() {} |
170+
| tst.js:45:1:45:13 | A.prototype.f | tst.js:45:17:47:1 | functio ... .g();\\n} |
166171
| tst.js:45:1:47:1 | A.proto ... .g();\\n} | tst.js:45:17:47:1 | functio ... .g();\\n} |
167172
| tst.js:45:17:47:1 | functio ... .g();\\n} | tst.js:45:17:47:1 | functio ... .g();\\n} |
168173
| tst.js:46:2:46:7 | this.g | tst.js:48:17:48:29 | function() {} |
169174
| tst.js:46:2:46:7 | this.g | tst.js:61:17:61:29 | function() {} |
170175
| tst.js:48:1:48:1 | A | tst.js:44:1:44:15 | function A() {} |
176+
| tst.js:48:1:48:13 | A.prototype.g | tst.js:48:17:48:29 | function() {} |
171177
| tst.js:48:1:48:29 | A.proto ... on() {} | tst.js:48:17:48:29 | function() {} |
172178
| tst.js:48:17:48:29 | function() {} | tst.js:48:17:48:29 | function() {} |
173179
| tst.js:50:1:50:15 | function B() {} | tst.js:50:1:50:15 | function B() {} |
@@ -186,11 +192,13 @@ test_getAFunctionValue
186192
| tst.js:60:1:60:1 | C | tst.js:59:1:59:15 | function C() {} |
187193
| tst.js:60:19:60:19 | A | tst.js:44:1:44:15 | function A() {} |
188194
| tst.js:61:1:61:1 | C | tst.js:59:1:59:15 | function C() {} |
195+
| tst.js:61:1:61:13 | C.prototype.g | tst.js:61:17:61:29 | function() {} |
189196
| tst.js:61:1:61:29 | C.proto ... on() {} | tst.js:61:17:61:29 | function() {} |
190197
| tst.js:61:17:61:29 | function() {} | tst.js:61:17:61:29 | function() {} |
191198
| tst.js:63:1:67:2 | (functi ... f();\\n}) | tst.js:63:2:67:1 | functio ... .f();\\n} |
192199
| tst.js:63:2:67:1 | functio ... .f();\\n} | tst.js:63:2:67:1 | functio ... .f();\\n} |
193200
| tst.js:64:17:64:17 | B | tst.js:50:1:50:15 | function B() {} |
201+
| tst.js:65:5:65:7 | b.f | tst.js:65:11:65:23 | function() {} |
194202
| tst.js:65:5:65:23 | b.f = function() {} | tst.js:65:11:65:23 | function() {} |
195203
| tst.js:65:11:65:23 | function() {} | tst.js:65:11:65:23 | function() {} |
196204
| tst.js:66:5:66:7 | b.f | tst.js:52:5:54:2 | functio ... g();\\n\\t} |

‎javascript/ql/test/library-tests/TypeTracking/ClassStyle.expected

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ test_ApiObject
1212
test_Connection
1313
| client.js:1:10:1:27 | exportedConnection |
1414
| tst.js:7:15:7:18 | conn |
15+
| tst.js:8:5:8:19 | this.connection |
1516
| tst.js:11:5:11:19 | this.connection |
1617
| tst.js:16:10:16:49 | api.cha ... ction() |
1718
| tst.js:19:7:19:21 | getConnection() |
@@ -20,7 +21,9 @@ test_Connection
2021
| tst.js:48:7:48:21 | getConnection() |
2122
| tst.js:54:37:54:51 | getConnection() |
2223
| tst.js:57:14:57:48 | config. ... ction') |
24+
| tst.js:62:3:62:36 | MyAppli ... nection |
2325
| tst.js:62:40:62:79 | api.cha ... ction() |
26+
| tst.js:63:3:63:34 | MyAppli ... onflict |
2427
| tst.js:63:38:63:77 | api.cha ... ction() |
2528
| tst.js:67:14:67:47 | MyAppli ... nection |
2629
| tst.js:78:35:78:49 | getConnection() |
@@ -41,6 +44,7 @@ test_Connection
4144
| tst.js:118:12:118:26 | getConnection() |
4245
| tst.js:120:21:120:24 | conn |
4346
| tst.js:126:22:126:25 | conn |
47+
| tst_conflict.js:6:3:6:34 | MyAppli ... onflict |
4448
| tst_conflict.js:6:38:6:77 | api.cha ... ction() |
4549
test_DataCallback
4650
| client.js:3:28:3:34 | x => {} |

‎javascript/ql/test/library-tests/TypeTracking/PredicateStyle.expected

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ apiObject
1111
| tst_conflict.js:6:38:6:58 | api.cha ... hain2() |
1212
connection
1313
| type tracker with call steps | tst.js:7:15:7:18 | conn |
14+
| type tracker with call steps | tst.js:8:5:8:19 | this.connection |
1415
| type tracker with call steps | tst.js:11:5:11:19 | this.connection |
1516
| type tracker with call steps | tst.js:80:16:80:19 | conn |
1617
| type tracker with call steps | tst.js:84:22:84:22 | x |
@@ -30,7 +31,9 @@ connection
3031
| type tracker without call steps | tst.js:48:7:48:21 | getConnection() |
3132
| type tracker without call steps | tst.js:54:37:54:51 | getConnection() |
3233
| type tracker without call steps | tst.js:57:14:57:48 | config. ... ction') |
34+
| type tracker without call steps | tst.js:62:3:62:36 | MyAppli ... nection |
3335
| type tracker without call steps | tst.js:62:40:62:79 | api.cha ... ction() |
36+
| type tracker without call steps | tst.js:63:3:63:34 | MyAppli ... onflict |
3437
| type tracker without call steps | tst.js:63:38:63:77 | api.cha ... ction() |
3538
| type tracker without call steps | tst.js:67:14:67:47 | MyAppli ... nection |
3639
| type tracker without call steps | tst.js:78:35:78:49 | getConnection() |
@@ -43,6 +46,7 @@ connection
4346
| type tracker without call steps | tst.js:118:12:118:26 | getConnection() |
4447
| type tracker without call steps | tst.js:120:21:120:24 | conn |
4548
| type tracker without call steps | tst.js:126:22:126:25 | conn |
49+
| type tracker without call steps | tst_conflict.js:6:3:6:34 | MyAppli ... onflict |
4650
| type tracker without call steps | tst_conflict.js:6:38:6:77 | api.cha ... ction() |
4751
| type tracker without call steps with property conflict | tst.js:63:3:63:25 | MyAppli ... mespace |
4852
| type tracker without call steps with property conflict | tst_conflict.js:6:3:6:25 | MyAppli ... mespace |

‎javascript/ql/test/library-tests/frameworks/Express/tests.expected

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3227,10 +3227,13 @@ getRouteHandlerContainerStep
32273227
| src/route-collection.js:1:18:4:1 | {\\n a: ... (req)\\n} | src/route-collection.js:3:6:3:35 | (req, r ... og(req) | src/advanced-routehandler-registration.js:116:14:116:30 | importedRoutes[p] |
32283228
| src/route-collection.js:1:18:4:1 | {\\n a: ... (req)\\n} | src/route-collection.js:3:6:3:35 | (req, r ... og(req) | src/advanced-routehandler-registration.js:119:14:119:29 | importedRoutes.b |
32293229
dbUse
3230+
| src/middleware-flow.js:6:5:6:10 | req.db |
32303231
| src/middleware-flow.js:6:5:6:21 | req.db = new DB() |
32313232
| src/middleware-flow.js:6:14:6:21 | new DB() |
3233+
| src/middleware-flow.js:7:5:7:15 | req.deep.db |
32323234
| src/middleware-flow.js:7:5:7:26 | req.dee ... ew DB() |
32333235
| src/middleware-flow.js:7:19:7:26 | new DB() |
3236+
| src/middleware-flow.js:8:5:8:22 | req.deep.access.db |
32343237
| src/middleware-flow.js:8:5:8:33 | req.dee ... ew DB() |
32353238
| src/middleware-flow.js:8:26:8:33 | new DB() |
32363239
| src/middleware-flow.js:18:9:18:14 | req.db |

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

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -159,6 +159,11 @@ nodes
159159
| xss-through-dom.js:141:25:141:27 | src |
160160
| xss-through-dom.js:150:24:150:26 | src |
161161
| xss-through-dom.js:150:24:150:26 | src |
162+
| xss-through-dom.js:154:25:154:27 | msg |
163+
| xss-through-dom.js:155:27:155:29 | msg |
164+
| xss-through-dom.js:155:27:155:29 | msg |
165+
| xss-through-dom.js:159:34:159:52 | $("textarea").val() |
166+
| xss-through-dom.js:159:34:159:52 | $("textarea").val() |
162167
edges
163168
| forms.js:8:23:8:28 | values | forms.js:9:31:9:36 | values |
164169
| forms.js:8:23:8:28 | values | forms.js:9:31:9:36 | values |
@@ -263,6 +268,10 @@ edges
263268
| xss-through-dom.js:139:11:139:52 | src | xss-through-dom.js:150:24:150:26 | src |
264269
| xss-through-dom.js:139:17:139:52 | documen ... k").src | xss-through-dom.js:139:11:139:52 | src |
265270
| xss-through-dom.js:139:17:139:52 | documen ... k").src | xss-through-dom.js:139:11:139:52 | src |
271+
| xss-through-dom.js:154:25:154:27 | msg | xss-through-dom.js:155:27:155:29 | msg |
272+
| xss-through-dom.js:154:25:154:27 | msg | xss-through-dom.js:155:27:155:29 | msg |
273+
| xss-through-dom.js:159:34:159:52 | $("textarea").val() | xss-through-dom.js:154:25:154:27 | msg |
274+
| xss-through-dom.js:159:34:159:52 | $("textarea").val() | xss-through-dom.js:154:25:154:27 | msg |
266275
#select
267276
| forms.js:9:31:9:40 | values.foo | forms.js:8:23:8:28 | values | forms.js:9:31:9:40 | values.foo | $@ is reinterpreted as HTML without escaping meta-characters. | forms.js:8:23:8:28 | values | DOM text |
268277
| forms.js:12:31:12:40 | values.bar | forms.js:11:24:11:29 | values | forms.js:12:31:12:40 | values.bar | $@ is reinterpreted as HTML without escaping meta-characters. | forms.js:11:24:11:29 | values | DOM text |
@@ -307,3 +316,4 @@ edges
307316
| xss-through-dom.js:140:19:140:21 | src | xss-through-dom.js:139:17:139:52 | documen ... k").src | xss-through-dom.js:140:19:140:21 | src | $@ is reinterpreted as HTML without escaping meta-characters. | xss-through-dom.js:139:17:139:52 | documen ... k").src | DOM text |
308317
| xss-through-dom.js:141:25:141:27 | src | xss-through-dom.js:139:17:139:52 | documen ... k").src | xss-through-dom.js:141:25:141:27 | src | $@ is reinterpreted as HTML without escaping meta-characters. | xss-through-dom.js:139:17:139:52 | documen ... k").src | DOM text |
309318
| xss-through-dom.js:150:24:150:26 | src | xss-through-dom.js:139:17:139:52 | documen ... k").src | xss-through-dom.js:150:24:150:26 | src | $@ is reinterpreted as HTML without escaping meta-characters. | xss-through-dom.js:139:17:139:52 | documen ... k").src | DOM text |
319+
| xss-through-dom.js:155:27:155:29 | msg | xss-through-dom.js:159:34:159:52 | $("textarea").val() | xss-through-dom.js:155:27:155:29 | msg | $@ is reinterpreted as HTML without escaping meta-characters. | xss-through-dom.js:159:34:159:52 | $("textarea").val() | DOM text |

‎javascript/ql/test/query-tests/Security/CWE-079/XssThroughDom/xss-through-dom.js

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -148,4 +148,15 @@ const cashDom = require("cash-dom");
148148
cashDom("#id").html(DOMPurify ? DOMPurify.sanitize(src) : src); // OK
149149

150150
$("<a />", { html: src }).appendTo("#id"); // NOT OK
151+
152+
function foo() {
153+
window.VeryUniqueXssTestName = {
154+
send: function (msg) {
155+
$("#id").html(msg); // NOT OK
156+
},
157+
};
158+
159+
VeryUniqueXssTestName.send($("textarea").val());
160+
}
161+
foo()
151162
})();

0 commit comments

Comments
 (0)