From 3e69d7edef66db92c0d0036b66c3fda0e12524f7 Mon Sep 17 00:00:00 2001 From: Jens Geyer Date: Sat, 11 Apr 2026 12:29:16 +0200 Subject: [PATCH 1/2] Harden Node.js WebSocket server handling Client: nodejs - Validate origin on WebSocket upgrade using the same CORS rules as HTTP requests - Fix path containment check to use trailing separator - Replace deprecated new Buffer() with Buffer.alloc() - Sanitize interpolated header values in upgrade response (04) Co-Authored-By: Claude Opus 4.6 --- lib/nodejs/lib/thrift/web_server.js | 31 ++++++++++++++++++++--------- 1 file changed, 22 insertions(+), 9 deletions(-) diff --git a/lib/nodejs/lib/thrift/web_server.js b/lib/nodejs/lib/thrift/web_server.js index f49f654b116..ddd13cd60d8 100644 --- a/lib/nodejs/lib/thrift/web_server.js +++ b/lib/nodejs/lib/thrift/web_server.js @@ -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 + @@ -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 @@ -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; @@ -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 { @@ -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, ""); socket.write( "HTTP/1.1 101 Switching Protocols\r\n" + "Upgrade: websocket\r\n" + @@ -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", ); @@ -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; From bbb579519de7b85e6fb1140732548415b74d4200 Mon Sep 17 00:00:00 2001 From: Jens Geyer Date: Sun, 12 Apr 2026 17:44:59 +0200 Subject: [PATCH 2/2] Extract sanitizeHeader() for CRLF stripping Client: nodejs Co-Authored-By: Claude Opus 4.6 --- lib/nodejs/lib/thrift/web_server.js | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/lib/nodejs/lib/thrift/web_server.js b/lib/nodejs/lib/thrift/web_server.js index ddd13cd60d8..ad3a34c5c34 100644 --- a/lib/nodejs/lib/thrift/web_server.js +++ b/lib/nodejs/lib/thrift/web_server.js @@ -27,6 +27,10 @@ var log = require("./log"); var MultiplexedProcessor = require("./multiplexed_processor").MultiplexedProcessor; +function sanitizeHeader(value) { + return (value || "").replace(/[\r\n]/g, ""); +} + var TBufferedTransport = require("./buffered_transport"); var TBinaryProtocol = require("./binary_protocol"); var InputBufferUnderrunError = require("./input_buffer_underrun_error"); @@ -566,10 +570,9 @@ 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, ""); + var origin = sanitizeHeader(request.headers.origin); + var host = sanitizeHeader(request.headers.host); + var reqUrl = sanitizeHeader(request.url); socket.write( "HTTP/1.1 101 Switching Protocols\r\n" + "Upgrade: websocket\r\n" +