Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

Commit 477461a

Browse filesBrowse files
addaleaxBethGriggs
authored andcommitted
http2: limit number of invalid incoming frames
Limit the number of invalid input frames, as they may be pointing towards a misbehaving peer. The limit is currently set to 1000 but could be changed or made configurable. This is intended to mitigate CVE-2019-9514. [This commit differs from the v12.x one due to the lack of libuv/libuv@ee24ce900e5714c950b248da2b. See the comment in the test for more details.] Backport-PR-URL: #29123 PR-URL: #29122 Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
1 parent 05dada4 commit 477461a
Copy full SHA for 477461a

File tree

Expand file treeCollapse file tree

3 files changed

+97
-0
lines changed
Open diff view settings
Filter options
Expand file treeCollapse file tree

3 files changed

+97
-0
lines changed
Open diff view settings
Collapse file

‎src/node_http2.cc‎

Copy file name to clipboardExpand all lines: src/node_http2.cc
+4Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1018,6 +1018,10 @@ int Http2Session::OnInvalidFrame(nghttp2_session* handle,
10181018
Http2Session* session = static_cast<Http2Session*>(user_data);
10191019

10201020
Debug(session, "invalid frame received, code: %d", lib_error_code);
1021+
if (session->invalid_frame_count_++ > 1000 &&
1022+
!IsReverted(SECURITY_REVERT_CVE_2019_9514)) {
1023+
return 1;
1024+
}
10211025

10221026
// If the error is fatal or if error code is ERR_STREAM_CLOSED... emit error
10231027
if (nghttp2_is_fatal(lib_error_code) ||
Collapse file

‎src/node_http2.h‎

Copy file name to clipboardExpand all lines: src/node_http2.h
+2Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1022,6 +1022,8 @@ class Http2Session : public AsyncWrap, public StreamListener {
10221022
// misbehaving peer. This counter is reset once new streams are being
10231023
// accepted again.
10241024
int32_t rejected_stream_count_ = 0;
1025+
// Also use the invalid frame count as a measure for rejecting input frames.
1026+
int32_t invalid_frame_count_ = 0;
10251027

10261028
void CopyDataIntoOutgoing(const uint8_t* src, size_t src_length);
10271029
void ClearOutgoing(int status);
Collapse file
+91Lines changed: 91 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,91 @@
1+
// Flags: --experimental-worker
2+
'use strict';
3+
const common = require('../common');
4+
if (!common.hasCrypto)
5+
common.skip('missing crypto');
6+
7+
const http2 = require('http2');
8+
const net = require('net');
9+
const { Worker, parentPort } = require('worker_threads');
10+
11+
// Verify that creating a number of invalid HTTP/2 streams will eventually
12+
// result in the peer closing the session.
13+
// This test uses separate threads for client and server to avoid
14+
// the two event loops intermixing, as we are writing in a busy loop here.
15+
16+
if (process.env.HAS_STARTED_WORKER) {
17+
const server = http2.createServer();
18+
server.on('stream', (stream) => {
19+
stream.respond({
20+
'content-type': 'text/plain',
21+
':status': 200
22+
});
23+
stream.end('Hello, world!\n');
24+
});
25+
server.listen(0, () => parentPort.postMessage(server.address().port));
26+
return;
27+
}
28+
29+
process.env.HAS_STARTED_WORKER = 1;
30+
const worker = new Worker(__filename).on('message', common.mustCall((port) => {
31+
const h2header = Buffer.alloc(9);
32+
const conn = net.connect(port);
33+
34+
conn.write('PRI * HTTP/2.0\r\n\r\nSM\r\n\r\n');
35+
36+
h2header[3] = 4; // Send a settings frame.
37+
conn.write(Buffer.from(h2header));
38+
39+
let inbuf = Buffer.alloc(0);
40+
let state = 'settingsHeader';
41+
let settingsFrameLength;
42+
conn.on('data', (chunk) => {
43+
inbuf = Buffer.concat([inbuf, chunk]);
44+
switch (state) {
45+
case 'settingsHeader':
46+
if (inbuf.length < 9) return;
47+
settingsFrameLength = inbuf.readIntBE(0, 3);
48+
inbuf = inbuf.slice(9);
49+
state = 'readingSettings';
50+
// Fallthrough
51+
case 'readingSettings':
52+
if (inbuf.length < settingsFrameLength) return;
53+
inbuf = inbuf.slice(settingsFrameLength);
54+
h2header[3] = 4; // Send a settings ACK.
55+
h2header[4] = 1;
56+
conn.write(Buffer.from(h2header));
57+
state = 'ignoreInput';
58+
writeRequests();
59+
}
60+
});
61+
62+
let gotError = false;
63+
64+
let i = 1;
65+
function writeRequests() {
66+
for (; !gotError; i += 2) {
67+
h2header[3] = 1; // HEADERS
68+
h2header[4] = 0x5; // END_HEADERS|END_STREAM
69+
h2header.writeIntBE(1, 0, 3); // Length: 1
70+
h2header.writeIntBE(i, 5, 4); // Stream ID
71+
// 0x88 = :status: 200
72+
conn.write(Buffer.concat([h2header, Buffer.from([0x88])]));
73+
74+
if (i % 1000 === 1) {
75+
// Delay writing a bit so we get the chance to actually observe
76+
// an error. This is not necessary on master/v12.x, because there
77+
// conn.write() can fail directly when writing to a connection
78+
// that was closed by the remote peer due to
79+
// https://github.com/libuv/libuv/commit/ee24ce900e5714c950b248da2b
80+
i += 2;
81+
return setImmediate(writeRequests);
82+
}
83+
}
84+
}
85+
86+
conn.once('error', common.mustCall(() => {
87+
gotError = true;
88+
worker.terminate();
89+
conn.destroy();
90+
}));
91+
}));

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.