Skip to content

Commit 0683ee5

Browse files
jasnellRafaelGSS
authored andcommitted
http: ensure agent data guard works without I/O
Before a kept-alive socket is given to a request, now ask the OS whether anything is waiting to be read on it. If there is, the Agent destroys the socket and uses another one or opens a new connection. The check doesn't consume any bytes. Signed-off-by: James M Snell <jasnell@gmail.com> Assisted-by: Opencode PR-URL: nodejs-private/node-private#983 Fixes: https://hackerone.com/reports/4038361 Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com> CVE-ID: CVE-2026-105481
1 parent 8a9c54c commit 0683ee5

5 files changed

Lines changed: 213 additions & 3 deletions

File tree

‎lib/_http_agent.js‎

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -138,6 +138,18 @@ function installFreeSocketDataGuard(socket) {
138138
}
139139
}
140140

141+
// The data guard only runs when the event loop polls for I/O. Data that
142+
// reached the OS socket buffer after the last poll has not been seen by it
143+
// yet, so check for it directly before handing an idle socket to a request.
144+
function hasPendingRead(socket) {
145+
// A socket that is still connecting has nothing to read yet, and a pending
146+
// connect error must be reported to the request rather than swallowed here.
147+
if (socket.connecting) return false;
148+
const handle = socket._handle;
149+
return typeof handle?.hasPendingRead === 'function' &&
150+
handle.hasPendingRead() === true;
151+
}
152+
141153
function removeFreeSocketDataGuard(socket) {
142154
if (socket[kFreeSocketDataGuard]) {
143155
socket.removeListener('connect', socket[kFreeSocketDataGuard]);
@@ -214,6 +226,12 @@ function Agent(options) {
214226

215227
const requests = this.requests[name];
216228
if (requests?.length) {
229+
if (hasPendingRead(socket)) {
230+
// onClose() will create a new socket for the pending request.
231+
debug('PENDING DATA on released socket - destroying');
232+
socket.destroy();
233+
return;
234+
}
217235
const req = requests.shift();
218236
const reqAsyncRes = req[kRequestAsyncResource];
219237
if (reqAsyncRes) {
@@ -391,6 +409,13 @@ Agent.prototype.addRequest = function addRequest(req, options, port/* legacy */,
391409
socket = this.scheduling === 'fifo' ?
392410
freeSockets.shift() :
393411
freeSockets.pop();
412+
while (socket && hasPendingRead(socket)) {
413+
debug('PENDING DATA on FREE socket - destroying poisoned socket');
414+
socket.destroy();
415+
socket = this.scheduling === 'fifo' ?
416+
freeSockets.shift() :
417+
freeSockets.pop();
418+
}
394419
if (!freeSockets.length)
395420
delete this.freeSockets[name];
396421
}

‎lib/internal/tls/wrap.js‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -732,7 +732,7 @@ exports.TLSSocket = TLSSocket;
732732
const proxiedMethods = [
733733
'ref', 'unref', 'open', 'bind', 'listen', 'connect', 'bind6',
734734
'connect6', 'getsockname', 'getpeername', 'setNoDelay', 'setKeepAlive',
735-
'setSimultaneousAccepts', 'setBlocking',
735+
'setSimultaneousAccepts', 'setBlocking', 'hasPendingRead',
736736

737737
// PipeWrap
738738
'setPendingInstances',

‎src/stream_wrap.cc‎

Lines changed: 65 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -33,9 +33,9 @@
3333
#include "udp_wrap.h"
3434
#include "util-inl.h"
3535

36-
#include <cstring> // memcpy()
36+
#include <cerrno> // errno
3737
#include <climits> // INT_MAX
38-
38+
#include <cstring> // memcpy()
3939

4040
namespace node {
4141

@@ -137,6 +137,7 @@ void LibuvStreamWrap::RegisterExternalReferences(
137137
registry->Register(IsConstructCallCallback);
138138
registry->Register(GetWriteQueueSize);
139139
registry->Register(SetBlocking);
140+
registry->Register(HasPendingRead);
140141
StreamBase::RegisterExternalReferences(registry);
141142
}
142143

@@ -175,6 +176,7 @@ Local<FunctionTemplate> LibuvStreamWrap::GetConstructorTemplate(
175176
Local<FunctionTemplate>(),
176177
static_cast<PropertyAttribute>(ReadOnly | DontDelete));
177178
SetProtoMethod(isolate, tmpl, "setBlocking", SetBlocking);
179+
SetProtoMethod(isolate, tmpl, "hasPendingRead", HasPendingRead);
178180
StreamBase::AddMethods(env, tmpl);
179181
env->set_libuv_stream_wrap_ctor_template(tmpl);
180182
}
@@ -343,6 +345,67 @@ void LibuvStreamWrap::SetBlocking(const FunctionCallbackInfo<Value>& args) {
343345
args.GetReturnValue().Set(uv_stream_set_blocking(wrap->stream(), enable));
344346
}
345347

348+
// Returns true if a read on the underlying socket would complete right now,
349+
// i.e. the OS has data, EOF, or a socket error queued that libuv has not
350+
// delivered yet because the event loop has not polled for I/O since it
351+
// arrived. Nothing is consumed. Returns false when there is nothing pending
352+
// or when the handle type does not support the check.
353+
void LibuvStreamWrap::HasPendingRead(const FunctionCallbackInfo<Value>& args) {
354+
LibuvStreamWrap* wrap;
355+
ASSIGN_OR_RETURN_UNWRAP(&wrap, args.This());
356+
args.GetReturnValue().Set(false);
357+
358+
if (!wrap->IsAlive() || wrap->IsClosing()) return;
359+
360+
#ifdef _WIN32
361+
// Only TCP sockets can be probed with recv() on Windows.
362+
if (!wrap->is_tcp()) return;
363+
#else
364+
if (!wrap->is_tcp() && !wrap->is_named_pipe()) return;
365+
#endif
366+
367+
uv_os_fd_t fd;
368+
if (uv_fileno(reinterpret_cast<uv_handle_t*>(wrap->stream()), &fd) != 0)
369+
return;
370+
371+
char c;
372+
#ifdef _WIN32
373+
// libuv puts TCP sockets in non-blocking mode and reads with zero-byte
374+
// overlapped reads, so unread data stays in the kernel buffer.
375+
int r = recv(reinterpret_cast<SOCKET>(fd), &c, 1, MSG_PEEK);
376+
if (r == SOCKET_ERROR) {
377+
switch (WSAGetLastError()) {
378+
case WSAEWOULDBLOCK:
379+
case WSAENOTCONN:
380+
case WSAENOTSOCK:
381+
return;
382+
}
383+
}
384+
#else
385+
int flags = MSG_PEEK;
386+
#ifdef MSG_DONTWAIT
387+
flags |= MSG_DONTWAIT;
388+
#endif
389+
ssize_t r;
390+
do {
391+
r = recv(fd, &c, 1, flags);
392+
} while (r == -1 && errno == EINTR);
393+
if (r == -1) {
394+
switch (errno) {
395+
case EAGAIN:
396+
#if EWOULDBLOCK != EAGAIN
397+
case EWOULDBLOCK:
398+
#endif
399+
case ENOTCONN:
400+
case ENOTSOCK:
401+
return;
402+
}
403+
}
404+
#endif
405+
406+
args.GetReturnValue().Set(true);
407+
}
408+
346409
typedef SimpleShutdownWrap<ReqWrap<uv_shutdown_t>> LibuvShutdownWrap;
347410
typedef SimpleWriteWrap<ReqWrap<uv_write_t>> LibuvWriteWrap;
348411

‎src/stream_wrap.h‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -114,6 +114,7 @@ class LibuvStreamWrap : public HandleWrap, public StreamBase {
114114
static void GetWriteQueueSize(
115115
const v8::FunctionCallbackInfo<v8::Value>& info);
116116
static void SetBlocking(const v8::FunctionCallbackInfo<v8::Value>& args);
117+
static void HasPendingRead(const v8::FunctionCallbackInfo<v8::Value>& args);
117118

118119
// Callbacks for libuv
119120
void OnUvAlloc(size_t suggested_size, uv_buf_t* buf);
Lines changed: 121 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,121 @@
1+
'use strict';
2+
3+
// The free socket data guard in http.Agent only runs when the event loop
4+
// polls for I/O. If data reaches an idle socket's OS buffer and a request
5+
// takes that socket before the loop has polled, the guard never sees the data
6+
// and it would be parsed as the response to that request. The Agent must
7+
// check for pending data before handing a kept-alive socket to a request.
8+
9+
const common = require('../common');
10+
const assert = require('assert');
11+
const http = require('http');
12+
13+
const poison =
14+
'HTTP/1.1 200 OK\r\n' +
15+
'X-Poisoned: true\r\n' +
16+
'Connection: keep-alive\r\n' +
17+
'Content-Length: 0\r\n' +
18+
'\r\n';
19+
20+
// Keep the event loop from polling until the bytes are in the client's
21+
// socket buffer, so libuv cannot deliver them to the data guard first.
22+
function waitForPendingRead(socket) {
23+
const deadline = Date.now() + common.platformTimeout(5000);
24+
while (!socket._handle.hasPendingRead()) {
25+
assert.ok(Date.now() < deadline, 'timed out waiting for pending data');
26+
}
27+
}
28+
29+
function checkResponse(req, expectedBody, cb) {
30+
req.on('response', common.mustCall((res) => {
31+
let body = '';
32+
res.setEncoding('utf8');
33+
res.on('data', (chunk) => { body += chunk; });
34+
res.on('end', common.mustCall(() => {
35+
assert.strictEqual(res.headers['x-poisoned'], undefined);
36+
assert.strictEqual(body, expectedBody);
37+
cb?.(res);
38+
}));
39+
}));
40+
}
41+
42+
// A socket taken from the free pool.
43+
{
44+
let serverSocket;
45+
const server = http.createServer(common.mustCall((req, res) => {
46+
serverSocket ||= req.socket;
47+
res.end(req.url);
48+
}, 2));
49+
50+
server.listen(0, common.mustCall(() => {
51+
const agent = new http.Agent({ keepAlive: true });
52+
const options = { host: '127.0.0.1', port: server.address().port, agent };
53+
const name = agent.getName(options);
54+
55+
const request1 = http.request({ ...options, path: '/request1' });
56+
request1.end();
57+
checkResponse(request1, '/request1');
58+
59+
request1.on('close', common.mustCall(() => {
60+
process.nextTick(common.mustCall(() => {
61+
assert.strictEqual(agent.freeSockets[name]?.length, 1);
62+
const freeSocket = agent.freeSockets[name][0];
63+
64+
serverSocket.write(poison);
65+
waitForPendingRead(freeSocket);
66+
67+
// Same tick: the loop has not polled since the data arrived.
68+
const request2 = http.request({ ...options, path: '/request2' });
69+
request2.end();
70+
71+
assert.strictEqual(freeSocket.destroyed, true);
72+
assert.strictEqual(request2.reusedSocket, false);
73+
74+
checkResponse(request2, '/request2', common.mustCall(() => {
75+
agent.destroy();
76+
server.close();
77+
}));
78+
}));
79+
}));
80+
}));
81+
}
82+
83+
// A released socket handed directly to a queued request.
84+
{
85+
let serverSocket;
86+
const server = http.createServer(common.mustCall((req, res) => {
87+
serverSocket ||= req.socket;
88+
res.end(req.url);
89+
}, 2));
90+
91+
server.listen(0, common.mustCall(() => {
92+
const agent = new http.Agent({ keepAlive: true, maxSockets: 1 });
93+
const options = { host: '127.0.0.1', port: server.address().port, agent };
94+
const name = agent.getName(options);
95+
96+
const request1 = http.request({ ...options, path: '/request1' });
97+
request1.end();
98+
const request2 = http.request({ ...options, path: '/request2' });
99+
request2.end();
100+
assert.strictEqual(agent.requests[name]?.length, 1);
101+
102+
let socket1;
103+
request1.on('socket', common.mustCall((socket) => {
104+
socket1 = socket;
105+
}));
106+
107+
// The socket is released to the queued request on the next tick after
108+
// 'end', so the loop does not poll in between.
109+
checkResponse(request1, '/request1', common.mustCall(() => {
110+
serverSocket.write(poison);
111+
waitForPendingRead(socket1);
112+
}));
113+
114+
checkResponse(request2, '/request2', common.mustCall(() => {
115+
assert.strictEqual(socket1.destroyed, true);
116+
assert.strictEqual(request2.reusedSocket, false);
117+
agent.destroy();
118+
server.close();
119+
}));
120+
}));
121+
}

0 commit comments

Comments
 (0)