Skip to content

Commit 3158b84

Browse files
authored
Merge pull request #2705 from erik-krogh/CVE75
Approved by asgerf
2 parents 120b50f + cb16116 commit 3158b84

File tree

3 files changed

+133
-9
lines changed

3 files changed

+133
-9
lines changed

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

Lines changed: 105 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -906,9 +906,7 @@ module NodeJSLib {
906906
* An instance of an EventEmitter that is imported through the 'events' module.
907907
*/
908908
private class ImportedNodeJSEventEmitter extends NodeJSEventEmitter {
909-
ImportedNodeJSEventEmitter() {
910-
this = getAnEventEmitterImport().getAnInstantiation()
911-
}
909+
ImportedNodeJSEventEmitter() { this = getAnEventEmitterImport().getAnInstantiation() }
912910
}
913911

914912
/**
@@ -922,20 +920,33 @@ module NodeJSLib {
922920
}
923921

924922
/**
925-
* An instantiation of a class that extends EventEmitter.
926-
*
923+
* An instantiation of a class that extends EventEmitter.
924+
*
927925
* By extending `NodeJSEventEmitter' we get data-flow on the events passing through this EventEmitter.
928926
*/
929-
private class CustomEventEmitter extends NodeJSEventEmitter {
927+
class CustomEventEmitter extends NodeJSEventEmitter {
928+
EventEmitterSubClass clazz;
930929
CustomEventEmitter() {
931-
this = any(EventEmitterSubClass clazz).getAClassReference().getAnInstantiation()
930+
if exists(clazz.getAClassReference().getAnInstantiation()) then
931+
this = clazz.getAClassReference().getAnInstantiation()
932+
else
933+
// In case there are no explicit instantiations of the clazz, then we still want to track data flow between `this` nodes.
934+
// This cannot produce false flow as the `.ref()` method below is always used when creating event-registrations/event-dispatches.
935+
this = clazz
936+
}
937+
938+
override DataFlow::SourceNode ref() {
939+
result = NodeJSEventEmitter.super.ref() and not this = clazz
940+
or
941+
result = clazz.getAReceiverNode()
932942
}
933943
}
934944

935945
/**
936946
* A registration of an event handler on a NodeJS EventEmitter instance.
937947
*/
938-
private class NodeJSEventRegistration extends EventRegistration::DefaultEventRegistration, DataFlow::MethodCallNode {
948+
private class NodeJSEventRegistration extends EventRegistration::DefaultEventRegistration,
949+
DataFlow::MethodCallNode {
939950
override NodeJSEventEmitter emitter;
940951

941952
NodeJSEventRegistration() { this = emitter.ref().getAMethodCall(EventEmitter::on()) }
@@ -944,9 +955,94 @@ module NodeJSLib {
944955
/**
945956
* A dispatch of an event on a NodeJS EventEmitter instance.
946957
*/
947-
private class NodeJSEventDispatch extends EventDispatch::DefaultEventDispatch, DataFlow::MethodCallNode {
958+
private class NodeJSEventDispatch extends EventDispatch::DefaultEventDispatch,
959+
DataFlow::MethodCallNode {
948960
override NodeJSEventEmitter emitter;
949961

950962
NodeJSEventDispatch() { this = emitter.ref().getAMethodCall("emit") }
951963
}
964+
965+
/**
966+
* An instance of net.createServer(), which creates a new TCP/IPC server.
967+
*/
968+
private class NodeJSNetServer extends DataFlow::SourceNode {
969+
NodeJSNetServer() { this = DataFlow::moduleMember("net", "createServer").getAnInvocation() }
970+
971+
private DataFlow::SourceNode ref(DataFlow::TypeTracker t) {
972+
t.start() and result = this
973+
or
974+
exists(DataFlow::TypeTracker t2 | result = ref(t2).track(t2, t))
975+
}
976+
977+
/**
978+
* Gets a reference to this server.
979+
*/
980+
DataFlow::SourceNode ref() { result = ref(DataFlow::TypeTracker::end()) }
981+
}
982+
983+
/**
984+
* A connection opened on a NodeJS net server.
985+
*/
986+
private class NodeJSNetServerConnection extends EventEmitter::Range {
987+
NodeJSNetServer server;
988+
989+
NodeJSNetServerConnection() {
990+
exists(DataFlow::MethodCallNode call |
991+
call = server.ref().getAMethodCall("on") and
992+
call.getArgument(0).mayHaveStringValue("connection")
993+
|
994+
this = call.getCallback(1).getParameter(0)
995+
)
996+
}
997+
998+
DataFlow::SourceNode ref() { result = EventEmitter::trackEventEmitter(this) }
999+
}
1000+
1001+
/**
1002+
* A registration of an event handler on a NodeJS net server instance.
1003+
*/
1004+
private class NodeJSNetServerRegistration extends EventRegistration::DefaultEventRegistration,
1005+
DataFlow::MethodCallNode {
1006+
override NodeJSNetServerConnection emitter;
1007+
1008+
NodeJSNetServerRegistration() { this = emitter.ref().getAMethodCall(EventEmitter::on()) }
1009+
}
1010+
1011+
/**
1012+
* A data flow node representing data received from a client to a NodeJS net server, viewed as remote user input.
1013+
*/
1014+
private class NodeJSNetServerItemAsRemoteFlow extends RemoteFlowSource {
1015+
NodeJSNetServerRegistration reg;
1016+
1017+
NodeJSNetServerItemAsRemoteFlow() { this = reg.getReceivedItem(_) }
1018+
1019+
override string getSourceType() { result = "NodeJS server" }
1020+
}
1021+
1022+
/**
1023+
* An instantiation of the `respjs` library, which is an EventEmitter.
1024+
*/
1025+
private class RespJS extends NodeJSEventEmitter {
1026+
RespJS() {
1027+
this = DataFlow::moduleImport("respjs").getAnInstantiation()
1028+
}
1029+
}
1030+
1031+
/**
1032+
* A event dispatch that serializes the input data and emits the result on the "data" channel.
1033+
*/
1034+
private class RespWrite extends EventDispatch::DefaultEventDispatch,
1035+
DataFlow::MethodCallNode {
1036+
override RespJS emitter;
1037+
1038+
RespWrite() { this = emitter.ref().getAMethodCall("write") }
1039+
1040+
override string getChannel() {
1041+
result = "data"
1042+
}
1043+
1044+
override DataFlow::Node getSentItem(int i) {
1045+
i = 0 and result = this.getArgument(i)
1046+
}
1047+
}
9521048
}
Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
const EventEmitter = require("events");
2+
3+
class MyEmitter extends EventEmitter {
4+
foo() {
5+
this.emit("foo", "bar");
6+
this.on("foo", (data) => {});
7+
}
8+
}
9+
10+
class MySecondEmitter extends EventEmitter {
11+
foo() {
12+
this.emit("bar", "baz");
13+
this.on("bar", (data) => {});
14+
}
15+
}
16+
17+
var x = new MySecondEmitter();
18+
x.emit("bar", "baz2");
19+
20+
var y = new MySecondEmitter();
21+
y.emit("bar", "baz3");
22+
y.on("bar", (yData) => {})

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

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,9 @@
1+
| customEmitter.js:5:20:5:24 | "bar" | customEmitter.js:6:19:6:22 | data |
2+
| customEmitter.js:12:21:12:25 | "baz" | customEmitter.js:13:23:13:26 | data |
3+
| customEmitter.js:12:21:12:25 | "baz" | customEmitter.js:22:14:22:18 | yData |
4+
| customEmitter.js:18:15:18:20 | "baz2" | customEmitter.js:13:23:13:26 | data |
5+
| customEmitter.js:21:15:21:20 | "baz3" | customEmitter.js:13:23:13:26 | data |
6+
| customEmitter.js:21:15:21:20 | "baz3" | customEmitter.js:22:14:22:18 | yData |
17
| tst.js:9:23:9:33 | 'FirstData' | tst.js:6:40:6:44 | first |
28
| tst.js:10:24:10:35 | 'SecondData' | tst.js:7:32:7:37 | second |
39
| tst.js:15:24:15:39 | 'OtherFirstData' | tst.js:14:41:14:50 | otherFirst |

0 commit comments

Comments
 (0)