diff options
author | Anna Henningsen <anna@addaleax.net> | 2021-03-23 17:48:41 +0300 |
---|---|---|
committer | Ruy Adorno <ruyadorno@hotmail.com> | 2021-03-30 03:17:18 +0300 |
commit | 1c043272ea775621156bfa4e78718108803b3315 (patch) | |
tree | 177934d65f180c0476aaa78e9f50b17f4ed40b96 | |
parent | a5bf7de6eb1acd1d1c10a4c8ad1331626ee23703 (diff) |
http2: treat non-EOF empty frames like other invalid frames
Use the existing mechanism that we have to keep track of invalid frames
for treating this specific kind of invalid frame.
The commit that originally introduced this check was 695e38be69a780417,
which was supposed to proected against CVE-2019-9518, which in turn
was specifically about a *flood* of empty data frames. While these are
still invalid frames either way, it makes sense to be forgiving here
and just treat them like other invalid frames, i.e. to allow a small
(configurable) number of them.
Fixes: https://github.com/nodejs/node/issues/37849
PR-URL: https://github.com/nodejs/node/pull/37875
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
-rw-r--r-- | src/node_http2.cc | 6 | ||||
-rw-r--r-- | test/fixtures/emptyframe.http2 | bin | 0 -> 4233 bytes | |||
-rw-r--r-- | test/parallel/test-http2-empty-frame-without-eof.js | 39 |
3 files changed, 44 insertions, 1 deletions
diff --git a/src/node_http2.cc b/src/node_http2.cc index ccff6dc4ca6..004bc8df22a 100644 --- a/src/node_http2.cc +++ b/src/node_http2.cc @@ -1285,7 +1285,11 @@ int Http2Session::HandleDataFrame(const nghttp2_frame* frame) { frame->hd.flags & NGHTTP2_FLAG_END_STREAM) { stream->EmitRead(UV_EOF); } else if (frame->hd.length == 0) { - return 1; // Consider 0-length frame without END_STREAM an error. + if (invalid_frame_count_++ > js_fields_->max_invalid_frames) { + Debug(this, "rejecting empty-frame-without-END_STREAM flood\n"); + // Consider a flood of 0-length frames without END_STREAM an error. + return 1; + } } return 0; } diff --git a/test/fixtures/emptyframe.http2 b/test/fixtures/emptyframe.http2 Binary files differnew file mode 100644 index 00000000000..c4a095c4334 --- /dev/null +++ b/test/fixtures/emptyframe.http2 diff --git a/test/parallel/test-http2-empty-frame-without-eof.js b/test/parallel/test-http2-empty-frame-without-eof.js new file mode 100644 index 00000000000..02da78d940a --- /dev/null +++ b/test/parallel/test-http2-empty-frame-without-eof.js @@ -0,0 +1,39 @@ +'use strict'; +const common = require('../common'); +if (!common.hasCrypto) + common.skip('missing crypto'); +const { readSync } = require('../common/fixtures'); +const net = require('net'); +const http2 = require('http2'); +const { once } = require('events'); + +async function main() { + const blobWithEmptyFrame = readSync('emptyframe.http2'); + const server = net.createServer((socket) => { + socket.end(blobWithEmptyFrame); + }).listen(0); + await once(server, 'listening'); + + for (const maxSessionInvalidFrames of [0, 2]) { + const client = http2.connect(`http://localhost:${server.address().port}`, { + maxSessionInvalidFrames + }); + const stream = client.request({ + ':method': 'GET', + ':path': '/' + }); + if (maxSessionInvalidFrames) { + stream.on('error', common.mustNotCall()); + client.on('error', common.mustNotCall()); + } else { + stream.on('error', common.mustCall()); + client.on('error', common.mustCall()); + } + stream.resume(); + await once(stream, 'end'); + client.close(); + } + server.close(); +} + +main().then(common.mustCall()); |