Skip to content

Commit 58617c5

Browse files
erik-krogherik-krogh
committed
recognize client websockets as ClientRequests
1 parent 1d9f8c2 commit 58617c5

File tree

5 files changed

+59
-2
lines changed

5 files changed

+59
-2
lines changed

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

Lines changed: 21 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -81,7 +81,7 @@ module ClientWebSocket {
8181
/**
8282
* A client WebSocket instance.
8383
*/
84-
class ClientSocket extends EventEmitter::Range, DataFlow::SourceNode {
84+
class ClientSocket extends EventEmitter::Range, DataFlow::NewNode, ClientRequest::Range {
8585
SocketClass socketClass;
8686

8787
ClientSocket() { this = socketClass.getAnInstantiation() }
@@ -90,6 +90,26 @@ module ClientWebSocket {
9090
* Gets the WebSocket library name.
9191
*/
9292
LibraryName getLibrary() { result = socketClass.getLibrary() }
93+
94+
override DataFlow::Node getUrl() { result = getArgument(0) }
95+
96+
override DataFlow::Node getHost() { none() }
97+
98+
override DataFlow::Node getADataNode() {
99+
exists(SendNode send |
100+
send.getEmitter() = this and
101+
result = send.getSentItem(_)
102+
)
103+
}
104+
105+
override DataFlow::Node getAResponseDataNode(string responseType, boolean promise) {
106+
responseType = "json" and
107+
promise = false and
108+
exists(WebSocketReceiveNode receiver |
109+
receiver.getEmitter() = this and
110+
result = receiver.getReceivedItem(_)
111+
)
112+
}
93113
}
94114

95115
/**

‎javascript/ql/test/library-tests/frameworks/ClientRequests/ClientRequests.expected

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -83,6 +83,7 @@ test_ClientRequest
8383
| tst.js:269:13:269:48 | httpPro ... ptions) |
8484
| tst.js:271:3:271:61 | proxy.w ... 080' }) |
8585
| tst.js:274:1:283:2 | httpPro ... true\\n}) |
86+
| tst.js:286:20:286:55 | new Web ... :8080') |
8687
test_getADataNode
8788
| tst.js:53:5:53:23 | axios({data: data}) | tst.js:53:18:53:21 | data |
8889
| tst.js:57:5:57:39 | axios.p ... data2}) | tst.js:57:19:57:23 | data1 |
@@ -121,6 +122,7 @@ test_getADataNode
121122
| tst.js:249:1:251:2 | form.su ... e();\\n}) | tst.js:246:26:246:43 | Buffer.from("foo") |
122123
| tst.js:249:1:251:2 | form.su ... e();\\n}) | tst.js:247:24:247:68 | request ... o.png') |
123124
| tst.js:257:1:262:2 | form.su ... rs()\\n}) | tst.js:255:25:255:35 | 'new_value' |
125+
| tst.js:286:20:286:55 | new Web ... :8080') | tst.js:288:21:288:35 | 'Hello Server!' |
124126
test_getHost
125127
| tst.js:87:5:87:39 | http.ge ... host}) | tst.js:87:34:87:37 | host |
126128
| tst.js:89:5:89:23 | axios({host: host}) | tst.js:89:18:89:21 | host |
@@ -218,6 +220,7 @@ test_getUrl
218220
| tst.js:267:1:267:61 | httpPro ... 9000'}) | tst.js:267:37:267:59 | 'http:/ ... t:9000' |
219221
| tst.js:271:3:271:61 | proxy.w ... 080' }) | tst.js:271:33:271:58 | 'http:/ ... m:8080' |
220222
| tst.js:274:1:283:2 | httpPro ... true\\n}) | tst.js:275:13:281:5 | {\\n ... ,\\n } |
223+
| tst.js:286:20:286:55 | new Web ... :8080') | tst.js:286:34:286:54 | 'ws://l ... t:8080' |
221224
test_getAResponseDataNode
222225
| tst.js:19:5:19:23 | requestPromise(url) | tst.js:19:5:19:23 | requestPromise(url) | text | true |
223226
| tst.js:21:5:21:23 | superagent.get(url) | tst.js:21:5:21:23 | superagent.get(url) | stream | true |
@@ -284,3 +287,4 @@ test_getAResponseDataNode
284287
| tst.js:231:5:233:6 | needle. ... \\n }) | tst.js:231:50:231:53 | body | json | false |
285288
| tst.js:235:5:237:6 | needle. ... \\n }) | tst.js:235:67:235:70 | resp | fetch.response | false |
286289
| tst.js:235:5:237:6 | needle. ... \\n }) | tst.js:235:73:235:76 | body | json | false |
290+
| tst.js:286:20:286:55 | new Web ... :8080') | tst.js:291:44:291:53 | event.data | json | false |

‎javascript/ql/test/library-tests/frameworks/ClientRequests/tst.js

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -280,4 +280,14 @@ httpProxy.createProxyServer({
280280
passphrase: 'password',
281281
},
282282
changeOrigin: true
283-
}).listen(8000);
283+
}).listen(8000);
284+
285+
function webSocket() {
286+
const socket = new WebSocket('ws://localhost:8080');
287+
socket.addEventListener('open', function (event) {
288+
socket.send('Hello Server!');
289+
});
290+
socket.addEventListener('message', function (event) {
291+
console.log("Data from server: " + event.data);
292+
});
293+
}

‎javascript/ql/test/query-tests/Security/CWE-918/RequestForgery.expected

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,14 @@ nodes
5757
| tst.js:74:29:74:35 | req.url |
5858
| tst.js:76:19:76:25 | tainted |
5959
| tst.js:76:19:76:25 | tainted |
60+
| tst.js:81:9:81:52 | tainted |
61+
| tst.js:81:19:81:42 | url.par ... , true) |
62+
| tst.js:81:19:81:48 | url.par ... ).query |
63+
| tst.js:81:19:81:52 | url.par ... ery.url |
64+
| tst.js:81:29:81:35 | req.url |
65+
| tst.js:81:29:81:35 | req.url |
66+
| tst.js:83:19:83:25 | tainted |
67+
| tst.js:83:19:83:25 | tainted |
6068
edges
6169
| tst.js:14:9:14:52 | tainted | tst.js:18:13:18:19 | tainted |
6270
| tst.js:14:9:14:52 | tainted | tst.js:18:13:18:19 | tainted |
@@ -113,6 +121,13 @@ edges
113121
| tst.js:74:19:74:52 | url.par ... ery.url | tst.js:74:9:74:52 | tainted |
114122
| tst.js:74:29:74:35 | req.url | tst.js:74:19:74:42 | url.par ... , true) |
115123
| tst.js:74:29:74:35 | req.url | tst.js:74:19:74:42 | url.par ... , true) |
124+
| tst.js:81:9:81:52 | tainted | tst.js:83:19:83:25 | tainted |
125+
| tst.js:81:9:81:52 | tainted | tst.js:83:19:83:25 | tainted |
126+
| tst.js:81:19:81:42 | url.par ... , true) | tst.js:81:19:81:48 | url.par ... ).query |
127+
| tst.js:81:19:81:48 | url.par ... ).query | tst.js:81:19:81:52 | url.par ... ery.url |
128+
| tst.js:81:19:81:52 | url.par ... ery.url | tst.js:81:9:81:52 | tainted |
129+
| tst.js:81:29:81:35 | req.url | tst.js:81:19:81:42 | url.par ... , true) |
130+
| tst.js:81:29:81:35 | req.url | tst.js:81:19:81:42 | url.par ... , true) |
116131
#select
117132
| tst.js:18:5:18:20 | request(tainted) | tst.js:14:29:14:35 | req.url | tst.js:18:13:18:19 | tainted | The $@ of this request depends on $@. | tst.js:18:13:18:19 | tainted | URL | tst.js:14:29:14:35 | req.url | a user-provided value |
118133
| tst.js:20:5:20:24 | request.get(tainted) | tst.js:14:29:14:35 | req.url | tst.js:20:17:20:23 | tainted | The $@ of this request depends on $@. | tst.js:20:17:20:23 | tainted | URL | tst.js:14:29:14:35 | req.url | a user-provided value |
@@ -130,3 +145,4 @@ edges
130145
| tst.js:64:3:64:38 | client. ... inted}) | tst.js:58:29:58:35 | req.url | tst.js:64:30:64:36 | tainted | The $@ of this request depends on $@. | tst.js:64:30:64:36 | tainted | URL | tst.js:58:29:58:35 | req.url | a user-provided value |
131146
| tst.js:68:3:68:38 | client. ... inted}) | tst.js:58:29:58:35 | req.url | tst.js:68:30:68:36 | tainted | The $@ of this request depends on $@. | tst.js:68:30:68:36 | tainted | URL | tst.js:58:29:58:35 | req.url | a user-provided value |
132147
| tst.js:76:5:76:26 | JSDOM.f ... ainted) | tst.js:74:29:74:35 | req.url | tst.js:76:19:76:25 | tainted | The $@ of this request depends on $@. | tst.js:76:19:76:25 | tainted | URL | tst.js:74:29:74:35 | req.url | a user-provided value |
148+
| tst.js:83:5:83:26 | new Web ... ainted) | tst.js:81:29:81:35 | req.url | tst.js:83:19:83:25 | tainted | The $@ of this request depends on $@. | tst.js:83:19:83:25 | tainted | URL | tst.js:81:29:81:35 | req.url | a user-provided value |

‎javascript/ql/test/query-tests/Security/CWE-918/tst.js

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -74,4 +74,11 @@ var server = http.createServer(async function(req, res) {
7474
var tainted = url.parse(req.url, true).query.url;
7575

7676
JSDOM.fromURL(tainted); // NOT OK
77+
});
78+
79+
import {JSDOM} from "jsdom";
80+
var server = http.createServer(async function(req, res) {
81+
var tainted = url.parse(req.url, true).query.url;
82+
83+
new WebSocket(tainted); // NOT OK
7784
});

0 commit comments

Comments
 (0)