add model for EventEmitter in NodeJS, and base the Electron::IPC model on top of the new EventEmitter model

javascript/ql/src/javascript.qll

@@ -73,6 +73,7 @@ import semmle.javascript.frameworks.Credentials
 import semmle.javascript.frameworks.CryptoLibraries
 import semmle.javascript.frameworks.DigitalOcean
 import semmle.javascript.frameworks.Electron
+import semmle.javascript.frameworks.EventEmitter
 import semmle.javascript.frameworks.Files
 import semmle.javascript.frameworks.Firebase
 import semmle.javascript.frameworks.jQuery

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

@@ -79,181 +79,108 @@ module Electron {
 
   /**
    * Provides classes and predicates for modelling Electron inter-process communication (IPC).
+   * The Electron IPC are EventEmitters, but they also expose a number of methods on top of the standard EventEmitter.
    */
   private module IPC {
-    class Process extends string {
-      Process() { this = "main" or this = "renderer" }
+    DataFlow::SourceNode main() { result = DataFlow::moduleMember("electron", "ipcMain") }
 
-      DataFlow::SourceNode getAnImport() {
-        this = Process::main() and result = DataFlow::moduleMember("electron", "ipcMain")
-        or
-        this = Process::renderer() and result = DataFlow::moduleMember("electron", "ipcRenderer")
+    DataFlow::SourceNode renderer() { result = DataFlow::moduleMember("electron", "ipcRenderer") }
+
+    /**
+     * A model for the Main and Renderer process in an Electron app.
+     */
+    abstract class Process extends EventEmitter::EventEmitter { }
+
+    /**
+     * An instance of the Main process of an Electron app.
+     * 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 }
+    }
+
+    /**
+     * 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. 
+     * 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")	
+      	)
       }
     }
-
-    module Process {
-      Process main() { result = "main" }
-
-      Process renderer() { result = "renderer" }
-    }
-
+    
     /**
-     * An IPC callback.
+     * 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. 
      */
-    class Callback extends DataFlow::FunctionNode {
-      DataFlow::Node channel;
-      Process process;
-
-      Callback() {
-        exists(DataFlow::MethodCallNode mc |
-          mc = process.getAnImport().getAMemberCall("on") and
-          this = mc.getCallback(1) and
-          channel = mc.getArgument(0)
-        )
+    class IPCSendRegistration extends EventEmitter::EventRegistration, DataFlow::MethodCallNode {
+      override Process emitter;
+      
+      IPCSendRegistration() {
+      	this = emitter.ref().getAMethodCall("on")
       }
-
-      /** Gets the process on which this callback is executed. */
-      Process getProcess() { result = process }
-
-      /** Gets the name of the channel the callback is listening on. */
-      string getChannelName() { result = channel.getStringValue() }
-
-      /** Gets the data flow node containing the message received by the callback. */
-      DataFlow::Node getMessage() { result = getParameter(1) }
-    }
-
-    /**
-     * An IPC message.
-     */
-    abstract class Message extends DataFlow::Node {
-      /** Gets the process that sends this message. */
-      abstract Process getProcess();
-
-      /** Gets the name of the channel this message is sent on. */
-      abstract string getChannelName();
-    }
-
-    /**
-     * An IPC message sent directly from a process.
-     */
-    class DirectMessage extends Message {
-      DataFlow::MethodCallNode mc;
-      Process process;
-      DataFlow::Node channel;
-      boolean isSync;
-
-      DirectMessage() {
-        exists(string send |
-          send = "send" and isSync = false
-          or
-          send = "sendSync" and isSync = true
-        |
-          mc = process.getAnImport().getAMemberCall(send) and
-          this = mc.getArgument(1) and
-          channel = mc.getArgument(0)
-        )
+      
+      override string getChannel() {
+      	this.getArgument(0).mayHaveStringValue(result)
       }
-
-      override Process getProcess() { result = process }
-
-      override string getChannelName() { result = channel.getStringValue() }
-    }
-
-    /**
-     * A synchronous IPC message sent directly from a process.
-     */
-    class SyncDirectMessage extends DirectMessage {
-      SyncDirectMessage() { isSync = true }
-
-      /** Gets the data flow node holding the reply to the message. */
-      DataFlow::Node getReply() { result = mc }
-    }
-
-    /**
-     * An asynchronous IPC reply sent from within an IPC callback.
-     */
-    class AsyncReplyMessage extends Message {
-      Callback callback;
-      DataFlow::Node channel;
-
-      AsyncReplyMessage() {
-        exists(DataFlow::MethodCallNode mc |
-          mc = callback.getParameter(0).getAPropertyRead("sender").getAMemberCall("send") and
-          this = mc.getArgument(1) and
-          channel = mc.getArgument(0)
-        )
+      
+      override DataFlow::Node getCallbackParameter(int i) {
+      	 result = this.getABoundCallbackParameter(1, i + 1) 
       }
-
-      override Process getProcess() { result = callback.getProcess() }
-
-      override string getChannelName() { result = channel.getStringValue() }
-    }
-
-    /**
-     * A synchronous IPC reply sent from within an IPC callback.
-     */
-    class SyncReplyMessage extends Message {
-      Callback callback;
-
-      SyncReplyMessage() {
-        this = callback.getParameter(0).getAPropertyWrite("returnValue").getRhs()
+      
+      override DataFlow::Node getAReturnedValue(EventEmitter::EventDispatch dispatch) {
+        dispatch.(DataFlow::InvokeNode).getCalleeName() = "sendSync" and
+        result = this.getABoundCallbackParameter(1, 0).getAPropertyWrite("returnValue").getRhs()
       }
-
-      override Process getProcess() { result = callback.getProcess() }
-
-      override string getChannelName() { result = callback.getChannelName() }
     }
-
+    
     /**
-     * An asynchronous Electron IPC message sent from the main process via a `webContents` object.
+     * 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). 
      */
-    class WebContentsMessage extends Message {
-      DataFlow::Node channel;
-
-      WebContentsMessage() {
-        exists(WebContents wc, DataFlow::MethodCallNode mc |
-          wc.flowsTo(mc.getReceiver()) and
-          this = mc.getArgument(1) and
-          channel = mc.getArgument(0) and
-          mc.getCalleeName() = "send"
-        )
+    class IPCDispatch extends EventEmitter::EventDispatch, DataFlow::InvokeNode {
+	  override Process emitter;
+	  
+      IPCDispatch() {
+      	exists(string methodName | methodName = "sendSync" or methodName = "send" | 
+      	  this = emitter.ref().getAMemberCall(methodName)
+      	)
       }
-
-      override Process getProcess() { result = Process::main() }
-
-      override string getChannelName() { result = channel.getStringValue() }
-    }
-
-    /**
-     * Holds if `pred` flows to `succ` via Electron IPC.
-     */
-    private predicate ipcFlowStep(DataFlow::Node pred, DataFlow::Node succ) {
-      // match a message sent from one process with a callback parameter in the other process
-      exists(Callback callback, Message msg |
-        callback.getChannelName() = msg.getChannelName() and
-        callback.getProcess() != msg.getProcess() and
-        pred = msg and
-        succ = callback.getMessage()
-      )
-      or
-      // match a synchronous reply sent from one process with a `sendSync` call in the other process
-      exists(SyncDirectMessage sendSync, SyncReplyMessage msg |
-        sendSync.getChannelName() = msg.getChannelName() and
-        sendSync.getProcess() != msg.getProcess() and
-        pred = msg and
-        succ = sendSync.getReply()
-      )
-    }
-
-    /**
-     * An additional flow step  via an Electron IPC message.
-     */
-    private class IPCAdditionalFlowStep extends DataFlow::AdditionalFlowStep {
-      IPCAdditionalFlowStep() { ipcFlowStep(this, _) }
-
-      override predicate step(DataFlow::Node pred, DataFlow::Node succ) {
-        pred = this and
-        ipcFlowStep(pred, succ)
+      
+      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. 
+       */
+      override DataFlow::Node getDispatchedArgument(int i) {
+      	i >= 1 and 
+      	result = getArgument(i)
+      }
+      
+      /**
+       * Holds if this dispatch can send an event to the given EventRegistration destination. 
+       */
+      override predicate canSendTo(EventEmitter::EventRegistration destination) {
+      	this.getEmitter() instanceof RendererProcess and
+      	destination.getEmitter() instanceof MainProcess
+      	or
+      	this.getEmitter() instanceof ProcessSender and
+      	destination.getEmitter() instanceof RendererProcess
       }
     }
   }

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

@@ -0,0 +1,167 @@
+import javascript
+
+module EventEmitter {
+  /** Gets the name of a method on `EventEmitter` that returns `this`. */
+  string chainableMethod() {
+    result = "off" or
+    result = "removeAllListeners" or
+    result = "removeListener" or
+    result = "setMaxListeners" or
+    result = on()
+  }
+
+  /** Gets the name of a method on `EventEmitter` that registers an event handler. */
+  string on() {
+    result = "addListener" or
+    result = "on" or
+    result = "once" or
+    result = "prependListener" or
+    result = "prependOnceListener"
+  }
+  
+  /**
+   * An instanceof of the NodeJS EventEmitter class. 
+   * Extend this class to mark something as being instanceof the EventEmitter class. 
+   */
+  abstract class EventEmitter extends DataFlow::Node {
+  	/**
+     * 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()
+        )
+      )
+    }
+	
+    /**
+     * Get a reference through type-tracking to this EventEmitter. 
+     * The type-tracking tracks through chainable methods.
+     */
+    DataFlow::SourceNode ref() { result = ref(DataFlow::TypeTracker::end()) }
+  }
+
+  /**
+   * A registration of an event handler on a particular EventEmitter.
+   */
+  abstract class EventRegistration extends DataFlow::Node {
+  	EventEmitter emitter;
+  	
+  	/** Gets the EventEmitter that the event handler is registered on. */
+  	final EventEmitter getEmitter() {
+  	  result = emitter	
+  	}
+  	
+    /** Gets the name of the channel if possible. */
+    abstract string getChannel();
+
+    /** Gets the `i`th parameter in the callback registered as the event handler. */
+    abstract DataFlow::Node getCallbackParameter(int i);
+    
+    /**
+     * Gets a value that is returned by the event handler to the `dispatch` where the event was dispatched. 
+     * The default implementation is that no value can be returned.
+     */
+    DataFlow::Node getAReturnedValue(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 has 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.getCallbackParameter(i)
+      )
+      or
+      pred = reg.getAReturnedValue(dispatch) and
+      succ = dispatch
+    }
+  }
+
+  /**
+   * Concrete classes for modelling EventEmitter in NodeJS.
+   */
+  private module NodeJSEventEmitter {
+    private class NodeJSEventEmitter extends EventEmitter {
+      NodeJSEventEmitter() {
+        exists(DataFlow::SourceNode clazz |
+          clazz = DataFlow::moduleImport("events") or
+          clazz = DataFlow::moduleMember("events", "EventEmitter")
+        |
+          this = clazz.getAnInstantiation()
+        )
+      }
+    }
+
+    private class EventEmitterRegistration extends EventRegistration, DataFlow::MethodCallNode {
+      override EventEmitter emitter;
+
+      EventEmitterRegistration() { this = emitter.ref().getAMethodCall(EventEmitter::on()) }
+
+      override string getChannel() { this.getArgument(0).mayHaveStringValue(result) }
+
+      override DataFlow::Node getCallbackParameter(int i) {
+      	result = this.(DataFlow::MethodCallNode).getABoundCallbackParameter(1, i)
+      }    
+    }
+
+    private class EventEmitterDispatch extends EventDispatch, DataFlow::MethodCallNode {
+      override EventEmitter emitter;
+      
+      EventEmitterDispatch() {
+      	this = emitter.ref().getAMethodCall("emit")
+      }
+    	      
+      override string getChannel() { this.getArgument(0).mayHaveStringValue(result) }
+
+      override DataFlow::Node getDispatchedArgument(int i) { result = this.getArgument(i + 1) }
+    }
+  }
+}

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

@@ -574,27 +574,6 @@ module SocketIOClient {
   }
 }
 
-/** Provides predicates for working with Node.js `EventEmitter`s. */
-private module EventEmitter {
-  /** Gets the name of a method on `EventEmitter` that returns `this`. */
-  string chainableMethod() {
-    result = "off" or
-    result = "removeAllListeners" or
-    result = "removeListener" or
-    result = "setMaxListeners" or
-    result = on()
-  }
-
-  /** Gets the name of a method on `EventEmitter` that registers an event handler. */
-  string on() {
-    result = "addListener" or
-    result = "on" or
-    result = "once" or
-    result = "prependListener" or
-    result = "prependOnceListener"
-  }
-}
-
 /** A data flow step through socket.io sockets. */
 private class SocketIoStep extends DataFlow::AdditionalFlowStep {
   DataFlow::Node pred;

javascript/ql/test/library-tests/frameworks/Electron/IpcFlow.expected

@@ -2,3 +2,5 @@
 | electron.js:48:23:48:28 | 'pong' | electron.js:58:1:58:36 | ipcRend ... 'ping') |
 | electron.js:56:27:56:32 | 'ping' | electron.js:42:29:42:31 | arg |
 | electron.js:58:30:58:35 | 'ping' | electron.js:47:28:47:30 | arg |
+| electron.js:68:24:68:28 | "foo" | electron.js:67:23:67:25 | foo |
+| electron.js:69:24:69:28 | "bar" | electron.js:67:46:67:48 | bar |

javascript/ql/test/library-tests/frameworks/Electron/WebContents.expected

@@ -1,4 +1,5 @@
 | electron.js:39:1:39:19 | foo(bw).webContents |
 | electron.js:40:1:40:19 | foo(bv).webContents |
+| electron.js:65:18:65:32 | win.webContents |
 | electron.ts:4:3:4:16 | bw.webContents |
 | electron.ts:5:3:5:16 | bv.webContents |

javascript/ql/test/library-tests/frameworks/Electron/electron.js

@@ -56,3 +56,15 @@ ipcRenderer.on('reply', (event, arg) => {
 ipcRenderer.send('async', 'ping');
 
 ipcRenderer.sendSync('sync', 'ping');
+
+
+(function () {
+  let win = new BrowserWindow({ width: 800, height: 1500 })
+  win.loadURL('http://github.com');
+
+  let contents = win.webContents;
+
+  contents.on("foo", (foo) => {}).on("bar", (bar) => {});
+  contents.emit("foo", "foo");
+  contents.emit("bar", "bar");
+})();

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

@@ -0,0 +1,6 @@
+| 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 |
+| tst.js:20:17:20:21 | "foo" | tst.js:19:16:19:18 | foo |
+| tst.js:21:17:21:21 | "bar" | tst.js:19:39:19:41 | bar |
+| tst.js:28:17:28:22 | "blab" | tst.js:25:16:25:20 | event |

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

@@ -0,0 +1,6 @@
+import javascript
+
+query predicate taintSteps(DataFlow::Node pred, DataFlow::Node succ) {
+  any(EventEmitter::EventEmitterTaintStep step).step(pred, succ)
+}
+

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

@@ -0,0 +1,28 @@
+var emitter = require('events').EventEmitter;
+
+var em = new emitter();
+
+// Splitting different channels
+em.addListener('FirstEvent', function (first) {});
+em.on('SecondEvent', function (second) {});
+
+em.emit('FirstEvent', 'FirstData');
+em.emit('SecondEvent', 'SecondData');
+
+// Splitting different emitters.
+var em2 = new emitter();
+em2.addListener('FirstEvent', function (otherFirst) {});
+em2.emit('FirstEvent', 'OtherFirstData');
+
+// Chaining.
+var em3 = new emitter();
+em3.on("foo", (foo) => {}).on("bar", (bar) => {});
+em3.emit("foo", "foo");
+em3.emit("bar", "bar");
+
+// Returning a value does not work here. 
+var em4 = new emitter();
+em3.on("bla", (event) => {
+	event.returnValue = "foo"
+});
+em3.emit("bla", "blab");