Skip to content

Commit bc0b538

Browse files
mcollinaaduh95
authored andcommitted
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: nodejs-private/node-private#846 Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com> CVE-ID: CVE-2026-48931
1 parent a67dd46 commit bc0b538

3 files changed

Lines changed: 105 additions & 1 deletion

File tree

‎lib/_http_agent.js‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -87,6 +87,16 @@ function freeSocketErrorListener(err) {
8787
socket.emit('agentRemove');
8888
}
8989

90+
// Guard against unsolicited data arriving while a socket is idle in the
91+
// freeSockets pool. When the HTTPParser is detached the data would sit
92+
// in the TCP buffer and be silently consumed as the response for the
93+
// *next* request that reuses the socket (response-queue poisoning).
94+
// See: https://hackerone.com/reports/3582376
95+
functionfreeSocketDataGuard(){
96+
debug('DATA on FREE socket - destroying poisoned socket');
97+
this.destroy();
98+
}
99+
90100
functionAgent(options){
91101
if(!(thisinstanceofAgent))
92102
returnnewAgent(options);
@@ -197,6 +207,8 @@ function Agent(options) {
197207
this.removeSocket(socket,options);
198208

199209
socket.once('error',freeSocketErrorListener);
210+
socket.on('data',freeSocketDataGuard);
211+
socket.resume();
200212
freeSockets.push(socket);
201213
});
202214

@@ -588,6 +600,7 @@ Agent.prototype.keepSocketAlive = function keepSocketAlive(socket) {
588600
Agent.prototype.reuseSocket=functionreuseSocket(socket,req){
589601
debug('have free socket');
590602
socket.removeListener('error',freeSocketErrorListener);
603+
socket.removeListener('data',freeSocketDataGuard);
591604
req.reusedSocket=true;
592605
socket.ref();
593606
};
Lines changed: 90 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,90 @@
1+
'use strict';
2+
3+
// Regression test for HackerOne report #3582376
4+
// HTTP Response Queue Poisoning via TOCTOU Race Condition in http.Agent
5+
//
6+
// When keepAlive is true, there is a window between a socket entering the
7+
// freeSockets pool (parser detached) and being reassigned. If the server
8+
// writes a full HTTP response during this window, it is consumed as the
9+
// response for the *next* request — poisoning the response queue.
10+
//
11+
// The fix attaches a data guard listener + resume() on idle sockets so
12+
// that unsolicited data causes the socket to be destroyed.
13+
14+
constcommon=require('../common');
15+
constassert=require('assert');
16+
consthttp=require('http');
17+
18+
letserverSocket;
19+
20+
constserver=http.createServer(common.mustCall((req,res)=>{
21+
// Capture the raw socket on the first request
22+
serverSocket||=req.socket;
23+
res.end(req.url);
24+
},2));// Expect request1 and request2
25+
26+
server.listen(0,common.mustCall(()=>{
27+
constagent=newhttp.Agent({keepAlive: true});
28+
constoptions={host: '127.0.0.1',port: server.address().port, agent };
29+
constname=agent.getName(options);
30+
31+
// Step 1: Send request1
32+
constrequest1=http.request({ ...options,path: '/request1'});
33+
request1.end();
34+
35+
request1.on('response',common.mustCall((response)=>{
36+
letbody='';
37+
response.setEncoding('utf8');
38+
response.on('data',(data)=>{body+=data;});
39+
response.on('end',common.mustCall(()=>{
40+
assert.strictEqual(body,'/request1');
41+
}));
42+
}));
43+
44+
request1.on('close',common.mustCall(()=>{
45+
// Use nextTick to ensure socket is in freeSockets
46+
process.nextTick(common.mustCall(()=>{
47+
// Verify the socket is in the free pool with parser detached
48+
assert.strictEqual(agent.freeSockets[name]?.length,1);
49+
constfreeSocket=agent.freeSockets[name][0];
50+
assert.strictEqual(freeSocket.parser,null);
51+
// With the fix, a data guard listener is attached
52+
assert.strictEqual(freeSocket.listenerCount('data'),1);
53+
54+
// Step 2: Server injects a poisoned response while socket is idle
55+
serverSocket.write(
56+
'HTTP/1.1 200 OK\r\n'+
57+
'X-Poisoned: true\r\n'+
58+
'Connection: keep-alive\r\n'+
59+
'Content-Length: 0\r\n'+
60+
'\r\n'
61+
);
62+
63+
// Step 3: Allow the event loop to poll I/O so the guard can fire.
64+
// In a real attack, there is always time between the poison arriving
65+
// and the next client request. setTimeout(0) runs after the I/O poll
66+
// phase, giving the guard a chance to receive the poisoned data.
67+
setTimeout(common.mustCall(()=>{
68+
// The guard should have destroyed the poisoned socket
69+
assert.strictEqual(freeSocket.destroyed,true);
70+
assert.strictEqual(agent.freeSockets[name],undefined);
71+
72+
// Step 4: Send request2 — should get a fresh connection
73+
constrequest2=http.request({ ...options,path: '/request2'});
74+
request2.end();
75+
76+
request2.on('response',common.mustCall((response)=>{
77+
letbody='';
78+
response.setEncoding('utf8');
79+
response.on('data',(data)=>{body+=data;});
80+
response.on('end',common.mustCall(()=>{
81+
assert.strictEqual(response.headers['x-poisoned'],undefined);
82+
assert.strictEqual(body,'/request2');
83+
agent.destroy();
84+
server.close();
85+
}));
86+
}));
87+
}),50);
88+
}));
89+
}));
90+
}));

‎test/parallel/test-http-agent-keepalive.js‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -149,7 +149,8 @@ server.listen(0, common.mustCall(() => {
149149
functioncheckListeners(socket){
150150
constcallback=common.mustCall(()=>{
151151
if(!socket.destroyed){
152-
assert.strictEqual(socket.listenerCount('data'),0);
152+
// Sockets have freeSocketDataGuard while in the free pool.
153+
assert.strictEqual(socket.listenerCount('data'),1);
153154
assert.strictEqual(socket.listenerCount('drain'),0);
154155
// Sockets have freeSocketErrorListener.
155156
assert.strictEqual(socket.listenerCount('error'),1);

0 commit comments

Comments
 (0)