Skip to content

JS: recognize event emitters in nodejs client requests #4996

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
codeql-ci merged 2 commits into from
Jan 21, 2021
Merged

JS: recognize event emitters in nodejs client requests #4996

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
1c100bb
JS: recognize event emitters in nodejs client requests
esbena Jan 21, 2021
cb25f2a
JS: add docstring with source examples
esbena Jan 21, 2021
File filter

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 14 additions & 2 deletions javascript/ql/src/semmle/javascript/frameworks/NodeJSLib.qll
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -826,7 +826,7 @@ module NodeJSLib {
/**
* A model of a URL request in the Node.js `http` library.
*/
private class NodeHttpUrlRequest extends NodeJSClientRequest::Range {
private class NodeHttpUrlRequest extends NodeJSClientRequest::Range, NodeJSEventEmitter {
DataFlow::Node url;

NodeHttpUrlRequest() {
Expand Down Expand Up @@ -881,8 +881,11 @@ module NodeJSLib {
exists(DataFlow::MethodCallNode mcn |
clientRequest.getAMethodCall(EventEmitter::on()) = mcn and
mcn.getArgument(0).mayHaveStringValue(handledEvent) and
flowsTo(mcn.getArgument(1))
this.flowsTo(mcn.getArgument(1))
)
or
this.flowsTo(clientRequest.(DataFlow::CallNode).getLastArgument()) and
handledEvent = "connection"
}

/**
Expand Down Expand Up @@ -1060,6 +1063,15 @@ module NodeJSLib {
}
}

private class ClientRequestEventEmitter extends NodeJSEventEmitter {
ClientRequestEventEmitter() {
exists(ClientRequestHandler handler |
not handler.getAHandledEvent() = "error" and
this = handler.getAParameter()
)
}
}

/**
* A registration of an event handler on a NodeJS EventEmitter instance.
*/
Expand Down
20 changes: 20 additions & 0 deletions javascript/ql/test/library-tests/frameworks/EventEmitter/test.expected
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
taintSteps
| customEmitter.js:5:20:5:24 | "bar" | customEmitter.js:6:19:6:22 | data |
| customEmitter.js:12:21:12:25 | "baz" | customEmitter.js:13:23:13:26 | data |
| customEmitter.js:12:21:12:25 | "baz" | customEmitter.js:22:14:22:18 | yData |
Expand All @@ -15,3 +16,22 @@
| tst.js:40:20:40:27 | "yabity" | tst.js:39:19:39:19 | x |
| tst.js:46:28:46:38 | 'FirstData' | tst.js:43:45:43:49 | first |
| tst.js:47:29:47:40 | 'SecondData' | tst.js:44:37:44:42 | second |
eventEmitter
| customEmitter.js:3:1:8:1 | class M ... );\\n\\t}\\n} |
| customEmitter.js:17:9:17:29 | new MyS ... itter() |
| customEmitter.js:20:9:20:29 | new MyS ... itter() |
| tst2.js:6:12:6:42 | new Con ... , opts) |
| tst2.js:16:10:16:24 | new Connector() |
| tst2.js:22:12:24:2 | http.re ... {});\\n}) |
| tst2.js:22:37:22:39 | res |
| tst2.js:25:28:25:33 | socket |
| tst2.js:29:12:31:2 | http.re ... {});\\n}) |
| tst2.js:29:37:29:39 | res |
| tst2.js:32:28:32:33 | socket |
| tst.js:3:10:3:22 | new emitter() |
| tst.js:13:11:13:23 | new emitter() |
| tst.js:18:11:18:23 | new emitter() |
| tst.js:24:11:24:23 | new emitter() |
| tst.js:32:11:32:30 | new MyEventEmitter() |
| tst.js:38:11:38:38 | new Ext ... itter() |
| tst.js:42:15:42:32 | require('process') |
2 changes: 2 additions & 0 deletions javascript/ql/test/library-tests/frameworks/EventEmitter/test.ql
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,3 +3,5 @@ import javascript
query predicate taintSteps(DataFlow::Node pred, DataFlow::Node succ) {
exists(DataFlow::AdditionalFlowStep step | step.step(pred, succ))
}

query predicate eventEmitter(EventEmitter e) { any() }
20 changes: 18 additions & 2 deletions javascript/ql/test/library-tests/frameworks/EventEmitter/tst2.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
var util = require('util');
var EventEmitter = require('events').EventEmitter;
var util = require("util");
var EventEmitter = require("events").EventEmitter;

var Connector = function() {
if (!(this instanceof Connector)) {
Expand All @@ -16,3 +16,19 @@ Connector.prototype.foo = function() {};
var em = new Connector();
em.on("foo", bar => {});
em.emit("foo", "bar");

var http = require("http");

let req1 = http.request(x, function(res) {
res.on("data", function(data) {});
});
req1.on("socket", function(socket) {
socket.on("data", function(data) {});
});

let req2 = http.request(x, function(res) {
res.on("error", function(error) {});
});
req2.on("socket", function(socket) {
socket.on("error", function(error) {});
});
2 changes: 2 additions & 0 deletions javascript/ql/test/library-tests/frameworks/NodeJSLib/http.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
var http = require("http");
http.request(x, data => data.on("data", d => undefined));
1 change: 1 addition & 0 deletions javascript/ql/test/library-tests/frameworks/NodeJSLib/tests.expected
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,7 @@ test_RouteSetup_getServer
| src/indirect2.js:18:14:18:35 | http.cr ... er(get) | src/indirect2.js:18:14:18:35 | http.cr ... er(get) |
| src/indirect.js:34:14:34:58 | http.cr ... dler()) | src/indirect.js:34:14:34:58 | http.cr ... dler()) |
test_ClientRequest
| http.js:2:1:2:56 | http.re ... fined)) |
| src/http.js:18:1:18:30 | http.re ... uth" }) |
| src/http.js:21:15:26:6 | http.re ... \\n }) |
| src/http.js:27:16:27:73 | http.re ... POST'}) |
Expand Down