Skip to content

Commit cc7c11b

Browse files
mcollinaaduh95
authored andcommitted
http2: cap originSet size to prevent unbounded memory growth
A malicious HTTP/2 server can send repeated ORIGIN frames with unique origins, causing unbounded growth of the client-side originSet for the lifetime of the session. Cap the set at 128 entries; once full, new origins from ORIGIN frames are silently dropped. Refs: https://hackerone.com/reports/3676863 PR-URL: nodejs-private/node-private#855 Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com> CVE-ID: CVE-2026-48619
1 parent 8e75c73 commit cc7c11b

5 files changed

Lines changed: 141 additions & 1 deletion

File tree

doc/api/errors.md

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1732,6 +1732,17 @@ added: v15.14.0
17321732
The limit of acceptable invalid HTTP/2 protocol frames sent by the peer,
17331733
as specified through the `maxSessionInvalidFrames` option, has been exceeded.
17341734

1735+
<a id="ERR_HTTP2_TOO_MANY_ORIGINS"></a>
1736+
1737+
### `ERR_HTTP2_TOO_MANY_ORIGINS`
1738+
1739+
<!-- YAML
1740+
added: REPLACEME
1741+
-->
1742+
1743+
The number of uniq origin sent by the server has exceeded the value defined in
1744+
`options.maxOriginSetSize`.
1745+
17351746
<a id="ERR_HTTP2_TRAILERS_ALREADY_SENT"></a>
17361747

17371748
### `ERR_HTTP2_TRAILERS_ALREADY_SENT`

doc/api/http2.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3260,6 +3260,8 @@ changes:
32603260
This is similar to [`server.maxHeadersCount`][] or
32613261
[`request.maxHeadersCount`][] in the `node:http` module. The minimum value
32623262
is `1`. **Default:** `128`.
3263+
* `maxOriginSetSize` {number} Sets the maximum number of uniq origin the sever
3264+
can send via ORIGIN frames. **Default:** `128`.
32633265
* `maxOutstandingPings` {number} Sets the maximum number of outstanding,
32643266
unacknowledged pings. **Default:** `10`.
32653267
* `maxReservedRemoteStreams` {number} Sets the maximum number of reserved push

