镜像站点 · 本页由第三方 GitHub 只读镜像提供,非 GitHub 官方站点,不接受任何登录或凭据输入。前往 github.com
Skip to content

Commit 9183db6

Browse files
mcollinaaduh95
authored andcommitted
http2: retain streams during pending writes
Native file responses create write requests without a JS reference to the Http2Stream wrapper. Resetting a stream while its socket write is pending could let GC delete the stream before WriteWrap::OnDone(). Keep a strong stream reference with each outgoing DATA buffer until the underlying write completes. Assisted-by: codex:gpt-5.6-sol Signed-off-by: Matteo Collina <hello@matteocollina.com> PR-URL: nodejs-private/node-private#1040 Refs: https://hackerone.com/reports/4150891 Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com> Reviewed-By: Ulises Gascón <ulisesgascongonzalez@gmail.com> CVE-ID: CVE-2026-123874
1 parent 8232390 commit 9183db6

3 files changed

Lines changed: 106 additions & 5 deletions

File tree

‎src/node_http2.cc‎

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2173,15 +2173,19 @@ int Http2Session::OnSendData(
21732173
if (write.buf.len <= length) {
21742174
// This write does not suffice by itself, so we can consume it completely.
21752175
length -= write.buf.len;
2176+
write.stream = stream;
21762177
session->PushOutgoingBuffer(std::move(write));
21772178
stream->queue_.pop();
21782179
continue;
21792180
}
21802181

21812182
// Slice off `length` bytes of the first write in the queue. Retain the
21822183
// originating request until the slice is no longer used by the socket.
2183-
session->PushOutgoingBuffer(NgHttp2StreamWrite{
2184-
nullptr, write.buffer_owner, uv_buf_init(write.buf.base, length)});
2184+
session->PushOutgoingBuffer(
2185+
NgHttp2StreamWrite{nullptr,
2186+
write.buffer_owner,
2187+
stream,
2188+
uv_buf_init(write.buf.base, length)});
21852189
write.buf.base += length;
21862190
write.buf.len -= length;
21872191
break;
@@ -2279,8 +2283,7 @@ void Http2Session::OnStreamRead(ssize_t nread, const uv_buf_t& buf_) {
22792283

22802284
bool Http2Session::HasWritesOnSocketForStream(Http2Stream* stream) {
22812285
for (const NgHttp2StreamWrite& wr : outgoing_buffers_) {
2282-
if (wr.req_wrap && WriteWrap::FromObject(wr.req_wrap)->stream() == stream)
2283-
return true;
2286+
if (wr.stream.get() == stream) return true;
22842287
}
22852288
return false;
22862289
}
@@ -3571,6 +3574,7 @@ void NgHttp2StreamWrite::MemoryInfo(MemoryTracker* tracker) const {
35713574
if (req_wrap)
35723575
tracker->TrackField("req_wrap", req_wrap);
35733576
if (buffer_owner) tracker->TrackField("buffer_owner", buffer_owner);
3577+
if (stream) tracker->TrackField("stream", stream);
35743578
tracker->TrackField("buf", buf);
35753579
}
35763580

‎src/node_http2.h‎

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -156,11 +156,14 @@ struct Http2RcBufferPointerTraits {
156156
using Http2Headers = NgHeaders<Http2HeadersTraits>;
157157
using Http2RcBufferPointer = NgRcBufPointer<Http2RcBufferPointerTraits>;
158158

159+
class Http2Stream;
160+
159161
struct NgHttp2StreamWrite : public MemoryRetainer {
160162
// Completed once all buffers from the request have been consumed.
161163
BaseObjectPtr<AsyncWrap> req_wrap;
162164
// Retains the request's buffers without completing it.
163165
BaseObjectPtr<AsyncWrap> buffer_owner;
166+
BaseObjectPtr<Http2Stream> stream;
164167
uv_buf_t buf;
165168

166169
inline explicit NgHttp2StreamWrite(uv_buf_t buf_) : buf(buf_) {}
@@ -170,6 +173,14 @@ struct NgHttp2StreamWrite : public MemoryRetainer {
170173
: req_wrap(std::move(req_wrap)),
171174
buffer_owner(std::move(buffer_owner)),
172175
buf(buf_) {}
176+
inline NgHttp2StreamWrite(BaseObjectPtr<AsyncWrap> req_wrap,
177+
BaseObjectPtr<AsyncWrap> buffer_owner,
178+
BaseObjectPtr<Http2Stream> stream,
179+
uv_buf_t buf_)
180+
: req_wrap(std::move(req_wrap)),
181+
buffer_owner(std::move(buffer_owner)),
182+
stream(std::move(stream)),
183+
buf(buf_) {}
173184

174185
void MemoryInfo(MemoryTracker* tracker) const override;
175186
SET_MEMORY_INFO_NAME(NgHttp2StreamWrite)
@@ -182,7 +193,6 @@ typedef uint32_t(*get_setting)(nghttp2_session* session,
182193
class Http2Ping;
183194
class Http2Session;
184195
class Http2Settings;
185-
class Http2Stream;
186196
class Origins;
187197

188198
// This scope should be present when any call into nghttp2 that may schedule
Lines changed: 87 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,87 @@
1+
// Flags: --expose-gc
2+
'use strict';
3+
4+
const common = require('../common');
5+
if (!common.hasCrypto)
6+
common.skip('missing crypto');
7+
8+
const assert = require('assert');
9+
const fs = require('fs');
10+
const http2 = require('http2');
11+
const { duplexPair } = require('stream');
12+
const { setImmediate } = require('timers/promises');
13+
const tmpdir = require('../common/tmpdir');
14+
15+
tmpdir.refresh();
16+
const filename = tmpdir.resolve('http2-file');
17+
fs.writeFileSync(filename, Buffer.alloc(1024 * 1024));
18+
19+
async function test(useFD) {
20+
const [clientSide, serverSide] = duplexPair();
21+
let reset = false;
22+
let pendingWrite;
23+
const originalWrite = serverSide._write;
24+
serverSide._write = common.mustCallAtLeast(function(buf, enc, cb) {
25+
originalWrite.call(this, buf, enc, common.mustCall(async () => {
26+
// Deliver the data, but allow the client to reset the stream before
27+
// completing the corresponding native write.
28+
await setImmediate();
29+
if (reset) {
30+
assert.strictEqual(pendingWrite, undefined);
31+
pendingWrite = cb;
32+
} else {
33+
cb();
34+
}
35+
}));
36+
});
37+
38+
const server = http2.createServer();
39+
const fd = useFD ? fs.openSync(filename, 'r') : undefined;
40+
let streamClosed;
41+
const closed = new Promise((resolve) => { streamClosed = resolve; });
42+
server.on('stream', common.mustCall((stream) => {
43+
stream.on('close', common.mustCall(streamClosed));
44+
if (useFD)
45+
stream.respondWithFD(fd);
46+
else
47+
stream.respondWithFile(filename);
48+
}));
49+
server.emit('connection', serverSide);
50+
51+
const client = http2.connect('http://localhost', {
52+
createConnection: () => clientSide,
53+
settings: { initialWindowSize: 1024 * 1024 },
54+
});
55+
client.on('connect', common.mustCall(() => {
56+
client.setLocalWindowSize(1024 * 1024);
57+
}));
58+
const req = client.request();
59+
req.once('data', common.mustCall(() => {
60+
reset = true;
61+
req.close(http2.constants.NGHTTP2_CANCEL);
62+
}));
63+
req.resume();
64+
65+
await closed;
66+
// Let deferred native stream cleanup and unpipe callbacks run, then collect
67+
// the wrapper while its file write is still awaiting completion.
68+
for (let i = 0; i < 10; i++) {
69+
await setImmediate();
70+
global.gc();
71+
}
72+
assert.strictEqual(typeof pendingWrite, 'function');
73+
reset = false;
74+
pendingWrite();
75+
pendingWrite = undefined;
76+
await setImmediate();
77+
78+
client.destroy();
79+
server.close();
80+
if (useFD)
81+
fs.closeSync(fd);
82+
}
83+
84+
(async () => {
85+
await test(false);
86+
await test(true);
87+
})().then(common.mustCall());

0 commit comments

Comments
 (0)