From 20312fc3bf5ceeb95aa1ed2e3806e2e1b4a7545c Mon Sep 17 00:00:00 2001 From: Max Schaefer Date: Fri, 5 Apr 2019 17:29:14 +0100 Subject: [PATCH] JavaScript: Improve socket.io model. Recognise `io` imports and use type-tracking to better track handlers. --- .../src/semmle/javascript/frameworks/SocketIO.qll | 13 ++++++++++++- .../library-tests/frameworks/SocketIO/client2.js | 4 +--- .../library-tests/frameworks/SocketIO/client4.js | 4 ++++ .../library-tests/frameworks/SocketIO/handler.js | 1 + .../frameworks/SocketIO/tests.expected | 15 +++++++++------ 5 files changed, 27 insertions(+), 10 deletions(-) create mode 100644 javascript/ql/test/library-tests/frameworks/SocketIO/client4.js create mode 100644 javascript/ql/test/library-tests/frameworks/SocketIO/handler.js diff --git a/javascript/ql/src/semmle/javascript/frameworks/SocketIO.qll b/javascript/ql/src/semmle/javascript/frameworks/SocketIO.qll index 69add8bef3e..339b8aece87 100644 --- a/javascript/ql/src/semmle/javascript/frameworks/SocketIO.qll +++ b/javascript/ql/src/semmle/javascript/frameworks/SocketIO.qll @@ -399,6 +399,8 @@ module SocketIOClient { exists(DataFlow::SourceNode io | io = DataFlow::globalVarRef("io") or io = DataFlow::globalVarRef("io").getAPropertyRead("connect") or + io = DataFlow::moduleImport("io") or + io = DataFlow::moduleMember("io", "connect") or io = DataFlow::moduleImport("socket.io-client") or io = DataFlow::moduleMember("socket.io-client", "connect") | @@ -479,9 +481,18 @@ module SocketIOClient { /** Gets the event name associated with the data, if it can be determined. */ string getEventName() { getArgument(0).mayHaveStringValue(result) } + private DataFlow::SourceNode getListener(DataFlow::TypeBackTracker t) { + t.start() and + result = getArgument(1).getALocalSource() + or + exists(DataFlow::TypeBackTracker t2 | + result = getListener(t2).backtrack(t2, t) + ) + } + /** Gets the callback that handles data received from the server. */ private DataFlow::FunctionNode getListener() { - result = getCallback(1) + result = getListener(DataFlow::TypeBackTracker::end()) } /** Gets the `i`th parameter through which data is received from the server. */ diff --git a/javascript/ql/test/library-tests/frameworks/SocketIO/client2.js b/javascript/ql/test/library-tests/frameworks/SocketIO/client2.js index eaa207046f2..19d0382d545 100644 --- a/javascript/ql/test/library-tests/frameworks/SocketIO/client2.js +++ b/javascript/ql/test/library-tests/frameworks/SocketIO/client2.js @@ -15,6 +15,4 @@ sock.emit('data', "hi", "there"); sock.write("do you copy?", () => {}); -sock2.on('message', (x) => { - console.log(x); -}); \ No newline at end of file +sock2.on('message', require('./handler')); \ No newline at end of file diff --git a/javascript/ql/test/library-tests/frameworks/SocketIO/client4.js b/javascript/ql/test/library-tests/frameworks/SocketIO/client4.js new file mode 100644 index 00000000000..dfce8940d5b --- /dev/null +++ b/javascript/ql/test/library-tests/frameworks/SocketIO/client4.js @@ -0,0 +1,4 @@ +import io from "io"; + +io(); +io.connect("/messages"); diff --git a/javascript/ql/test/library-tests/frameworks/SocketIO/handler.js b/javascript/ql/test/library-tests/frameworks/SocketIO/handler.js new file mode 100644 index 00000000000..db541132ff8 --- /dev/null +++ b/javascript/ql/test/library-tests/frameworks/SocketIO/handler.js @@ -0,0 +1 @@ +module.exports = (x) => console.log(x); diff --git a/javascript/ql/test/library-tests/frameworks/SocketIO/tests.expected b/javascript/ql/test/library-tests/frameworks/SocketIO/tests.expected index e174abd56b5..765fef19e93 100644 --- a/javascript/ql/test/library-tests/frameworks/SocketIO/tests.expected +++ b/javascript/ql/test/library-tests/frameworks/SocketIO/tests.expected @@ -1,7 +1,7 @@ test_ClientReceiveNode_getEventName | client2.js:4:1:6:2 | sock.on ... y);\\n}) | message | | client2.js:10:1:12:2 | sock.on ... d");\\n}) | event | -| client2.js:18:1:20:2 | sock2.o ... (x);\\n}) | message | +| client2.js:18:1:18:41 | sock2.o ... dler')) | message | test_NamespaceNode | tst.js:25:10:25:19 | io.sockets | socket.io namespace with path '/' | | tst.js:26:11:26:27 | io.of("/foo/bar") | socket.io namespace with path '/foo/bar' | @@ -47,7 +47,7 @@ test_ClientReceiveNode_getASender | client2.js:8:1:8:33 | sock.on ... => {}) | tst.js:55:3:55:27 | socket. ... ssage') | | client2.js:10:1:12:2 | sock.on ... d");\\n}) | tst.js:30:1:30:28 | ns.emit ... event') | | client2.js:10:1:12:2 | sock.on ... d");\\n}) | tst.js:51:3:51:22 | socket.emit('event') | -| client2.js:18:1:20:2 | sock2.o ... (x);\\n}) | tst.js:32:1:32:22 | ns2.wri ... ssage') | +| client2.js:18:1:18:41 | sock2.o ... dler')) | tst.js:32:1:32:22 | ns2.wri ... ssage') | test_ReceiveNode | tst.js:70:3:70:35 | socket. ... => {}) | tst.js:69:22:69:27 | socket | | tst.js:71:3:71:46 | socket. ... => {}) | tst.js:69:22:69:27 | socket | @@ -66,11 +66,12 @@ test_AdditionalFlowStep | client2.js:16:12:16:25 | "do you copy?" | tst.js:70:25:70:27 | msg | | client2.js:16:12:16:25 | "do you copy?" | tst.js:71:27:71:31 | data1 | | client3.js:1:8:1:9 | io | client3.js:1:8:1:9 | io | +| client4.js:1:8:1:9 | io | client4.js:1:8:1:9 | io | | tst.js:30:18:30:27 | 'an event' | client2.js:8:23:8:25 | msg | | tst.js:30:18:30:27 | 'an event' | client2.js:10:19:10:19 | x | | tst.js:31:9:31:19 | 'a message' | client2.js:4:21:4:21 | x | | tst.js:31:9:31:19 | 'a message' | client2.js:8:23:8:25 | msg | -| tst.js:32:11:32:21 | 'a message' | client2.js:18:22:18:22 | x | +| tst.js:32:11:32:21 | 'a message' | handler.js:1:19:1:19 | x | | tst.js:39:20:39:30 | 'a message' | client2.js:4:21:4:21 | x | | tst.js:39:20:39:30 | 'a message' | client2.js:8:23:8:25 | msg | | tst.js:40:9:40:19 | 'a message' | client2.js:4:21:4:21 | x | @@ -123,6 +124,8 @@ test_ClientSocketNode | client2.js:1:12:1:56 | require ... lhost") | / | | client2.js:2:13:2:85 | require ... v#abc") | /foo/bar | | client3.js:3:1:3:4 | io() | / | +| client4.js:3:1:3:4 | io() | / | +| client4.js:4:1:4:23 | io.conn ... sages") | /messages | test_ReceiveNode_getASender | tst.js:70:3:70:35 | socket. ... => {}) | client2.js:16:1:16:36 | sock.wr ... => {}) | | tst.js:71:3:71:46 | socket. ... => {}) | client2.js:16:1:16:36 | sock.wr ... => {}) | @@ -162,7 +165,7 @@ test_ClientReceiveNode_getReceivedItem | client2.js:4:1:6:2 | sock.on ... y);\\n}) | 1 | client2.js:4:24:4:24 | y | | client2.js:8:1:8:33 | sock.on ... => {}) | 0 | client2.js:8:23:8:25 | msg | | client2.js:10:1:12:2 | sock.on ... d");\\n}) | 0 | client2.js:10:19:10:19 | x | -| client2.js:18:1:20:2 | sock2.o ... (x);\\n}) | 0 | client2.js:18:22:18:22 | x | +| client2.js:18:1:18:41 | sock2.o ... dler')) | 0 | handler.js:1:19:1:19 | x | test_NamespaceObject | socket.io namespace with path '/' | tst.js:1:12:1:33 | socket.io server | / | | socket.io namespace with path '/' | tst.js:4:13:4:24 | socket.io server | / | @@ -172,7 +175,7 @@ test_ClientReceiveNode | client2.js:4:1:6:2 | sock.on ... y);\\n}) | client2.js:1:12:1:56 | require ... lhost") | | client2.js:8:1:8:33 | sock.on ... => {}) | client2.js:1:12:1:56 | require ... lhost") | | client2.js:10:1:12:2 | sock.on ... d");\\n}) | client2.js:1:12:1:56 | require ... lhost") | -| client2.js:18:1:20:2 | sock2.o ... (x);\\n}) | client2.js:2:13:2:85 | require ... v#abc") | +| client2.js:18:1:18:41 | sock2.o ... dler')) | client2.js:2:13:2:85 | require ... v#abc") | test_ClientSendNode | client2.js:14:1:14:32 | sock.em ... there") | client2.js:1:12:1:56 | require ... lhost") | / | | client2.js:16:1:16:36 | sock.wr ... => {}) | client2.js:1:12:1:56 | require ... lhost") | / | @@ -193,7 +196,7 @@ test_SendNode_getAReceiver | tst.js:30:1:30:28 | ns.emit ... event') | client2.js:10:1:12:2 | sock.on ... d");\\n}) | | tst.js:31:1:31:20 | ns.send('a message') | client2.js:4:1:6:2 | sock.on ... y);\\n}) | | tst.js:31:1:31:20 | ns.send('a message') | client2.js:8:1:8:33 | sock.on ... => {}) | -| tst.js:32:1:32:22 | ns2.wri ... ssage') | client2.js:18:1:20:2 | sock2.o ... (x);\\n}) | +| tst.js:32:1:32:22 | ns2.wri ... ssage') | client2.js:18:1:18:41 | sock2.o ... dler')) | | tst.js:39:1:39:31 | io.emit ... ssage') | client2.js:4:1:6:2 | sock.on ... y);\\n}) | | tst.js:39:1:39:31 | io.emit ... ssage') | client2.js:8:1:8:33 | sock.on ... => {}) | | tst.js:40:1:40:20 | io.send('a message') | client2.js:4:1:6:2 | sock.on ... y);\\n}) |