Skip to content

Commit

Permalink
http2: limit number of invalid incoming frames
Browse files Browse the repository at this point in the history
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: #29124
PR-URL: #29122
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
  • Loading branch information
addaleax authored and BethGriggs committed Aug 15, 2019
1 parent 11b4e2c commit ac28a62
Show file tree
Hide file tree
Showing 3 changed files with 100 additions and 0 deletions.
4 changes: 4 additions & 0 deletions src/node_http2.cc
Expand Up @@ -1070,6 +1070,10 @@ inline int Http2Session::OnInvalidFrame(nghttp2_session* handle,

DEBUG_HTTP2SESSION2(session, "invalid frame received, code: %d",
lib_error_code);
if (session->invalid_frame_count_++ > 1000 &&
!IsReverted(SECURITY_REVERT_CVE_2019_9514)) {
return 1;
}

// If the error is fatal or if error code is ERR_STREAM_CLOSED... emit error
if (nghttp2_is_fatal(lib_error_code) ||
Expand Down
2 changes: 2 additions & 0 deletions src/node_http2.h
Expand Up @@ -1076,6 +1076,8 @@ class Http2Session : public AsyncWrap {
// misbehaving peer. This counter is reset once new streams are being
// accepted again.
int32_t rejected_stream_count_ = 0;
// Also use the invalid frame count as a measure for rejecting input frames.
int32_t invalid_frame_count_ = 0;

void CopyDataIntoOutgoing(const uint8_t* src, size_t src_length);
void ClearOutgoing(int status);
Expand Down
94 changes: 94 additions & 0 deletions test/parallel/test-http2-reset-flood.js
@@ -0,0 +1,94 @@
'use strict';
const common = require('../common');
if (!common.hasCrypto)
common.skip('missing crypto');

const child_process = require('child_process');
const http2 = require('http2');
const net = require('net');

// Verify that creating a number of invalid HTTP/2 streams will eventually
// result in the peer closing the session.
// This test uses separate processes for client and server to avoid
// the two event loops intermixing, as we are writing in a busy loop here.

if (process.argv[2] === 'child') {
const server = http2.createServer();
server.on('stream', (stream) => {
stream.respond({
'content-type': 'text/plain',
':status': 200
});
stream.end('Hello, world!\n');
});
server.listen(0, () => {
process.stdout.write(`${server.address().port}`);
});
return;
}

const child = child_process.spawn(process.execPath, [__filename, 'child'], {
stdio: ['inherit', 'pipe', 'inherit']
});
child.stdout.on('data', common.mustCall((port) => {
const h2header = Buffer.alloc(9);
const conn = net.connect(+port);

conn.write('PRI * HTTP/2.0\r\n\r\nSM\r\n\r\n');

h2header[3] = 4; // Send a settings frame.
conn.write(Buffer.from(h2header));

let inbuf = Buffer.alloc(0);
let state = 'settingsHeader';
let settingsFrameLength;
conn.on('data', (chunk) => {
inbuf = Buffer.concat([inbuf, chunk]);
switch (state) {
case 'settingsHeader':
if (inbuf.length < 9) return;
settingsFrameLength = inbuf.readIntBE(0, 3);
inbuf = inbuf.slice(9);
state = 'readingSettings';
// Fallthrough
case 'readingSettings':
if (inbuf.length < settingsFrameLength) return;
inbuf = inbuf.slice(settingsFrameLength);
h2header[3] = 4; // Send a settings ACK.
h2header[4] = 1;
conn.write(Buffer.from(h2header));
state = 'ignoreInput';
writeRequests();
}
});

let gotError = false;

let i = 1;
function writeRequests() {
for (; !gotError; i += 2) {
h2header[3] = 1; // HEADERS
h2header[4] = 0x5; // END_HEADERS|END_STREAM
h2header.writeIntBE(1, 0, 3); // Length: 1
h2header.writeIntBE(i, 5, 4); // Stream ID
// 0x88 = :status: 200
conn.write(Buffer.concat([h2header, Buffer.from([0x88])]));

if (i % 1000 === 1) {
// Delay writing a bit so we get the chance to actually observe
// an error. This is not necessary on master/v12.x, because there
// conn.write() can fail directly when writing to a connection
// that was closed by the remote peer due to
// https://github.com/libuv/libuv/commit/ee24ce900e5714c950b248da2b
i += 2;
return setImmediate(writeRequests);
}
}
}

conn.once('error', common.mustCall(() => {
gotError = true;
child.kill();
conn.destroy();
}));
}));

0 comments on commit ac28a62

Please sign in to comment.