Skip to content

Commit

Permalink
fix: properly handle invalid data sent by a malicious websocket client
Browse files Browse the repository at this point in the history
**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 `[email protected]`, so
previous releases are not impacted.

[1]: f3c291f

Thanks to Marcus Wejderot from Mevisio for the responsible disclosure.
  • Loading branch information
darrachequesne committed Jan 11, 2022
1 parent b04967b commit c0e194d
Show file tree
Hide file tree
Showing 2 changed files with 40 additions and 3 deletions.
3 changes: 0 additions & 3 deletions lib/server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -609,9 +609,6 @@ export class Server extends BaseServer {
client.maybeUpgrade(transport);
}
} else {
// transport error handling takes over
websocket.removeListener("error", onUpgradeError);

const closeConnection = (errorCode, errorContext) =>
abortUpgrade(socket, errorCode, errorContext);
this.handshake(req._query.transport, req, closeConnection);
Expand Down
40 changes: 40 additions & 0 deletions test/server.js
Original file line number Diff line number Diff line change
Expand Up @@ -157,6 +157,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", () => {
Expand Down

0 comments on commit c0e194d

Please sign in to comment.