Skip to content

Commit f66898b

Browse files
committed
http2: fix out-of-bounds write in altsvc frame buffers
Signed-off-by: nashit hayyat <nashit@bugqore.com>
1 parent e53d87a commit f66898b

2 files changed

Lines changed: 59 additions & 7 deletions

File tree

src/node_http2.cc

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

34043404
MaybeStackBuffer<uint8_t> origin(origin_len);
34053405
MaybeStackBuffer<uint8_t> value(value_len);
3406-
origin_str->WriteOneByteV2(env->isolate(),
3407-
0,
3408-
origin_len,
3409-
*origin,
3410-
String::WriteFlags::kNullTerminate);
3411-
value_str->WriteOneByteV2(
3412-
env->isolate(), 0, value_len, *value, String::WriteFlags::kNullTerminate);
3406+
origin_str->WriteOneByteV2(env->isolate(), 0, origin_len, *origin);
3407+
value_str->WriteOneByteV2(env->isolate(), 0, value_len, *value);
34133408

34143409
session->AltSvc(id, *origin, origin_len, *value, value_len);
34153410
}
Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,57 @@
1+
'use strict';
2+
3+
// Regression test for a one-byte out-of-bounds write in Http2Session::AltSvc.
4+
// The native handler allocated origin/value buffers sized to the string length
5+
// but wrote them with a null terminator, so an origin or alt value longer than
6+
// the inline stack buffer (1024 bytes) overflowed the heap allocation by one
7+
// byte. Exercise both paths with values above that threshold and confirm the
8+
// frames round-trip intact.
9+
10+
const common = require('../common');
11+
if (!common.hasCrypto)
12+
common.skip('missing crypto');
13+
14+
const assert = require('assert');
15+
const http2 = require('http2');
16+
const Countdown = require('../common/countdown');
17+
18+
// alt is limited to a quoted-string; padding is well past the 1024-byte inline
19+
// buffer so the value is heap-allocated at its exact length.
20+
const largeAlt = `h2=":8000"; ma=${'0'.repeat(2000)}`;
21+
const largeOrigin = `https://${'a'.repeat(1200)}.example.org`;
22+
23+
const server = http2.createServer();
24+
server.on('stream', common.mustCall((stream) => {
25+
// origin is empty here, so this exercises the value (alt) buffer.
26+
stream.session.altsvc(largeAlt, stream.id);
27+
stream.respond();
28+
stream.end('ok');
29+
}));
30+
server.on('session', common.mustCall((session) => {
31+
// stream id 0 with a long origin exercises the origin buffer.
32+
session.altsvc('h2=":8000"', largeOrigin);
33+
}));
34+
35+
server.listen(0, common.mustCall(() => {
36+
const client = http2.connect(`http://localhost:${server.address().port}`);
37+
38+
const countdown = new Countdown(2, () => {
39+
client.close();
40+
server.close();
41+
});
42+
43+
client.on('altsvc', common.mustCall((alt, origin, stream) => {
44+
if (stream === 0) {
45+
assert.strictEqual(alt, 'h2=":8000"');
46+
assert.strictEqual(origin, new URL(largeOrigin).origin);
47+
} else {
48+
assert.strictEqual(alt, largeAlt);
49+
assert.strictEqual(origin, '');
50+
}
51+
countdown.dec();
52+
}, 2));
53+
54+
const req = client.request();
55+
req.resume();
56+
req.on('close', common.mustCall());
57+
}));

0 commit comments

Comments
 (0)