Skip to content

Commit 1c100bb

Browse files
committed
JS: recognize event emitters in nodejs client requests
1 parent bc41c26 commit 1c100bb

File tree

6 files changed

+57
-4
lines changed

6 files changed

+57
-4
lines changed

javascript/ql/src/semmle/javascript/frameworks/NodeJSLib.qll

+14-2
Original file line numberDiff line numberDiff line change
@@ -826,7 +826,7 @@ module NodeJSLib {
826826
/**
827827
* A model of a URL request in the Node.js `http` library.
828828
*/
829-
private class NodeHttpUrlRequest extends NodeJSClientRequest::Range {
829+
private class NodeHttpUrlRequest extends NodeJSClientRequest::Range, NodeJSEventEmitter {
830830
DataFlow::Node url;
831831

832832
NodeHttpUrlRequest() {
@@ -881,8 +881,11 @@ module NodeJSLib {
881881
exists(DataFlow::MethodCallNode mcn |
882882
clientRequest.getAMethodCall(EventEmitter::on()) = mcn and
883883
mcn.getArgument(0).mayHaveStringValue(handledEvent) and
884-
flowsTo(mcn.getArgument(1))
884+
this.flowsTo(mcn.getArgument(1))
885885
)
886+
or
887+
this.flowsTo(clientRequest.(DataFlow::CallNode).getLastArgument()) and
888+
handledEvent = "connection"
886889
}
887890

888891
/**
@@ -1060,6 +1063,15 @@ module NodeJSLib {
10601063
}
10611064
}
10621065

1066+
private class ClientRequestEventEmitter extends NodeJSEventEmitter {
1067+
ClientRequestEventEmitter() {
1068+
exists(ClientRequestHandler handler |
1069+
not handler.getAHandledEvent() = "error" and
1070+
this = handler.getAParameter()
1071+
)
1072+
}
1073+
}
1074+
10631075
/**
10641076
* A registration of an event handler on a NodeJS EventEmitter instance.
10651077
*/

javascript/ql/test/library-tests/frameworks/EventEmitter/test.expected

+20
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
taintSteps
12
| customEmitter.js:5:20:5:24 | "bar" | customEmitter.js:6:19:6:22 | data |
23
| customEmitter.js:12:21:12:25 | "baz" | customEmitter.js:13:23:13:26 | data |
34
| customEmitter.js:12:21:12:25 | "baz" | customEmitter.js:22:14:22:18 | yData |
@@ -15,3 +16,22 @@
1516
| tst.js:40:20:40:27 | "yabity" | tst.js:39:19:39:19 | x |
1617
| tst.js:46:28:46:38 | 'FirstData' | tst.js:43:45:43:49 | first |
1718
| tst.js:47:29:47:40 | 'SecondData' | tst.js:44:37:44:42 | second |
19+
eventEmitter
20+
| customEmitter.js:3:1:8:1 | class M ... );\\n\\t}\\n} |
21+
| customEmitter.js:17:9:17:29 | new MyS ... itter() |
22+
| customEmitter.js:20:9:20:29 | new MyS ... itter() |
23+
| tst2.js:6:12:6:42 | new Con ... , opts) |
24+
| tst2.js:16:10:16:24 | new Connector() |
25+
| tst2.js:22:12:24:2 | http.re ... {});\\n}) |
26+
| tst2.js:22:37:22:39 | res |
27+
| tst2.js:25:28:25:33 | socket |
28+
| tst2.js:29:12:31:2 | http.re ... {});\\n}) |
29+
| tst2.js:29:37:29:39 | res |
30+
| tst2.js:32:28:32:33 | socket |
31+
| tst.js:3:10:3:22 | new emitter() |
32+
| tst.js:13:11:13:23 | new emitter() |
33+
| tst.js:18:11:18:23 | new emitter() |
34+
| tst.js:24:11:24:23 | new emitter() |
35+
| tst.js:32:11:32:30 | new MyEventEmitter() |
36+
| tst.js:38:11:38:38 | new Ext ... itter() |
37+
| tst.js:42:15:42:32 | require('process') |

javascript/ql/test/library-tests/frameworks/EventEmitter/test.ql

+2
Original file line numberDiff line numberDiff line change
@@ -3,3 +3,5 @@ import javascript
33
query predicate taintSteps(DataFlow::Node pred, DataFlow::Node succ) {
44
exists(DataFlow::AdditionalFlowStep step | step.step(pred, succ))
55
}
6+
7+
query predicate eventEmitter(EventEmitter e) { any() }

javascript/ql/test/library-tests/frameworks/EventEmitter/tst2.js

+18-2
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
1-
var util = require('util');
2-
var EventEmitter = require('events').EventEmitter;
1+
var util = require("util");
2+
var EventEmitter = require("events").EventEmitter;
33

44
var Connector = function() {
55
if (!(this instanceof Connector)) {
@@ -16,3 +16,19 @@ Connector.prototype.foo = function() {};
1616
var em = new Connector();
1717
em.on("foo", bar => {});
1818
em.emit("foo", "bar");
19+
20+
var http = require("http");
21+
22+
let req1 = http.request(x, function(res) {
23+
res.on("data", function(data) {});
24+
});
25+
req1.on("socket", function(socket) {
26+
socket.on("data", function(data) {});
27+
});
28+
29+
let req2 = http.request(x, function(res) {
30+
res.on("error", function(error) {});
31+
});
32+
req2.on("socket", function(socket) {
33+
socket.on("error", function(error) {});
34+
});
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
var http = require("http");
2+
http.request(x, data => data.on("data", d => undefined));

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

+1
Original file line numberDiff line numberDiff line change
@@ -96,6 +96,7 @@ test_RouteSetup_getServer
9696
| src/indirect2.js:18:14:18:35 | http.cr ... er(get) | src/indirect2.js:18:14:18:35 | http.cr ... er(get) |
9797
| src/indirect.js:34:14:34:58 | http.cr ... dler()) | src/indirect.js:34:14:34:58 | http.cr ... dler()) |
9898
test_ClientRequest
99+
| http.js:2:1:2:56 | http.re ... fined)) |
99100
| src/http.js:18:1:18:30 | http.re ... uth" }) |
100101
| src/http.js:21:15:26:6 | http.re ... \\n }) |
101102
| src/http.js:27:16:27:73 | http.re ... POST'}) |

0 commit comments

Comments
 (0)