From 115b684bee3a04ede974edf4d0c078c0009a24e0 Mon Sep 17 00:00:00 2001 From: Matteo Collina Date: Sun, 29 Mar 2026 17:42:54 +0200 Subject: [PATCH] http: fix response queue poisoning in http.Agent Attach a data guard listener on idle keepAlive sockets in the freeSockets pool. If unsolicited data arrives while the socket is idle, destroy it immediately to prevent response queue poisoning. Refs: https://hackerone.com/reports/3582376 PR-URL: https://github.com/nodejs-private/node-private/pull/846 Reviewed-By: Antoine du Hamel CVE-ID: CVE-2026-48931 origin: https://github.com/nodejs/node/commit/0a22d40180cb796e0d68e94c1a7a8a05a8f47c10 bug: https://nodejs.org/en/blog/vulnerability/june-2026-security-releases#http-response-queue-poisoning-via-toctou-race-condition-in-httpagent-cve-2026-48931---low Gbp-Pq: Topic sec Gbp-Pq: Name CVE-2026-48931.patch --- lib/_http_agent.js | 13 +++ .../test-http-agent-free-socket-data-guard.js | 90 +++++++++++++++++++ test/parallel/test-http-agent-keepalive.js | 3 +- 3 files changed, 105 insertions(+), 1 deletion(-) create mode 100644 test/parallel/test-http-agent-free-socket-data-guard.js diff --git a/lib/_http_agent.js b/lib/_http_agent.js index 04c75218c..27e418c00 100644 --- a/lib/_http_agent.js +++ b/lib/_http_agent.js @@ -87,6 +87,16 @@ function freeSocketErrorListener(err) { socket.emit('agentRemove'); } +// Guard against unsolicited data arriving while a socket is idle in the +// freeSockets pool. When the HTTPParser is detached the data would sit +// in the TCP buffer and be silently consumed as the response for the +// *next* request that reuses the socket (response-queue poisoning). +// See: https://hackerone.com/reports/3582376 +function freeSocketDataGuard() { + debug('DATA on FREE socket - destroying poisoned socket'); + this.destroy(); +} + function Agent(options) { if (!(this instanceof Agent)) return new Agent(options); @@ -184,6 +194,8 @@ function Agent(options) { this.removeSocket(socket, options); socket.once('error', freeSocketErrorListener); + socket.on('data', freeSocketDataGuard); + socket.resume(); ArrayPrototypePush(freeSockets, socket); }); @@ -494,6 +506,7 @@ Agent.prototype.keepSocketAlive = function keepSocketAlive(socket) { Agent.prototype.reuseSocket = function reuseSocket(socket, req) { debug('have free socket'); socket.removeListener('error', freeSocketErrorListener); + socket.removeListener('data', freeSocketDataGuard); req.reusedSocket = true; socket.ref(); }; diff --git a/test/parallel/test-http-agent-free-socket-data-guard.js b/test/parallel/test-http-agent-free-socket-data-guard.js new file mode 100644 index 000000000..9c1a526aa --- /dev/null +++ b/test/parallel/test-http-agent-free-socket-data-guard.js @@ -0,0 +1,90 @@ +'use strict'; + +// Regression test for HackerOne report #3582376 +// HTTP Response Queue Poisoning via TOCTOU Race Condition in http.Agent +// +// When keepAlive is true, there is a window between a socket entering the +// freeSockets pool (parser detached) and being reassigned. If the server +// writes a full HTTP response during this window, it is consumed as the +// response for the *next* request — poisoning the response queue. +// +// The fix attaches a data guard listener + resume() on idle sockets so +// that unsolicited data causes the socket to be destroyed. + +const common = require('../common'); +const assert = require('assert'); +const http = require('http'); + +let serverSocket; + +const server = http.createServer(common.mustCall((req, res) => { + // Capture the raw socket on the first request + serverSocket ||= req.socket; + res.end(req.url); +}, 2)); // Expect request1 and request2 + +server.listen(0, common.mustCall(() => { + const agent = new http.Agent({ keepAlive: true }); + const options = { host: '127.0.0.1', port: server.address().port, agent }; + const name = agent.getName(options); + + // Step 1: Send request1 + const request1 = http.request({ ...options, path: '/request1' }); + request1.end(); + + request1.on('response', common.mustCall((response) => { + let body = ''; + response.setEncoding('utf8'); + response.on('data', (data) => { body += data; }); + response.on('end', common.mustCall(() => { + assert.strictEqual(body, '/request1'); + })); + })); + + request1.on('close', common.mustCall(() => { + // Use nextTick to ensure socket is in freeSockets + process.nextTick(common.mustCall(() => { + // Verify the socket is in the free pool with parser detached + assert.strictEqual(agent.freeSockets[name]?.length, 1); + const freeSocket = agent.freeSockets[name][0]; + assert.strictEqual(freeSocket.parser, null); + // With the fix, a data guard listener is attached + assert.strictEqual(freeSocket.listenerCount('data'), 1); + + // Step 2: Server injects a poisoned response while socket is idle + serverSocket.write( + 'HTTP/1.1 200 OK\r\n' + + 'X-Poisoned: true\r\n' + + 'Connection: keep-alive\r\n' + + 'Content-Length: 0\r\n' + + '\r\n' + ); + + // Step 3: Allow the event loop to poll I/O so the guard can fire. + // In a real attack, there is always time between the poison arriving + // and the next client request. setTimeout(0) runs after the I/O poll + // phase, giving the guard a chance to receive the poisoned data. + setTimeout(common.mustCall(() => { + // The guard should have destroyed the poisoned socket + assert.strictEqual(freeSocket.destroyed, true); + assert.strictEqual(agent.freeSockets[name], undefined); + + // Step 4: Send request2 — should get a fresh connection + const request2 = http.request({ ...options, path: '/request2' }); + request2.end(); + + request2.on('response', common.mustCall((response) => { + let body = ''; + response.setEncoding('utf8'); + response.on('data', (data) => { body += data; }); + response.on('end', common.mustCall(() => { + assert.strictEqual(response.headers['x-poisoned'], undefined); + assert.strictEqual(body, '/request2'); + agent.destroy(); + server.close(); + })); + })); + }), 50); + })); + })); +})); diff --git a/test/parallel/test-http-agent-keepalive.js b/test/parallel/test-http-agent-keepalive.js index f7424634d..e4f5c09de 100644 --- a/test/parallel/test-http-agent-keepalive.js +++ b/test/parallel/test-http-agent-keepalive.js @@ -149,7 +149,8 @@ server.listen(0, common.mustCall(() => { function checkListeners(socket) { const callback = common.mustCall(() => { if (!socket.destroyed) { - assert.strictEqual(socket.listenerCount('data'), 0); + // Sockets have freeSocketDataGuard while in the free pool. + assert.strictEqual(socket.listenerCount('data'), 1); assert.strictEqual(socket.listenerCount('drain'), 0); // Sockets have freeSocketErrorListener. assert.strictEqual(socket.listenerCount('error'), 1); -- 2.39.5