refactored some duplicated methods into the abstract class, and specialized the type of emitter in NodeJS EventEmitter dispatch/registration

Author: erik-krogh

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

@@ -73,7 +73,7 @@ module Electron {
   /**
    * A reference to the `webContents` property of a browser object.
    */
-  class WebContents extends DataFlow::SourceNode {
+  class WebContents extends DataFlow::SourceNode, EventEmitter::EventEmitterRange::NodeJSEventEmitter {
     WebContents() { this.(DataFlow::PropRead).accesses(any(BrowserObject bo), "webContents") }
   }
 
@@ -96,7 +96,7 @@ module Electron {
      * Communication in an electron app generally happens from the renderer process to the main process.
      */
     class MainProcess extends Process {
-      MainProcess() { this = main() or this instanceof WebContents }
+      MainProcess() { this = main() }
     }
 
     /**
@@ -127,20 +127,14 @@ module Electron {
       DataFlow::MethodCallNode {
       override Process emitter;
 
-      IPCSendRegistration() { this = emitter.ref().getAMethodCall("on") }
-
-      override string getChannel() { this.getArgument(0).mayHaveStringValue(result) }
-
-      override DataFlow::Node getEventHandlerParameter(int i) {
-        result = this.getABoundCallbackParameter(1, i + 1)
-      }
+      IPCSendRegistration() { this = emitter.ref().getAMethodCall(EventEmitter::on()) }
 
       override DataFlow::Node getAReturnedValue() {
         result = this.getABoundCallbackParameter(1, 0).getAPropertyWrite("returnValue").getRhs()
       }
 
       override predicate canReturnTo(EventEmitter::EventDispatch dispatch) {
-        dispatch.(DataFlow::InvokeNode).getCalleeName() = "sendSync"
+        dispatch.getCalleeName() = "sendSync"
       }
     }
 
@@ -158,8 +152,6 @@ module Electron {
         )
       }
 
-      override string getChannel() { this.getArgument(0).mayHaveStringValue(result) }
-
       /**
        * Gets the `i`th dispatched argument to the event handler.
        * The 0th parameter in the callback is a event generated by the IPC system,

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

@@ -64,8 +64,10 @@ module EventEmitter {
       DataFlow::SourceNode ref() { result = ref(DataFlow::TypeTracker::end()) }
     }
 
-    private class NodeJSEventEmitter extends Range {
-      NodeJSEventEmitter() {
+    abstract class NodeJSEventEmitter extends Range {}
+
+    private class ImportedNodeJSEventEmitter extends NodeJSEventEmitter {
+      ImportedNodeJSEventEmitter() {
         exists(DataFlow::SourceNode clazz |
           clazz = DataFlow::moduleImport("events") or
           clazz = DataFlow::moduleMember("events", "EventEmitter")
@@ -107,35 +109,35 @@ module EventEmitter {
   }
 
   module EventRegistration {
-    abstract class Range extends DataFlow::Node {
+    abstract class Range extends DataFlow::CallNode {
       EventEmitterRange::Range emitter;
 
       final EventEmitter getEmitter() { result = emitter }
 
-      abstract string getChannel();
+      string getChannel() {
+        this.getArgument(0).mayHaveStringValue(result)
+      }
 
-      abstract DataFlow::Node getEventHandlerParameter(int i);
+      DataFlow::Node getEventHandlerParameter(int i) {
+        result = this.getABoundCallbackParameter(1, i)
+      }
 
       DataFlow::Node getAReturnedValue() { none() }
 
       predicate canReturnTo(EventDispatch dispatch) { none() }
     }
 
     private class NodeJSEventRegistration extends Range, DataFlow::MethodCallNode {
-      NodeJSEventRegistration() { this = emitter.ref().getAMethodCall(EventEmitter::on()) }
-
-      override string getChannel() { this.getArgument(0).mayHaveStringValue(result) }
+      override EventEmitterRange::NodeJSEventEmitter emitter;
 
-      override DataFlow::Node getEventHandlerParameter(int i) {
-        result = this.(DataFlow::MethodCallNode).getABoundCallbackParameter(1, i)
-      }
+      NodeJSEventRegistration() { this = emitter.ref().getAMethodCall(EventEmitter::on()) }
     }
   }
 
   /**
    * A dispatch of an event on an EventEmitter.
    */
-  final class EventDispatch extends DataFlow::Node {
+  final class EventDispatch extends DataFlow::CallNode {
     EventDispatch::Range range;
 
     EventDispatch() { this = range }
@@ -157,26 +159,28 @@ module EventEmitter {
   }
 
   module EventDispatch {
-    abstract class Range extends DataFlow::Node {
+    abstract class Range extends DataFlow::CallNode {
       EventEmitterRange::Range emitter;
 
       final EventEmitter getEmitter() { result = emitter }
 
-      abstract string getChannel();
+      string getChannel() {
+        this.getArgument(0).mayHaveStringValue(result)
+      }
 
-      abstract DataFlow::Node getDispatchedArgument(int i);
+      DataFlow::Node getDispatchedArgument(int i) {
+        result = this.getArgument(i + 1)
+      }
 
       predicate canSendTo(EventRegistration destination) {
         this.getEmitter() = destination.getEmitter()
       }
     }
 
     private class NodeJSEventDispatch extends Range, DataFlow::MethodCallNode {
-      NodeJSEventDispatch() { this = emitter.ref().getAMethodCall("emit") }
-
-      override string getChannel() { this.getArgument(0).mayHaveStringValue(result) }
+      override EventEmitterRange::NodeJSEventEmitter emitter;
 
-      override DataFlow::Node getDispatchedArgument(int i) { result = this.getArgument(i + 1) }
+      NodeJSEventDispatch() { this = emitter.ref().getAMethodCall("emit") }
     }
   }