adjust type-tracking on custom EventEmitters

Author: erik-krogh

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

@@ -892,11 +892,6 @@ module NodeJSLib {
      * Get a Node that refers to a NodeJS EventEmitter instance.
      */
     DataFlow::SourceNode ref() { result = EventEmitter::trackEventEmitter(this) }
-    
-    /**
-      * Gets the node that will used for comparison when determining whether one EventEmitter can send an event to another EventEmitter.
-      */
-    DataFlow::SourceNode getBaseEmitter() {result = this}
   }
 
   /**
@@ -932,12 +927,18 @@ module NodeJSLib {
   class CustomEventEmitter extends NodeJSEventEmitter {
   	EventEmitterSubClass clazz;
     CustomEventEmitter() {
-      this = clazz.getAnInstanceReference()
+      if exists(clazz.getAClassReference().getAnInstantiation()) then
+        this = clazz.getAClassReference().getAnInstantiation()
+      else
+        // In case there are no explicit instantiations of the clazz, then we still want to track data flow between `this` nodes. 
+        // This cannot produce false flow as the `.ref()` method below is always used when creating event-registrations/event-dispatches. 
+        this = clazz
     }
 
-    // we define that events can flow between all instances of the same class, as we thereby support the `this` references in the class itself.
-    override DataFlow::SourceNode getBaseEmitter() {
-      result = clazz
+    override DataFlow::SourceNode ref() {
+      result = NodeJSEventEmitter.super.ref() and not this = clazz
+      or
+      result = clazz.getAReceiverNode()
     }
   }
 
@@ -959,8 +960,6 @@ module NodeJSLib {
     override NodeJSEventEmitter emitter;
 
     NodeJSEventDispatch() { this = emitter.ref().getAMethodCall("emit") }
-    
-    override EventRegistration::Range getAReceiver() { emitter.getBaseEmitter() = result.getEmitter().(NodeJSEventEmitter).getBaseEmitter() }
   }
 
   /**

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

@@ -0,0 +1,22 @@
+const EventEmitter = require("events");
+
+class MyEmitter extends EventEmitter {
+	foo() {
+		this.emit("foo", "bar");
+		this.on("foo", (data) => {});
+	}
+}
+
+class MySecondEmitter extends EventEmitter {
+	foo() {
+	  this.emit("bar", "baz");
+      this.on("bar", (data) => {});
+	}
+}
+
+var x = new MySecondEmitter();
+x.emit("bar", "baz2");
+
+var y = new MySecondEmitter();
+y.emit("bar", "baz3");
+y.on("bar", (yData) => {})

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

@@ -1,3 +1,9 @@
+| 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 |
+| customEmitter.js:18:15:18:20 | "baz2" | customEmitter.js:13:23:13:26 | data |
+| customEmitter.js:21:15:21:20 | "baz3" | customEmitter.js:13:23:13:26 | data |
+| customEmitter.js:21:15:21:20 | "baz3" | customEmitter.js:22:14:22:18 | yData |
 | tst.js:9:23:9:33 | 'FirstData' | tst.js:6:40:6:44 | first |
 | tst.js:10:24:10:35 | 'SecondData' | tst.js:7:32:7:37 | second |
 | tst.js:15:24:15:39 | 'OtherFirstData' | tst.js:14:41:14:50 | otherFirst |