lib/internal/errors.js

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1323,6 +1323,8 @@ E('ERR_HTTP2_STREAM_SELF_DEPENDENCY',
13231323
E('ERR_HTTP2_TOO_MANY_CUSTOM_SETTINGS',
13241324
'Number of custom settings exceeds MAX_ADDITIONAL_SETTINGS', Error);
13251325
E('ERR_HTTP2_TOO_MANY_INVALID_FRAMES', 'Too many invalid HTTP/2 frames', Error);
1326+
E('ERR_HTTP2_TOO_MANY_ORIGINS',
1327+
'The server sent more ORIGIN frames than the allowed number of %s', Error);
13261328
E('ERR_HTTP2_TRAILERS_ALREADY_SENT',
13271329
'Trailing headers have already been sent', Error);
13281330
E('ERR_HTTP2_TRAILERS_NOT_READY',

lib/internal/http2/core.js

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -102,6 +102,7 @@ const {
102102
ERR_HTTP2_STREAM_ERROR,
103103
ERR_HTTP2_STREAM_SELF_DEPENDENCY,
104104
ERR_HTTP2_TOO_MANY_CUSTOM_SETTINGS,
105+
ERR_HTTP2_TOO_MANY_ORIGINS,
105106
ERR_HTTP2_TRAILERS_ALREADY_SENT,
106107
ERR_HTTP2_TRAILERS_NOT_READY,
107108
ERR_HTTP2_UNSUPPORTED_PROTOCOL,
@@ -255,6 +256,7 @@ const kInit = Symbol('init');
255256
const kInfoHeaders = Symbol('sent-info-headers');
256257
const kLocalSettings = Symbol('local-settings');
257258
const kNativeFields = Symbol('kNativeFields');
259+
const kMaxOriginSetSize = Symbol('max-ORIGIN-set-size');
258260
const kOptions = Symbol('options');
259261
const kOwner = owner_symbol;
260262
const kOrigin = Symbol('origin');
@@ -723,8 +725,13 @@ function onOrigin(origins) {
723725
if (!session.encrypted || session.destroyed)
724726
return undefined;
725727
const originSet = initOriginSet(session);
726-
for (let n = 0; n < origins.length; n++)
728+
for (let n = 0; n < origins.length; n++) {
729+
if (originSet.size >= session[kMaxOriginSetSize]) {
730+
session.destroy(new ERR_HTTP2_TOO_MANY_ORIGINS(session[kMaxOriginSetSize]));
731+
return;
732+
}
727733
originSet.add(origins[n]);
734+
}
728735
session.emit('origin', origins);
729736
}
730737

@@ -3579,6 +3586,13 @@ function connect(authority, options, listener) {
35793586
assertIsObject(options, 'options');
35803587
options = { ...options };
35813588

3589+
let { maxOriginSetSize } = options;
3590+
if (maxOriginSetSize != null) {
3591+
validateNumber(maxOriginSetSize, 'options.maxOriginSetSize', 0);
3592+
} else {
3593+
maxOriginSetSize = 128;
3594+
}
3595+
35823596
assertIsArray(options.remoteCustomSettings, 'options.remoteCustomSettings');
35833597
if (options.remoteCustomSettings) {
35843598
options.remoteCustomSettings = [ ...options.remoteCustomSettings ];
@@ -3634,6 +3648,7 @@ function connect(authority, options, listener) {
36343648

36353649
session[kAuthority] = `${options.servername || host}:${port}`;
36363650
session[kProtocol] = protocol;
3651+
session[kMaxOriginSetSize] = maxOriginSetSize;
36373652

36383653
if (typeof listener === 'function')
36393654
session.once('connect', listener);
Lines changed: 110 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,110 @@
1+
import {
2+
expectsError,
3+
hasCrypto,
4+
mustCall,
5+
mustNotCall,
6+
mustSucceed,
7+
skip,
8+
} from '../common/index.mjs';
9+
import * as fixtures from '../common/fixtures.mjs';
10+
import assert from 'node:assert';
11+
12+
if (!hasCrypto)
13+
skip('missing crypto');
14+
15+
const {
16+
createSecureServer,
17+
connect,
18+
} = await import('node:http2');
19+
20+
const key = fixtures.readKey('agent8-key.pem', 'binary');
21+
const cert = fixtures.readKey('agent8-cert.pem', 'binary');
22+
const ca = fixtures.readKey('fake-startcom-root-cert.pem', 'binary');
23+
24+
const server = createSecureServer({ key, cert });
25+
server.on('stream', (stream) => {
26+
stream.respond();
27+
stream.end('ok');
28+
});
29+
server.on('session', (session) => {
30+
let i = 0;
31+
const timer = setInterval(() => {
32+
try {
33+
session.origin(...Array.from({ length: 10 }, () => `https://o${i++}.example.com`));
34+
} catch {
35+
clearInterval(timer);
36+
}
37+
}, 10);
38+
39+
session.on('close', () => {
40+
clearInterval(timer);
41+
});
42+
});
43+
44+
await new Promise((resolve) => server.listen(0, resolve));
45+
46+
// Test fantasist values
47+
[Symbol(), '0', 1n, {}, [], true, false, /s/, () => {}].forEach((maxOriginSetSize) => {
48+
assert.throws(
49+
() => connect(`https://localhost:${server.address().port}`, { ca, maxOriginSetSize }),
50+
{ code: 'ERR_INVALID_ARG_TYPE' },
51+
);
52+
});
53+
[NaN, -1].forEach((maxOriginSetSize) => {
54+
assert.throws(
55+
() => connect(`https://localhost:${server.address().port}`, { ca, maxOriginSetSize }),
56+
{ code: 'ERR_OUT_OF_RANGE' },
57+
);
58+
});
59+
60+
await new Promise((resolve) => server.getConnections(mustSucceed((count) => {
61+
assert.strictEqual(count, 0);
62+
resolve();
63+
})));
64+
65+
// Test default value
66+
await new Promise((resolve) => {
67+
const client = connect(`https://localhost:${server.address().port}`, { ca });
68+
69+
client.on('origin', mustCall(12)); // Default value is 128, the first 12 frames should pass, the 13th one should error
70+
client.on('error', expectsError({
71+
code: 'ERR_HTTP2_TOO_MANY_ORIGINS',
72+
}));
73+
client.on('goaway', mustNotCall());
74+
client.on('close', resolve);
75+
76+
client.request().resume();
77+
});
78+
79+
// Test non-default values
80+
await Promise.all([-0, 9, 1.5].map((maxOriginSetSize) => new Promise((resolve) => {
81+
const client = connect(`https://localhost:${server.address().port}`, { ca, maxOriginSetSize });
82+
83+
client.on('origin', mustNotCall()); // The server send 10 origins on the first frame, that's already too many.
84+
client.on('error', expectsError({
85+
code: 'ERR_HTTP2_TOO_MANY_ORIGINS',
86+
}));
87+
client.on('goaway', mustNotCall());
88+
client.on('close', resolve);
89+
90+
client.request().resume();
91+
})));
92+
93+
94+
// Test values higher than the default value
95+
await Promise.all([512, Infinity].map((maxOriginSetSize) => new Promise((resolve) => {
96+
const client = connect(`https://localhost:${server.address().port}`, { ca, maxOriginSetSize });
97+
98+
client.on('origin', mustCall(() => {
99+
if (client.originSet.length > 128) {
100+
client.destroy();
101+
}
102+
}, 13));
103+
client.on('error', mustNotCall());
104+
client.on('goaway', mustNotCall());
105+
client.on('close', resolve);
106+
107+
client.request().resume();
108+
})));
109+
110+
server.close();

0 commit comments

Comments
 (0)