Merge pull request #3119 from erik-krogh/SockJS

Approved by esbena

javascript/ql/src/experimental/SockJS/SockJS.qll

@@ -1,46 +0,0 @@
-/**
- * Provides classes for working with [SockJS](http://sockjs.org).
- */
-
-import javascript
-
-/**
- * A model of the `SockJS` websocket data handler (https://sockjs.org).
- */
-module SockJS {
-  /**
-   * Access to user-controlled data object received from websocket
-   * For example:
-   * ```
-   * server.on('connection', function(conn) {
-   *     conn.on('data', function(message) {
-   *       ...
-   *     });
-   *  });
-   * ```
-   */
-  class SourceFromSocketJS extends RemoteFlowSource {
-    SourceFromSocketJS() {
-      exists(
-        DataFlow::CallNode createServer, DataFlow::CallNode connNode,
-        DataFlow::CallNode dataHandlerNode
-      |
-        createServer = appCreation() and
-        connNode = createServer.getAMethodCall("on") and
-        connNode.getArgument(0).getStringValue() = "connection" and
-        dataHandlerNode = connNode.getCallback(1).getParameter(0).getAMethodCall("on") and
-        dataHandlerNode.getArgument(0).getStringValue() = "data" and
-        this = dataHandlerNode.getCallback(1).getParameter(0)
-      )
-    }
-
-    override string getSourceType() { result = "input from SockJS WebSocket" }
-  }
-
-  /**
-   * Gets a new SockJS server.
-   */
-  private DataFlow::CallNode appCreation() {
-    result = DataFlow::moduleImport("sockjs").getAMemberCall("createServer")
-  }
-}

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

@@ -1,5 +1,5 @@
 /**
- * Provides classes for working with [WebSocket](https://developer.mozilla.org/en-US/docs/Web/API/WebSocket) and [ws](https://github.com/websockets/ws).
+ * Provides classes for working with [WebSocket](https://developer.mozilla.org/en-US/docs/Web/API/WebSocket), [ws](https://github.com/websockets/ws), and [SockJS](http://sockjs.org).
  *
  * The model is based on the EventEmitter model, and there is therefore a
  * data-flow step from where a WebSocket event is sent to where the message
@@ -18,26 +18,64 @@ import javascript
  */
 private string channelName() { result = "message" }
 
