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

Commit 9bce421

Browse files
mcollinaaduh95
authored andcommitted
http2: release headers when a stream is removed
When a peer sends more header fields than `maxHeaderListPairs`, the stream is reset and the fields it had already accepted stay in `current_headers_`. Each of them holds nghttp2 buffers. `RemoveStream()` adjusted the session memory accounting but kept the headers. A destroyed stream that still has a write on the socket is not detached and can be collected after its session. The session destructor only detaches the streams it still knows about, so the nghttp2 memory held by the removed stream is still allocated and `CHECK_EQ(current_nghttp2_memory_, 0)` aborts the process. A client can trigger this with a single connection. Clear the headers in `RemoveStream()`, while the session is still alive, and in the session destructor for the streams that are still registered, since `Detach()` does not free a stream that is referenced elsewhere. Refs: https://hackerone.com/reports/4150893 Signed-off-by: Matteo Collina <hello@matteocollina.com> PR-URL: nodejs-private/node-private#1026 Reviewed-By: Robert Nagy <ronagy@icloud.com> Reviewed-By: Tim Perry <pimterry@gmail.com> CVE-ID: CVE-2026-123863
1 parent ea159e8 commit 9bce421

2 files changed

Lines changed: 95 additions & 0 deletions

File tree

‎src/node_http2.cc‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -631,6 +631,9 @@ Http2Session::~Http2Session() {
631631
// Ensure that all `Http2Stream` instances and the memory they hold
632632
// on to are destroyed before the nghttp2 session is.
633633
for (const auto& [id, stream] : streams_) {
634+
// Detach() only frees the stream if nothing else holds a strong
635+
// reference to it.
636+
stream->ClearHeaders();
634637
stream->Detach();
635638
}
636639
streams_.clear();
@@ -940,6 +943,11 @@ BaseObjectPtr<Http2Stream> Http2Session::RemoveStream(int32_t id) {
940943
stream = FindStream(id);
941944
if (stream) {
942945
streams_.erase(id);
946+
// The stream can outlive the session, for example while one of its writes
947+
// is still on the socket. Headers that were not passed to JS, such as an
948+
// oversized block that caused the stream to be reset, hold nghttp2 memory
949+
// that has to be released before the session is.
950+
stream->ClearHeaders();
943951
if (stream->current_headers_length_ > 0) {
944952
DecrementCurrentSessionMemory(stream->current_headers_length_);
945953
stream->current_headers_length_ = 0;
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+
// A stream that is reset because of an oversized header block keeps the
9+
// headers it had already accepted. If the stream is destroyed while one of
10+
// its writes is still on the socket, it outlives the session, and the session
11+
// destructor used to abort because that memory was still allocated.
12+
13+
const http2 = require('http2');
14+
const net = require('net');
15+
const { setTimeout: sleep } = require('timers/promises');
16+
17+
// Enough response data to fill the socket buffers on loopback.
18+
const kStreams = 80;
19+
20+
function frame(type, flags, id, payload = Buffer.alloc(0)) {
21+
const header = Buffer.alloc(9);
22+
header.writeUIntBE(payload.length, 0, 3);
23+
header[3] = type;
24+
header[4] = flags;
25+
header.writeUInt32BE(id, 5);
26+
return Buffer.concat([header, payload]);
27+
}
28+
29+
// :method POST, :scheme http, :path /, :authority localhost
30+
const requestHeaders = Buffer.from([
31+
0x83, 0x86, 0x84, 0x01, 0x09, ...Buffer.from('localhost'),
32+
]);
33+
34+
// More trailer fields than the default maxHeaderListPairs (128).
35+
const trailers = Buffer.concat(Array.from({ length: 200 }, (_, i) => {
36+
const name = `x-${i}`;
37+
return Buffer.from([0x00, name.length, ...Buffer.from(name), 0x01, 0x76]);
38+
}));
39+
40+
const server = http2.createServer(common.mustCall((req, res) => {
41+
req.on('error', () => {});
42+
res.on('error', () => {});
43+
// Respond before the request has ended, with more data than the client
44+
// reads, so that the response stays in a pending socket write.
45+
res.end(Buffer.alloc(100 * 1024));
46+
}, kStreams));
47+
48+
server.on('session', common.mustCall((session) => {
49+
session.on('error', () => {});
50+
session.on('close', common.mustCall(async () => {
51+
server.close();
52+
// Collect the session before the streams that are still referenced by
53+
// their pending writes.
54+
for (let i = 0; i < 10; i++) {
55+
globalThis.gc();
56+
await sleep(10);
57+
}
58+
}));
59+
}));
60+
61+
server.listen(0, common.mustCall(async () => {
62+
const client = net.connect(server.address().port);
63+
client.on('error', () => {});
64+
client.pause();
65+
66+
const settings = Buffer.from([0x00, 0x04, 0x7f, 0xff, 0xff, 0xff]);
67+
const windowUpdate = Buffer.alloc(4);
68+
windowUpdate.writeUInt32BE(0x7fffffff - 65535);
69+
const requests = [];
70+
for (let i = 0; i < kStreams; i++)
71+
requests.push(frame(1, 0x4, 1 + 2 * i, requestHeaders));
72+
client.write(Buffer.concat([
73+
Buffer.from('PRI * HTTP/2.0\r\n\r\nSM\r\n\r\n'),
74+
frame(4, 0, 0, settings),
75+
frame(8, 0, 0, windowUpdate),
76+
...requests,
77+
]));
78+
await sleep(common.platformTimeout(500));
79+
80+
const blocks = [];
81+
for (let i = 0; i < kStreams; i++)
82+
blocks.push(frame(1, 0x5, 1 + 2 * i, trailers));
83+
client.write(Buffer.concat(blocks));
84+
await sleep(common.platformTimeout(200));
85+
86+
client.resetAndDestroy();
87+
}));

0 commit comments

Comments
 (0)