Skip to content
Permalink
Browse files

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.

PR-URL: #29122
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
  • Loading branch information...
addaleax authored and targos committed Aug 12, 2019
1 parent 4570ed1 commit a21a1c007bdd501e0de090d8165ab5c4aeeb1034
Showing with 85 additions and 0 deletions.
  1. +4 −0 src/node_http2.cc
  2. +2 −0 src/node_http2.h
  3. +79 −0 test/parallel/test-http2-reset-flood.js
@@ -1017,6 +1017,10 @@ int Http2Session::OnInvalidFrame(nghttp2_session* handle,
Http2Session* session = static_cast<Http2Session*>(user_data);

Debug(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) ||
@@ -1010,6 +1010,8 @@ class Http2Session : public AsyncWrap, public StreamListener {
// 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);
@@ -0,0 +1,79 @@
'use strict';
const common = require('../common');
if (!common.hasCrypto)
common.skip('missing crypto');

const http2 = require('http2');
const net = require('net');
const { Worker, parentPort } = require('worker_threads');

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

if (process.env.HAS_STARTED_WORKER) {
const server = http2.createServer();
server.on('stream', (stream) => {
stream.respond({
'content-type': 'text/plain',
':status': 200
});
stream.end('Hello, world!\n');
});
server.listen(0, () => parentPort.postMessage(server.address().port));
return;
}

process.env.HAS_STARTED_WORKER = 1;
const worker = new Worker(__filename).on('message', 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;

function writeRequests() {
for (let i = 1; !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])]));
}
}

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

0 comments on commit a21a1c0

Please sign in to comment.
You can’t perform that action at this time.