+/**
+ * The names of the libraries modelled in this file.
+ */
+private module LibraryNames {
+  string sockjs() { result = "SockJS" }
+
+  string websocket() { result = "WebSocket" }
+
+  string ws() { result = "ws" }
+
+  class LibraryName extends string {
+    LibraryName() { this = sockjs() or this = websocket() or this = ws() }
+  }
+}
+
+/**
+ * Holds if the websocket library named `client` can send a message to the library named `server`.
+ * Both `client` and `server` are library names defined in `LibraryNames`.
+ */
+private predicate areLibrariesCompatible(
+  LibraryNames::LibraryName client, LibraryNames::LibraryName server
+) {
+  // sockjs is a WebSocket emulating library, but not actually an implementation of WebSockets.
+  client = LibraryNames::sockjs() and server = LibraryNames::sockjs()
+  or
+  server = LibraryNames::ws() and
+  (client = LibraryNames::ws() or client = LibraryNames::websocket())
+}
+
 /**
  * Provides classes that model WebSockets clients.
  */
 module ClientWebSocket {
+  private import LibraryNames
+
   /**
    * A class that can be used to instantiate a WebSocket instance.
    */
   class SocketClass extends DataFlow::SourceNode {
-    boolean isNode;
+    LibraryName library; // the name of the WebSocket library. Can be one of the libraries defined in `LibraryNames`.
 
     SocketClass() {
-      this = DataFlow::globalVarRef("WebSocket") and isNode = false
+      this = DataFlow::globalVarRef("WebSocket") and library = websocket()
       or
-      this = DataFlow::moduleImport("ws") and isNode = true
+      this = DataFlow::moduleImport("ws") and library = ws()
+      or
+      // the sockjs-client library:https://www.npmjs.com/package/sockjs-client
+      library = sockjs() and
+      (
+        this = DataFlow::moduleImport("sockjs-client") or
+        this = DataFlow::globalVarRef("SockJS")
+      )
     }
 
     /**
-     * Holds if this class is an import of the "ws" module.
+     * Gets the WebSocket library name.
      */
-    predicate isNode() { isNode = true }
+    LibraryName getLibrary() { result = library }
   }
 
   /**
@@ -49,11 +87,9 @@ module ClientWebSocket {
     ClientSocket() { this = socketClass.getAnInstantiation() }
 
     /**
-     * Holds if this ClientSocket is created from the "ws" module.
-     *
-     * The predicate is used to differentiate where the behavior of the "ws" module differs from the native WebSocket in browsers.
+     * Gets the WebSocket library name.
      */
-    predicate isNode() { socketClass.isNode() }
+    LibraryName getLibrary() { result = socketClass.getLibrary() }
   }
 
   /**
@@ -68,7 +104,10 @@ module ClientWebSocket {
 
     override DataFlow::Node getSentItem(int i) { i = 0 and result = this.getArgument(0) }
 
-    override ServerWebSocket::ReceiveNode getAReceiver() { any() }
+    override ServerWebSocket::ReceiveNode getAReceiver() {
+      areLibrariesCompatible(emitter.getLibrary(),
+        result.getEmitter().(ServerWebSocket::ServerSocket).getLibrary())
+    }
   }
 
   /**
@@ -116,7 +155,7 @@ module ClientWebSocket {
    */
   private class WSReceiveNode extends ClientWebSocket::ReceiveNode {
     WSReceiveNode() {
-      emitter.isNode() and
+      emitter.getLibrary() = ws() and
       this = getAMessageHandler(emitter, EventEmitter::on())
     }
 
@@ -128,21 +167,38 @@ module ClientWebSocket {
  * Provides classes that model WebSocket servers.
  */
 module ServerWebSocket {
+  private import LibraryNames
+
+  /**
+   * Gets a server created by a library named `library`.
+   */
+  DataFlow::SourceNode getAServer(LibraryName library) {
+    library = ws() and
+    result = DataFlow::moduleImport("ws").getAConstructorInvocation("Server")
+    or
+    library = sockjs() and
+    result = DataFlow::moduleImport("sockjs").getAMemberCall("createServer")
+  }
+
   /**
    * A server WebSocket instance.
    */
   class ServerSocket extends EventEmitter::Range, DataFlow::SourceNode {
+    LibraryName library;
+
     ServerSocket() {
       exists(DataFlow::CallNode onCall |
-        onCall =
-          DataFlow::moduleImport("ws")
-              .getAConstructorInvocation("Server")
-              .getAMemberCall(EventEmitter::on()) and
+        onCall = getAServer(library).getAMemberCall(EventEmitter::on()) and
         onCall.getArgument(0).mayHaveStringValue("connection")
       |
         this = onCall.getCallback(1).getParameter(0)
       )
     }
+
+    /**
+     * Gets the name of the library that created this server socket.
+     */
+    LibraryName getLibrary() { result = library }
   }
 
   /**
@@ -151,7 +207,13 @@ module ServerWebSocket {
   class SendNode extends EventDispatch::Range, DataFlow::CallNode {
     override ServerSocket emitter;
 
-    SendNode() { this = emitter.getAMemberCall("send") }
+    SendNode() {
+      emitter.getLibrary() = ws() and
+      this = emitter.getAMemberCall("send")
+      or
+      emitter.getLibrary() = sockjs() and
+      this = emitter.getAMemberCall("write")
+    }
 
     override string getChannel() { result = channelName() }
 
@@ -160,7 +222,10 @@ module ServerWebSocket {
       result = getArgument(0)
     }
 
-    override ClientWebSocket::ReceiveNode getAReceiver() { any() }
+    override ClientWebSocket::ReceiveNode getAReceiver() {
+      areLibrariesCompatible(result.getEmitter().(ClientWebSocket::ClientSocket).getLibrary(),
+        emitter.getLibrary())
+    }
   }
 
   /**
@@ -170,8 +235,14 @@ module ServerWebSocket {
     override ServerSocket emitter;
 
     ReceiveNode() {
-      this = emitter.getAMemberCall(EventEmitter::on()) and
-      this.getArgument(0).mayHaveStringValue("message")
+      exists(string eventName |
+        emitter.getLibrary() = ws() and eventName = "message"
+        or
+        emitter.getLibrary() = sockjs() and eventName = "data"
+      |
+        this = emitter.getAMemberCall(EventEmitter::on()) and
+        this.getArgument(0).mayHaveStringValue(eventName)
+      )
     }
 
     override string getChannel() { result = channelName() }

javascript/ql/test/library-tests/frameworks/WebSocket/browser.js

@@ -2,14 +2,31 @@
 	const socket = new WebSocket('ws://localhost:8080');
 
 	socket.addEventListener('open', function (event) {
-    	socket.send('Hi from browser!');
+		socket.send('Hi from browser!');
 	});
 
 	socket.addEventListener('message', function (event) {
-	    console.log('Message from server ', event.data);
+		console.log('Message from server ', event.data);
 	});
+
+	socket.onmessage = function (event) {
+		console.log("Message from server 2", event.data)
+	};
+})();
+
+
+(function () {
+	var sock = new SockJS('http://0.0.0.0:9999/echo');
+	sock.onopen = function () {
+		sock.send('test');
+	};
 	
-	socket.onmessage = function(event) {
-      console.log("Message from server 2", event.data)
-    };
-})();
+	sock.onmessage = function (e) {
+		console.log('message', e.data);
+		sock.close();
+	};
+	
+	sock.addEventListener('message', function (event) {
+		console.log('Using addEventListener ', event.data);
+	});
+})

javascript/ql/test/library-tests/frameworks/WebSocket/client.js

@@ -4,10 +4,10 @@
 	const ws = new WebSocket('ws://example.org');
 
 	ws.on('open', function open() {
-  		ws.send('Hi from client!');
+		ws.send('Hi from client!');
 	});
 
 	ws.on('message', function incoming(data) {
-  		console.log(data);
+		console.log(data);
 	});
 })();

javascript/ql/test/library-tests/frameworks/WebSocket/server.js

@@ -4,10 +4,10 @@
 	const wss = new WebSocket.Server({ port: 8080 });
 
 	wss.on('connection', function connection(ws) {
-  		ws.on('message', function incoming(message) {
-    		console.log('received: %s', message);
-  		});
+		ws.on('message', function incoming(message) {
+			console.log('received: %s', message);
+		});
 
-  		ws.send('Hi from server!');
+		ws.send('Hi from server!');
 	});
 })();

javascript/ql/test/library-tests/frameworks/WebSocket/sockjs.js

@@ -5,12 +5,12 @@ const sockjs = require('sockjs');
 const app = express();
 const server = http.createServer(app);
 const sockjs_echo = sockjs.createServer({});
-sockjs_echo.on('connection', function(conn) {
-    conn.on('data', function(message) {
+sockjs_echo.on('connection', function (conn) {
+    conn.on('data', function (message) {
         var data = JSON.parse(message);
         conn.write(JSON.stringify(eval(data.test)));
     });
 });
 
-sockjs_echo.installHandlers(server, {prefix:'/echo'});
+sockjs_echo.installHandlers(server, { prefix: '/echo' });
 server.listen(9090, '127.0.0.1');

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

@@ -1,22 +1,35 @@
 clientSocket
 | browser.js:2:17:2:52 | new Web ... :8080') |
+| browser.js:19:13:19:50 | new Soc ... /echo') |
 | client.js:4:13:4:45 | new Web ... e.org') |
 clientSend
-| browser.js:5:6:5:36 | socket. ... wser!') |
-| client.js:7:5:7:30 | ws.send ... ient!') |
+| browser.js:5:3:5:33 | socket. ... wser!') |
+| browser.js:21:3:21:19 | sock.send('test') |
+| client.js:7:3:7:28 | ws.send ... ient!') |
 clientReceive
 | browser.js:8:37:10:2 | functio ... ta);\\n\\t} |
-| browser.js:12:21:14:5 | functio ... )\\n    } |
+| browser.js:12:21:14:2 | functio ... ata)\\n\\t} |
+| browser.js:24:19:27:2 | functio ... e();\\n\\t} |
+| browser.js:29:35:31:2 | functio ... ta);\\n\\t} |
 | client.js:10:19:12:2 | functio ... ta);\\n\\t} |
 serverSocket
 | server.js:6:43:6:44 | ws |
+| sockjs.js:8:40:8:43 | conn |
 serverSend
-| server.js:11:5:11:30 | ws.send ... rver!') |
+| server.js:11:3:11:28 | ws.send ... rver!') |
+| sockjs.js:11:9:11:51 | conn.wr ... test))) |
 serverReceive
-| server.js:7:5:9:6 | ws.on(' ... \\n  \\t\\t}) |
+| server.js:7:3:9:4 | ws.on(' ... );\\n\\t\\t}) |
+| sockjs.js:9:5:12:6 | conn.on ... \\n    }) |
 taintStep
-| browser.js:5:18:5:35 | 'Hi from browser!' | server.js:7:40:7:46 | message |
-| client.js:7:13:7:29 | 'Hi from client!' | server.js:7:40:7:46 | message |
-| server.js:11:13:11:29 | 'Hi from server!' | browser.js:9:42:9:51 | event.data |
-| server.js:11:13:11:29 | 'Hi from server!' | browser.js:13:44:13:53 | event.data |
-| server.js:11:13:11:29 | 'Hi from server!' | client.js:10:37:10:40 | data |
+| browser.js:5:15:5:32 | 'Hi from browser!' | server.js:7:38:7:44 | message |
+| browser.js:21:13:21:18 | 'test' | sockjs.js:9:31:9:37 | message |
+| client.js:7:11:7:27 | 'Hi from client!' | server.js:7:38:7:44 | message |
+| server.js:11:11:11:27 | 'Hi from server!' | browser.js:9:39:9:48 | event.data |
+| server.js:11:11:11:27 | 'Hi from server!' | browser.js:13:40:13:49 | event.data |
+| server.js:11:11:11:27 | 'Hi from server!' | client.js:10:37:10:40 | data |
+| sockjs.js:11:20:11:50 | JSON.st ... .test)) | browser.js:25:26:25:31 | e.data |
+| sockjs.js:11:20:11:50 | JSON.st ... .test)) | browser.js:30:42:30:51 | event.data |
+remoteFlow
+| server.js:7:38:7:44 | message |
+| sockjs.js:9:31:9:37 | message |

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

@@ -15,3 +15,5 @@ query ServerWebSocket::ReceiveNode serverReceive() { any() }
 query predicate taintStep(DataFlow::Node pred, DataFlow::Node succ) {
   any(DataFlow::AdditionalFlowStep s).step(pred, succ)
 }
+
+query RemoteFlowSource remoteFlow() { any() }