Skip to content

Commit be2898b

Browse files
mcollinaaduh95
authored andcommitted
http2: avoid overflowing ALTSVC buffers
Http2Session::AltSvc allocated origin and field-value buffers at their exact lengths but requested V8 to append a null terminator. This wrote one byte past heap-backed buffers. nghttp2_submit_altsvc() consumes explicit lengths and makes its own terminated copies. Write only the string contents. Assisted-by: OpenAI Codex Signed-off-by: Matteo Collina <hello@matteocollina.com> PR-URL: nodejs-private/node-private#1035 Reviewed-By: Tim Perry <pimterry@gmail.com> CVE-ID: CVE-2026-123828
1 parent 988d73f commit be2898b

2 files changed

Lines changed: 27 additions & 19 deletions

File tree

‎src/node_http2.cc‎

Lines changed: 2 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -3371,13 +3371,8 @@ void Http2Session::AltSvc(const FunctionCallbackInfo<Value>& args) {
33713371

33723372
MaybeStackBuffer<uint8_t> origin(origin_len);
33733373
MaybeStackBuffer<uint8_t> value(value_len);
3374-
origin_str->WriteOneByteV2(env->isolate(),
3375-
0,
3376-
origin_len,
3377-
*origin,
3378-
String::WriteFlags::kNullTerminate);
3379-
value_str->WriteOneByteV2(
3380-
env->isolate(), 0, value_len, *value, String::WriteFlags::kNullTerminate);
3374+
origin_str->WriteOneByteV2(env->isolate(), 0, origin_len, *origin);
3375+
value_str->WriteOneByteV2(env->isolate(), 0, value_len, *value);
33813376

33823377
session->AltSvc(id, *origin, origin_len, *value, value_len);
33833378
}

‎test/parallel/test-http2-altsvc.js‎

Lines changed: 25 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,9 @@ const assert = require('assert');
88
const http2 = require('http2');
99
const Countdown = require('../common/countdown');
1010

11+
const longOrigin = `https://${'a'.repeat(1040)}`;
12+
const longAlt = `h2=":${'8'.repeat(1042)}"`;
13+
1114
const server = http2.createServer();
1215
server.on('stream', common.mustCall((stream) => {
1316
stream.session.altsvc('h2=":8000"', stream.id);
@@ -22,6 +25,8 @@ server.on('session', common.mustCall((session) => {
2225
session.altsvc('h2=":8000"', 'https://example.org:8111/this');
2326
session.altsvc('h2=":8000"', new URL('https://example.org:8111/this'));
2427
session.altsvc('h2=":8000"', { origin: 'https://example.org:8111' });
28+
session.altsvc('h2=":8001"', longOrigin);
29+
session.altsvc(longAlt, 'https://example.org:8112');
2530

2631
// Won't error, but won't send anything because the stream does not exist
2732
session.altsvc('h2=":8000"', 3);
@@ -104,25 +109,33 @@ server.on('session', common.mustCall((session) => {
104109
server.listen(0, common.mustCall(() => {
105110
const client = http2.connect(`http://localhost:${server.address().port}`);
106111

107-
const countdown = new Countdown(4, () => {
112+
const countdown = new Countdown(6, () => {
108113
client.close();
109114
server.close();
110115
});
111116

112117
client.on('altsvc', common.mustCall((alt, origin, stream) => {
113-
assert.strictEqual(alt, 'h2=":8000"');
114-
switch (stream) {
115-
case 0:
116-
assert.strictEqual(origin, 'https://example.org:8111');
117-
break;
118-
case 1:
119-
assert.strictEqual(origin, '');
120-
break;
121-
default:
122-
assert.fail('should not happen');
118+
if (alt === 'h2=":8001"') {
119+
assert.strictEqual(origin, longOrigin);
120+
assert.strictEqual(stream, 0);
121+
} else if (alt === longAlt) {
122+
assert.strictEqual(origin, 'https://example.org:8112');
123+
assert.strictEqual(stream, 0);
124+
} else {
125+
assert.strictEqual(alt, 'h2=":8000"');
126+
switch (stream) {
127+
case 0:
128+
assert.strictEqual(origin, 'https://example.org:8111');
129+
break;
130+
case 1:
131+
assert.strictEqual(origin, '');
132+
break;
133+
default:
134+
assert.fail('should not happen');
135+
}
123136
}
124137
countdown.dec();
125-
}, 4));
138+
}, 6));
126139

127140
const req = client.request();
128141
req.resume();

0 commit comments

Comments
 (0)