Skip to content

Harden Node.js WebSocket server handling #3391

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
Jens-G wants to merge 2 commits into from
+25 −9
Closed

Harden Node.js WebSocket server handling #3391

Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
3e69d7e
Harden Node.js WebSocket server handling
Jens-G Apr 11, 2026
bbb5795
Extract sanitizeHeader() for CRLF stripping
Jens-G Apr 12, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 22 additions & 9 deletions lib/nodejs/lib/thrift/web_server.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ var wsFrame = {
* @returns {Buffer} - The WebSocket frame, ready to send
*/
encode: function (data, mask, binEncoding) {
var frame = new Buffer(wsFrame.frameSizeFromData(data, mask));
var frame = Buffer.alloc(wsFrame.frameSizeFromData(data, mask));
//Byte 0 - FIN & OPCODE
frame[0] =
wsFrame.fin.FIN +
Expand Down Expand Up @@ -171,19 +171,19 @@ var wsFrame = {
}
//MASK
if (wsFrame.mask.TO_SERVER == (frame[1] & wsFrame.mask.TO_SERVER)) {
result.mask = new Buffer(4);
result.mask = Buffer.alloc(4);
frame.copy(result.mask, 0, dataOffset, dataOffset + 4);
dataOffset += 4;
}
//Payload
result.data = new Buffer(len);
result.data = Buffer.alloc(len);
frame.copy(result.data, 0, dataOffset, dataOffset + len);
if (result.mask) {
wsFrame.applyMask(result.data, result.mask);
}
//Next Frame
if (frame.length > dataOffset + len) {
result.nextFrame = new Buffer(frame.length - (dataOffset + len));
result.nextFrame = Buffer.alloc(frame.length - (dataOffset + len));
frame.copy(result.nextFrame, 0, dataOffset + len, frame.length);
}
//Don't forward control frames
Expand Down Expand Up @@ -450,7 +450,8 @@ exports.createWebServer = function (options) {
var filename = path.resolve(path.join(baseDir, uri));

//Ensure the basedir path is not able to be escaped
if (filename.indexOf(baseDir) != 0) {
var normalizedBase = baseDir.endsWith(path.sep) ? baseDir : baseDir + path.sep;
if (filename !== baseDir && filename.indexOf(normalizedBase) !== 0) {
response.writeHead(400, "Invalid request path", {});
response.end();
return;
Expand Down Expand Up @@ -543,6 +544,14 @@ exports.createWebServer = function (options) {
}
})
.on("upgrade", function (request, socket, head) {
//Verify CORS origin for WebSocket upgrades
if (request.headers.origin && options.cors) {
if (!options.cors["*"] && !options.cors[request.headers.origin]) {
socket.write("HTTP/1.1 403 Origin not allowed\r\n\r\n");
socket.destroy();
return;
}
}
//Lookup service
var svc;
try {
Expand All @@ -557,6 +566,10 @@ exports.createWebServer = function (options) {
request.headers["sec-websocket-key"] +
"258EAFA5-E914-47DA-95CA-C5AB0DC85B11",
);
//Sanitize header values to prevent CRLF injection
var origin = (request.headers.origin || "").replace(/[\r\n]/g, "");
var host = (request.headers.host || "").replace(/[\r\n]/g, "");
var reqUrl = (request.url || "").replace(/[\r\n]/g, "");
Copy link
Copy Markdown
Member

@jimexist jimexist Apr 11, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can have a dedicated function to do this

socket.write(
"HTTP/1.1 101 Switching Protocols\r\n" +
"Upgrade: websocket\r\n" +
Expand All @@ -565,11 +578,11 @@ exports.createWebServer = function (options) {
hash.digest("base64") +
"\r\n" +
"Sec-WebSocket-Origin: " +
request.headers.origin +
origin +
"\r\n" +
"Sec-WebSocket-Location: ws://" +
request.headers.host +
request.url +
host +
reqUrl +
"\r\n" +
"\r\n",
);
Expand All @@ -582,7 +595,7 @@ exports.createWebServer = function (options) {
//Prepend any existing decoded data
if (data) {
if (result.data) {
var newData = new Buffer(data.length + result.data.length);
var newData = Buffer.alloc(data.length + result.data.length);
data.copy(newData);
result.data.copy(newData, data.length);
result.data = newData;
Expand Down
Loading