refactor EventEmitter model to use the ::Range pattern

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

@@ -89,7 +89,7 @@ module Electron {
     /**
      * A model for the Main and Renderer process in an Electron app.
      */
-    abstract class Process extends EventEmitter::EventEmitter { }
+    abstract class Process extends EventEmitter::EventEmitterRange::Range { }
 
     /**
      * An instance of the Main process of an Electron app.
@@ -100,83 +100,78 @@ module Electron {
     }
 
     /**
-     * An instance of the renderer process of an Electron app. 
+     * An instance of the renderer process of an Electron app.
      */
     class RendererProcess extends Process {
       RendererProcess() { this = renderer() }
     }
-    
+
     /**
-     * The `sender` property of the event in an IPC event handler. 
+     * The `sender` property of the event in an IPC event handler.
      * This sender is used to send a response back from the main process to the renderer.
      */
     class ProcessSender extends Process {
       ProcessSender() {
-        exists(IPCSendRegistration reg | reg.getEmitter() instanceof MainProcess | 
-         this = reg.getABoundCallbackParameter(1, 0).getAPropertyRead("sender")
+        exists(IPCSendRegistration reg | reg.getEmitter() instanceof MainProcess |
+          this = reg.getABoundCallbackParameter(1, 0).getAPropertyRead("sender")
         )
       }
     }
-    
+
     /**
      * A registration of an Electron IPC event handler.
-     * Does mostly the same as an EventEmitter event handler, 
-     * except that values can be returned through the `event.returnValue` property. 
+     * Does mostly the same as an EventEmitter event handler,
+     * except that values can be returned through the `event.returnValue` property.
      */
-    class IPCSendRegistration extends EventEmitter::EventRegistration, DataFlow::MethodCallNode {
+    class IPCSendRegistration extends EventEmitter::EventRegistration::Range,
+      DataFlow::MethodCallNode {
       override Process emitter;
-      
-      IPCSendRegistration() {
-        this = emitter.ref().getAMethodCall("on")
-      }
-      
-      override string getChannel() {
-        this.getArgument(0).mayHaveStringValue(result)
-      }
-      
+
+      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) 
+        result = this.getABoundCallbackParameter(1, i + 1)
       }
-      
+
       override DataFlow::Node getAReturnedValue() {
         result = this.getABoundCallbackParameter(1, 0).getAPropertyWrite("returnValue").getRhs()
       }
-      
+
       override predicate canReturnTo(EventEmitter::EventDispatch dispatch) {
         dispatch.(DataFlow::InvokeNode).getCalleeName() = "sendSync"
       }
     }
-    
+
     /**
-     * A dispatch of an IPC event. 
+     * A dispatch of an IPC event.
      * An IPC event is sent from the Renderer to the Main process.
-     * And a value can be returned through the `returnValue` property of the event (first parameter in the callback). 
+     * And a value can be returned through the `returnValue` property of the event (first parameter in the callback).
      */
-    class IPCDispatch extends EventEmitter::EventDispatch, DataFlow::InvokeNode {
+    class IPCDispatch extends EventEmitter::EventDispatch::Range, DataFlow::InvokeNode {
       override Process emitter;
-      
+
       IPCDispatch() {
-        exists(string methodName | methodName = "sendSync" or methodName = "send" | 
+        exists(string methodName | methodName = "sendSync" or methodName = "send" |
           this = emitter.ref().getAMemberCall(methodName)
         )
       }
-      
-      override string getChannel() {
-        this.getArgument(0).mayHaveStringValue(result)
-      }
-      
+
+      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, 
-       * therefore these arguments start at 1. 
+       * Gets the `i`th dispatched argument to the event handler.
+       * The 0th parameter in the callback is a event generated by the IPC system,
+       * therefore these arguments start at 1.
        */
       override DataFlow::Node getDispatchedArgument(int i) {
-        i >= 1 and 
+        i >= 1 and
         result = getArgument(i)
       }
-      
+
       /**
-       * Holds if this dispatch can send an event to the given EventRegistration destination. 
+       * Holds if this dispatch can send an event to the given EventRegistration destination.
        */
       override predicate canSendTo(EventEmitter::EventRegistration destination) {
         this.getEmitter() instanceof RendererProcess and

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

@@ -18,125 +18,53 @@ module EventEmitter {
     result = "prependListener" or
     result = "prependOnceListener"
   }
-  
+
   /**
-   * An instance of the NodeJS EventEmitter class. 
-   * Extend this class to mark something as being an instance of the EventEmitter class. 
+   * An instance of the NodeJS EventEmitter class.
+   * Extend this class to mark something as being an instance of the EventEmitter class.
    */
-  abstract class EventEmitter extends DataFlow::Node {
+  final class EventEmitter extends DataFlow::Node {
+    EventEmitterRange::Range range;
+
+    EventEmitter() { this = range }
+
     /**
      * Get a method name that returns `this` on this type of emitter.
      */
-    string getAChainableMethod() { result = EventEmitter::chainableMethod() }
-
-    private DataFlow::SourceNode ref(DataFlow::TypeTracker t) {
-      t.start() and result = this
-      or
-      exists(DataFlow::TypeTracker t2, DataFlow::SourceNode pred | pred = ref(t2) |
-        result = pred.track(t2, t)
-        or
-        // invocation of a chainable method
-        exists(DataFlow::MethodCallNode mcn |
-          mcn = pred.getAMethodCall(this.getAChainableMethod()) and
-          // exclude getter versions
-          exists(mcn.getAnArgument()) and
-          result = mcn and
-          t = t2.continue()
-        )
-      )
-    }
+    string getAChainableMethod() { result = range.getAChainableMethod() }
 
     /**
-     * Get a reference through type-tracking to this EventEmitter. 
+     * Get a reference through type-tracking to this EventEmitter.
      * The type-tracking tracks through chainable methods.
      */
-    DataFlow::SourceNode ref() { result = ref(DataFlow::TypeTracker::end()) }
+    DataFlow::SourceNode ref() { result = range.ref() }
   }
 
-  /**
-   * A registration of an event handler on a particular EventEmitter.
-   */
-  abstract class EventRegistration extends DataFlow::Node {
-    EventEmitter emitter;
+  module EventEmitterRange {
+    abstract class Range extends DataFlow::Node {
+      string getAChainableMethod() { result = EventEmitter::chainableMethod() }
 
-    /** Gets the EventEmitter that the event handler is registered on. */
-    final EventEmitter getEmitter() {
-      result = emitter
+      private DataFlow::SourceNode ref(DataFlow::TypeTracker t) {
+        t.start() and result = this
+        or
+        exists(DataFlow::TypeTracker t2, DataFlow::SourceNode pred | pred = ref(t2) |
+          result = pred.track(t2, t)
+          or
+          // invocation of a chainable method
+          exists(DataFlow::MethodCallNode mcn |
+            mcn = pred.getAMethodCall(this.getAChainableMethod()) and
+            // exclude getter versions
+            exists(mcn.getAnArgument()) and
+            result = mcn and
+            t = t2.continue()
+          )
+        )
+      }
+
+      DataFlow::SourceNode ref() { result = ref(DataFlow::TypeTracker::end()) }
     }
 
-    /** Gets the name of the channel if possible. */
-    abstract string getChannel();
-
-    /** Gets the `i`th parameter in the event handler. */
-    abstract DataFlow::Node getEventHandlerParameter(int i);
-    
-    /**
-     * Gets a value that is returned by the event handler. 
-     * The default implementation is that no value can be returned.
-     */
-    DataFlow::Node getAReturnedValue() { none() }
-    
-    /**
-     * Holds if this event handler can return a value to the given `dispatch`. 
-     * The default implementation is that there exists no such dispatch. 
-     */
-    predicate canReturnTo(EventDispatch dispatch) { none() }
-  }
-
-  /**
-   * A dispatch of an event on an EventEmitter.
-   */
-  abstract class EventDispatch extends DataFlow::Node {
-    EventEmitter emitter;
-
-    /** Gets the emitter that the event dispatch happens on. */    
-    final EventEmitter getEmitter() {
-      result = emitter
-    }
-    
-    /** Gets the name of the channel if possible. */
-    abstract string getChannel();
-
-    /** Gets the `i`th argument that is send to the event handler. */
-    abstract DataFlow::Node getDispatchedArgument(int i);
-    
-    /**
-     * Holds if this event dispatch can send an event to the given even registration. 
-     * The default implementation is that the emitters of the dispatch and registration have to be equal.
-     */
-    predicate canSendTo(EventRegistration destination) { this.getEmitter() = destination.getEmitter() }
-  }
-  
-  /**
-   * A taint-step that models data-flow between event handlers and event dispatchers.
-   */
-  private class EventEmitterTaintStep extends DataFlow::AdditionalFlowStep {
-    EventRegistration reg;
-    EventDispatch dispatch;
-
-    EventEmitterTaintStep() {
-      this = dispatch and 
-      dispatch.canSendTo(reg) and 
-      reg.getChannel() = dispatch.getChannel()
-    }
-
-    override predicate step(DataFlow::Node pred, DataFlow::Node succ) {
-      exists(int i | i >= 0 | 
-        pred = dispatch.getDispatchedArgument(i) and
-        succ = reg.getEventHandlerParameter(i)
-      )
-      or
-      reg.canReturnTo(dispatch) and
-      pred = reg.getAReturnedValue() and
-      succ = dispatch
-    }
-  }
-
-  /**
-   * Concrete classes for modeling EventEmitter in NodeJS.
-   */
-  private module NodeJSEventEmitter {
-    private class NodeJSEventEmitter extends EventEmitter {
+    private class NodeJSEventEmitter extends Range {
       NodeJSEventEmitter() {
         exists(DataFlow::SourceNode clazz |
           clazz = DataFlow::moduleImport("events") or
@@ -146,25 +74,134 @@ module EventEmitter {
         )
       }
     }
+  }
 
-    private class EventEmitterRegistration extends EventRegistration, DataFlow::MethodCallNode {
-      EventEmitterRegistration() { this = emitter.ref().getAMethodCall(EventEmitter::on()) }
+  /**
+   * A registration of an event handler on a particular EventEmitter.
+   */
+  final class EventRegistration extends DataFlow::Node {
+    EventRegistration::Range range;
+
+    EventRegistration() { this = range }
+
+    /** Gets the EventEmitter that the event handler is registered on. */
+    final EventEmitter getEmitter() { result = range.getEmitter() }
+
+    /** Gets the name of the channel if possible. */
+    string getChannel() { result = range.getChannel() }
+
+    /** Gets the `i`th parameter in the event handler. */
+    DataFlow::Node getEventHandlerParameter(int i) { result = range.getEventHandlerParameter(i) }
+
+    /**
+     * Gets a value that is returned by the event handler.
+     * The default implementation is that no value can be returned.
+     */
+    DataFlow::Node getAReturnedValue() { result = range.getAReturnedValue() }
+
+    /**
+     * Holds if this event handler can return a value to the given `dispatch`.
+     * The default implementation is that there exists no such dispatch.
+     */
+    predicate canReturnTo(EventDispatch dispatch) { range.canReturnTo(dispatch) }
+  }
+
+  module EventRegistration {
+    abstract class Range extends DataFlow::Node {
+      EventEmitterRange::Range emitter;
+
+      final EventEmitter getEmitter() { result = emitter }
+
+      abstract string getChannel();
+
+      abstract DataFlow::Node getEventHandlerParameter(int 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 DataFlow::Node getEventHandlerParameter(int i) {
         result = this.(DataFlow::MethodCallNode).getABoundCallbackParameter(1, i)
-      }    
+      }
+    }
+  }
+
+  /**
+   * A dispatch of an event on an EventEmitter.
+   */
+  final class EventDispatch extends DataFlow::Node {
+    EventDispatch::Range range;
+
+    EventDispatch() { this = range }
+
+    /** Gets the emitter that the event dispatch happens on. */
+    EventEmitter getEmitter() { result = range.getEmitter() }
+
+    /** Gets the name of the channel if possible. */
+    string getChannel() { result = range.getChannel() }
+
+    /** Gets the `i`th argument that is send to the event handler. */
+    DataFlow::Node getDispatchedArgument(int i) { result = range.getDispatchedArgument(i) }
+
+    /**
+     * Holds if this event dispatch can send an event to the given even registration.
+     * The default implementation is that the emitters of the dispatch and registration have to be equal.
+     */
+    predicate canSendTo(EventRegistration destination) { range.canSendTo(destination) }
+  }
+
+  module EventDispatch {
+    abstract class Range extends DataFlow::Node {
+      EventEmitterRange::Range emitter;
+
+      final EventEmitter getEmitter() { result = emitter }
+
+      abstract string getChannel();
+
+      abstract DataFlow::Node getDispatchedArgument(int i);
+
+      predicate canSendTo(EventRegistration destination) {
+        this.getEmitter() = destination.getEmitter()
+      }
     }
 
-    private class EventEmitterDispatch extends EventDispatch, DataFlow::MethodCallNode {
-      EventEmitterDispatch() {
-        this = emitter.ref().getAMethodCall("emit")
-      }
+    private class NodeJSEventDispatch extends Range, DataFlow::MethodCallNode {
+      NodeJSEventDispatch() { this = emitter.ref().getAMethodCall("emit") }
 
       override string getChannel() { this.getArgument(0).mayHaveStringValue(result) }
 
       override DataFlow::Node getDispatchedArgument(int i) { result = this.getArgument(i + 1) }
     }
   }
+
+  /**
+   * A taint-step that models data-flow between event handlers and event dispatchers.
+   */
+  private class EventEmitterTaintStep extends DataFlow::AdditionalFlowStep {
+    EventRegistration reg;
+    EventDispatch dispatch;
+
+    EventEmitterTaintStep() {
+      this = dispatch and
+      dispatch.canSendTo(reg) and
+      reg.getChannel() = dispatch.getChannel()
+    }
+
+    override predicate step(DataFlow::Node pred, DataFlow::Node succ) {
+      exists(int i | i >= 0 |
+        pred = dispatch.getDispatchedArgument(i) and
+        succ = reg.getEventHandlerParameter(i)
+      )
+      or
+      reg.canReturnTo(dispatch) and
+      pred = reg.getAReturnedValue() and
+      succ = dispatch
+    }
+  }
 }