Skip to content

Commit 25f4b76

Browse files
codeql-cicodeql-ci
authored
Merge pull request github#5045 from erik-krogh/bindRoute
Approved by asgerf
2 parents ad665b7 + c993f9a commit 25f4b76

File tree

7 files changed

+89
-27
lines changed

7 files changed

+89
-27
lines changed

‎javascript/ql/src/semmle/javascript/dataflow/DataFlow.qll

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1547,7 +1547,9 @@ module DataFlow {
15471547
*/
15481548
predicate localFieldStep(DataFlow::Node pred, DataFlow::Node succ) {
15491549
exists(ClassNode cls, string prop |
1550-
pred = cls.getAReceiverNode().getAPropertyWrite(prop).getRhs() and
1550+
pred = cls.getAReceiverNode().getAPropertyWrite(prop).getRhs() or
1551+
pred = cls.getInstanceMethod(prop)
1552+
|
15511553
succ = cls.getAReceiverNode().getAPropertyRead(prop)
15521554
)
15531555
}

‎javascript/ql/src/semmle/javascript/dataflow/Nodes.qll

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1008,6 +1008,31 @@ class ClassNode extends DataFlow::SourceNode {
10081008
result = getAnInstanceReference(DataFlow::TypeTracker::end())
10091009
}
10101010

1011+
/**
1012+
* Gets a property read that accesses the property `name` on an instance of this class.
1013+
*
1014+
* Concretely, this holds when the base is an instance of this class or a subclass thereof.
1015+
*/
1016+
pragma[nomagic]
1017+
DataFlow::PropRead getAnInstanceMemberAccess(string name, DataFlow::TypeTracker t) {
1018+
result = this.getAnInstanceReference(t.continue()).getAPropertyRead(name)
1019+
or
1020+
exists(DataFlow::ClassNode subclass |
1021+
result = subclass.getAnInstanceMemberAccess(name, t) and
1022+
not exists(subclass.getInstanceMember(name, _)) and
1023+
this = subclass.getADirectSuperClass()
1024+
)
1025+
}
1026+
1027+
/**
1028+
* Gets a property read that accesses the property `name` on an instance of this class.
1029+
*
1030+
* Concretely, this holds when the base is an instance of this class or a subclass thereof.
1031+
*/
1032+
DataFlow::PropRead getAnInstanceMemberAccess(string name) {
1033+
result = this.getAnInstanceMemberAccess(name, DataFlow::TypeTracker::end())
1034+
}
1035+
10111036
/**
10121037
* Gets an access to a static member of this class.
10131038
*/

‎javascript/ql/src/semmle/javascript/dataflow/internal/CallGraphs.qll

Lines changed: 15 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -54,27 +54,37 @@ module CallGraph {
5454
PreCallGraphStep::step(any(DataFlow::Node n | function.flowsTo(n)), result)
5555
or
5656
imprecision = 0 and
57+
result = callgraphStep(function, t)
58+
}
59+
60+
/**
61+
* Gets a reference to `function` type-tracked by `t`.
62+
*
63+
* This only includes steps that aren't included in ordinary type-tracking.
64+
* For example, this steps from a method definition to an access on an instance, but
65+
* does not step through access paths, as those are included in type-tracking already.
66+
*/
67+
cached
68+
DataFlow::SourceNode callgraphStep(DataFlow::FunctionNode function, DataFlow::TypeTracker t) {
5769
exists(DataFlow::ClassNode cls |
5870
exists(string name |
5971
function = cls.getInstanceMethod(name) and
60-
getAnInstanceMemberAccess(cls, name, t.continue()).flowsTo(result)
72+
cls.getAnInstanceMemberAccess(name, t.continue()) = result
6173
or
6274
function = cls.getStaticMethod(name) and
63-
cls.getAClassReference(t.continue()).getAPropertyRead(name).flowsTo(result)
75+
cls.getAClassReference(t.continue()).getAPropertyRead(name) = result
6476
)
6577
or
6678
function = cls.getConstructor() and
67-
cls.getAClassReference(t.continue()).flowsTo(result)
79+
cls.getAClassReference(t.continue()) = result
6880
)
6981
or
70-
imprecision = 0 and
7182
exists(DataFlow::FunctionNode outer |
7283
result = getAFunctionReference(outer, 0, t.continue()).getAnInvocation() and
7384
locallyReturnedFunction(outer, function)
7485
)
7586
}
7687

77-
cached
7888
private predicate locallyReturnedFunction(
7989
DataFlow::FunctionNode outer, DataFlow::FunctionNode inner
8090
) {
@@ -133,26 +143,6 @@ module CallGraph {
133143
)
134144
}
135145

136-
/**
137-
* Gets a property read that accesses the property `name` on an instance of this class.
138-
*
139-
* Concretely, this holds when the base is an instance of this class or a subclass thereof.
140-
*
141-
* This predicate may be overridden to customize the class hierarchy analysis.
142-
*/
143-
pragma[nomagic]
144-
private DataFlow::PropRead getAnInstanceMemberAccess(
145-
DataFlow::ClassNode cls, string name, DataFlow::TypeTracker t
146-
) {
147-
result = cls.getAnInstanceReference(t.continue()).getAPropertyRead(name)
148-
or
149-
exists(DataFlow::ClassNode subclass |
150-
result = getAnInstanceMemberAccess(subclass, name, t) and
151-
not exists(subclass.getInstanceMember(name, _)) and
152-
cls = subclass.getADirectSuperClass()
153-
)
154-
}
155-
156146
/**
157147
* Gets a possible callee of `node` with the given `imprecision`.
158148
*

‎javascript/ql/src/semmle/javascript/frameworks/HTTP.qll

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
import javascript
66
private import semmle.javascript.DynamicPropertyAccess
77
private import semmle.javascript.dataflow.internal.StepSummary
8+
private import semmle.javascript.dataflow.internal.CallGraphs
89

910
module HTTP {
1011
/**
@@ -299,6 +300,9 @@ module HTTP {
299300
exists(DataFlow::PartialInvokeNode call |
300301
succ = call.getBoundFunction(any(DataFlow::Node n | pred.flowsTo(n)), 0)
301302
)
303+
or
304+
// references to class methods
305+
succ = CallGraph::callgraphStep(pred, DataFlow::TypeTracker::end())
302306
}
303307

304308
/**

‎javascript/ql/test/library-tests/TypeTracking/class.js

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
class Foo {
2+
foo() {} // name: foo
3+
4+
bar() {
5+
this.foo; // track: foo
6+
}
7+
}

‎javascript/ql/test/library-tests/frameworks/NodeJSLib/createServer.js

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,4 +12,21 @@ const tls = require('tls');
1212

1313
const server = (isSecure ? tls : net).createServer(options, (socket) => {
1414
socket.on("data", (data) => {})
15-
});
15+
});
16+
17+
18+
const http = require("http");
19+
20+
(function () {
21+
function MyApp(data) {this.data = data};
22+
MyApp.prototype.getRequestHandler = function () {
23+
return this.handleRequest.bind(this)
24+
}
25+
MyApp.prototype.handleRequest = function (req, res) {
26+
res.end(this.data);
27+
}
28+
29+
var app = new MyApp("data");
30+
31+
const srv = http.createServer(app.getRequestHandler());
32+
})();

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

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ test_isCreateServer
22
| createServer.js:2:1:2:42 | https.c ... es) {}) |
33
| createServer.js:3:1:3:45 | https.c ... es) {}) |
44
| createServer.js:4:1:4:47 | require ... => {}) |
5+
| createServer.js:31:17:31:58 | http.cr ... dler()) |
56
| src/http.js:4:14:10:2 | http.cr ... foo;\\n}) |
67
| src/http.js:12:1:16:2 | http.cr ... r");\\n}) |
78
| src/http.js:57:1:57:31 | http.cr ... dler()) |
@@ -42,6 +43,8 @@ test_ResponseExpr
4243
| createServer.js:2:35:2:37 | res | createServer.js:2:20:2:41 | functio ... res) {} |
4344
| createServer.js:3:38:3:40 | res | createServer.js:3:23:3:44 | functio ... res) {} |
4445
| createServer.js:4:37:4:39 | res | createServer.js:4:31:4:46 | (req, res) => {} |
46+
| createServer.js:25:52:25:54 | res | createServer.js:25:37:27:5 | functio ... ;\\n } |
47+
| createServer.js:26:9:26:11 | res | createServer.js:25:37:27:5 | functio ... ;\\n } |
4548
| src/http.js:4:46:4:48 | res | src/http.js:4:32:10:1 | functio ... .foo;\\n} |
4649
| src/http.js:7:3:7:5 | res | src/http.js:4:32:10:1 | functio ... .foo;\\n} |
4750
| src/http.js:12:33:12:35 | res | src/http.js:12:19:16:1 | functio ... ar");\\n} |
@@ -87,6 +90,7 @@ test_RouteSetup_getServer
8790
| createServer.js:2:1:2:42 | https.c ... es) {}) | createServer.js:2:1:2:42 | https.c ... es) {}) |
8891
| createServer.js:3:1:3:45 | https.c ... es) {}) | createServer.js:3:1:3:45 | https.c ... es) {}) |
8992
| createServer.js:4:1:4:47 | require ... => {}) | createServer.js:4:1:4:47 | require ... => {}) |
93+
| createServer.js:31:17:31:58 | http.cr ... dler()) | createServer.js:31:17:31:58 | http.cr ... dler()) |
9094
| src/http.js:4:14:10:2 | http.cr ... foo;\\n}) | src/http.js:4:14:10:2 | http.cr ... foo;\\n}) |
9195
| src/http.js:12:1:16:2 | http.cr ... r");\\n}) | src/http.js:12:1:16:2 | http.cr ... r");\\n}) |
9296
| src/http.js:57:1:57:31 | http.cr ... dler()) | src/http.js:57:1:57:31 | http.cr ... dler()) |
@@ -114,6 +118,7 @@ test_ServerDefinition
114118
| createServer.js:2:1:2:42 | https.c ... es) {}) |
115119
| createServer.js:3:1:3:45 | https.c ... es) {}) |
116120
| createServer.js:4:1:4:47 | require ... => {}) |
121+
| createServer.js:31:17:31:58 | http.cr ... dler()) |
117122
| src/http.js:4:14:10:2 | http.cr ... foo;\\n}) |
118123
| src/http.js:12:1:16:2 | http.cr ... r");\\n}) |
119124
| src/http.js:57:1:57:31 | http.cr ... dler()) |
@@ -139,6 +144,8 @@ test_RouteHandler_getAResponseExpr
139144
| createServer.js:2:20:2:41 | functio ... res) {} | createServer.js:2:35:2:37 | res |
140145
| createServer.js:3:23:3:44 | functio ... res) {} | createServer.js:3:38:3:40 | res |
141146
| createServer.js:4:31:4:46 | (req, res) => {} | createServer.js:4:37:4:39 | res |
147+
| createServer.js:25:37:27:5 | functio ... ;\\n } | createServer.js:25:52:25:54 | res |
148+
| createServer.js:25:37:27:5 | functio ... ;\\n } | createServer.js:26:9:26:11 | res |
142149
| src/http.js:4:32:10:1 | functio ... .foo;\\n} | src/http.js:4:46:4:48 | res |
143150
| src/http.js:4:32:10:1 | functio ... .foo;\\n} | src/http.js:7:3:7:5 | res |
144151
| src/http.js:12:19:16:1 | functio ... ar");\\n} | src/http.js:12:33:12:35 | res |
@@ -177,6 +184,7 @@ test_ServerDefinition_getARouteHandler
177184
| createServer.js:2:1:2:42 | https.c ... es) {}) | createServer.js:2:20:2:41 | functio ... res) {} |
178185
| createServer.js:3:1:3:45 | https.c ... es) {}) | createServer.js:3:23:3:44 | functio ... res) {} |
179186
| createServer.js:4:1:4:47 | require ... => {}) | createServer.js:4:31:4:46 | (req, res) => {} |
187+
| createServer.js:31:17:31:58 | http.cr ... dler()) | createServer.js:25:37:27:5 | functio ... ;\\n } |
180188
| src/http.js:4:14:10:2 | http.cr ... foo;\\n}) | src/http.js:4:32:10:1 | functio ... .foo;\\n} |
181189
| src/http.js:12:1:16:2 | http.cr ... r");\\n}) | src/http.js:12:19:16:1 | functio ... ar");\\n} |
182190
| src/http.js:57:1:57:31 | http.cr ... dler()) | src/http.js:55:12:55:30 | function(req,res){} |
@@ -192,6 +200,7 @@ test_ServerDefinition_getARouteHandler
192200
| src/indirect.js:34:14:34:58 | http.cr ... dler()) | src/indirect.js:25:24:27:3 | (req, r ... ");\\n } |
193201
| src/indirect.js:34:14:34:58 | http.cr ... dler()) | src/indirect.js:28:15:30:3 | functio ... ");\\n } |
194202
test_ResponseSendArgument
203+
| createServer.js:26:17:26:25 | this.data | createServer.js:25:37:27:5 | functio ... ;\\n } |
195204
| src/http.js:14:13:14:17 | "foo" | src/http.js:12:19:16:1 | functio ... ar");\\n} |
196205
| src/http.js:15:11:15:15 | "bar" | src/http.js:12:19:16:1 | functio ... ar");\\n} |
197206
| src/http.js:64:11:64:16 | "bar2" | src/http.js:62:19:65:1 | functio ... r2");\\n} |
@@ -204,6 +213,11 @@ test_RouteSetup_getARouteHandler
204213
| createServer.js:2:1:2:42 | https.c ... es) {}) | createServer.js:2:20:2:41 | functio ... res) {} |
205214
| createServer.js:3:1:3:45 | https.c ... es) {}) | createServer.js:3:23:3:44 | functio ... res) {} |
206215
| createServer.js:4:1:4:47 | require ... => {}) | createServer.js:4:31:4:46 | (req, res) => {} |
216+
| createServer.js:31:17:31:58 | http.cr ... dler()) | createServer.js:22:41:24:5 | return of anonymous function |
217+
| createServer.js:31:17:31:58 | http.cr ... dler()) | createServer.js:23:16:23:33 | this.handleRequest |
218+
| createServer.js:31:17:31:58 | http.cr ... dler()) | createServer.js:23:16:23:44 | this.ha ... d(this) |
219+
| createServer.js:31:17:31:58 | http.cr ... dler()) | createServer.js:25:37:27:5 | functio ... ;\\n } |
220+
| createServer.js:31:17:31:58 | http.cr ... dler()) | createServer.js:31:35:31:57 | app.get ... ndler() |
207221
| src/http.js:4:14:10:2 | http.cr ... foo;\\n}) | src/http.js:4:32:10:1 | functio ... .foo;\\n} |
208222
| src/http.js:12:1:16:2 | http.cr ... r");\\n}) | src/http.js:12:19:16:1 | functio ... ar");\\n} |
209223
| src/http.js:57:1:57:31 | http.cr ... dler()) | src/http.js:54:1:56:1 | return of function getHandler |
@@ -251,6 +265,7 @@ test_RouteHandler
251265
| createServer.js:2:20:2:41 | functio ... res) {} | createServer.js:2:1:2:42 | https.c ... es) {}) |
252266
| createServer.js:3:23:3:44 | functio ... res) {} | createServer.js:3:1:3:45 | https.c ... es) {}) |
253267
| createServer.js:4:31:4:46 | (req, res) => {} | createServer.js:4:1:4:47 | require ... => {}) |
268+
| createServer.js:25:37:27:5 | functio ... ;\\n } | createServer.js:31:17:31:58 | http.cr ... dler()) |
254269
| src/http.js:4:32:10:1 | functio ... .foo;\\n} | src/http.js:4:14:10:2 | http.cr ... foo;\\n}) |
255270
| src/http.js:12:19:16:1 | functio ... ar");\\n} | src/http.js:12:1:16:2 | http.cr ... r");\\n}) |
256271
| src/http.js:55:12:55:30 | function(req,res){} | src/http.js:57:1:57:31 | http.cr ... dler()) |
@@ -269,6 +284,7 @@ test_RequestExpr
269284
| createServer.js:2:30:2:32 | req | createServer.js:2:20:2:41 | functio ... res) {} |
270285
| createServer.js:3:33:3:35 | req | createServer.js:3:23:3:44 | functio ... res) {} |
271286
| createServer.js:4:32:4:34 | req | createServer.js:4:31:4:46 | (req, res) => {} |
287+
| createServer.js:25:47:25:49 | req | createServer.js:25:37:27:5 | functio ... ;\\n } |
272288
| src/http.js:4:41:4:43 | req | src/http.js:4:32:10:1 | functio ... .foo;\\n} |
273289
| src/http.js:6:26:6:28 | req | src/http.js:4:32:10:1 | functio ... .foo;\\n} |
274290
| src/http.js:8:3:8:5 | req | src/http.js:4:32:10:1 | functio ... .foo;\\n} |
@@ -310,6 +326,7 @@ test_RouteHandler_getARequestExpr
310326
| createServer.js:2:20:2:41 | functio ... res) {} | createServer.js:2:30:2:32 | req |
311327
| createServer.js:3:23:3:44 | functio ... res) {} | createServer.js:3:33:3:35 | req |
312328
| createServer.js:4:31:4:46 | (req, res) => {} | createServer.js:4:32:4:34 | req |
329+
| createServer.js:25:37:27:5 | functio ... ;\\n } | createServer.js:25:47:25:49 | req |
313330
| src/http.js:4:32:10:1 | functio ... .foo;\\n} | src/http.js:4:41:4:43 | req |
314331
| src/http.js:4:32:10:1 | functio ... .foo;\\n} | src/http.js:6:26:6:28 | req |
315332
| src/http.js:4:32:10:1 | functio ... .foo;\\n} | src/http.js:8:3:8:5 | req |

0 commit comments

Comments
 (0)