From a70800d7e96da32f6e6622804ef659ebc58659db Mon Sep 17 00:00:00 2001 From: Damien Arrachequesne Date: Tue, 11 Jan 2022 15:34:18 +0100 Subject: [PATCH] fix: properly handle invalid data sent by a malicious websocket client **IMPORTANT SECURITY FIX** A malicious client could send a specially crafted HTTP request, triggering an uncaught exception and killing the Node.js process: > RangeError: Invalid WebSocket frame: RSV2 and RSV3 must be clear > at Receiver.getInfo (/.../node_modules/ws/lib/receiver.js:176:14) > at Receiver.startLoop (/.../node_modules/ws/lib/receiver.js:136:22) > at Receiver._write (/.../node_modules/ws/lib/receiver.js:83:10) > at writeOrBuffer (internal/streams/writable.js:358:12) This bug was introduced by [1], included in `engine.io@4.0.0`, so previous releases are not impacted. [1]: https://github.com/socketio/engine.io/commit/f3c291fa613a9d50c924d74293035737fdace4f2 Thanks to Marcus Wejderot from Mevisio for the responsible disclosure. Backported from master: https://github.com/socketio/engine.io/commit/c0e194d44933bd83bf9a4b126fca68ba7bf5098c --- lib/server.js | 3 --- test/server.js | 40 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 40 insertions(+), 3 deletions(-) diff --git a/lib/server.js b/lib/server.js index a7007357c..f2441bc9f 100644 --- a/lib/server.js +++ b/lib/server.js @@ -372,9 +372,6 @@ class Server extends EventEmitter { client.maybeUpgrade(transport); } } else { - // transport error handling takes over - socket.removeListener("error", onUpgradeError); - this.handshake(req._query.transport, req); } diff --git a/test/server.js b/test/server.js index 9d846123f..4ec08fcd5 100644 --- a/test/server.js +++ b/test/server.js @@ -107,6 +107,46 @@ describe("server", () => { } ); }); + + it("should not throw when the client sends invalid data during the handshake (ws only)", done => { + listen(port => { + // will throw "RangeError: Invalid WebSocket frame: RSV2 and RSV3 must be clear" + request + .get(`http://localhost:${port}/engine.io/`) + .set("connection", "upgrade") + .set("upgrade", "websocket") + .set("Sec-WebSocket-Version", "13") + .set("Sec-WebSocket-Key", "DXR4dX615eRds8nRmlhqtw==") + .query({ transport: "websocket", EIO: 4 }) + .send("test") + .end(() => {}); + + setTimeout(done, 50); + }); + }); + + it("should not throw when the client sends invalid data during the handshake (upgrade)", done => { + listen(port => { + request + .get(`http://localhost:${port}/engine.io/`) + .query({ transport: "polling", EIO: 4 }) + .end((err, res) => { + const sid = JSON.parse(res.text.substring(1)).sid; + + request + .get(`http://localhost:${port}/engine.io/`) + .set("connection", "upgrade") + .set("upgrade", "websocket") + .set("Sec-WebSocket-Version", "13") + .set("Sec-WebSocket-Key", "DXR4dX615eRds8nRmlhqtw==") + .query({ transport: "websocket", EIO: 4, sid }) + .send("test") + .end(() => {}); + + setTimeout(done, 50); + }); + }); + }); }); describe("handshake", () => {