Skip to content

Commit 11b4e2c

Browse files
addaleaxBethGriggs
authored andcommittedAug 15, 2019
http2: limit number of rejected stream openings
Limit the number of streams that are rejected upon creation. Since each such rejection is associated with an `NGHTTP2_ENHANCE_YOUR_CALM` error that should tell the peer to not open any more streams, continuing to open streams should be read as a sign of a misbehaving peer. The limit is currently set to 100 but could be changed or made configurable. This is intended to mitigate CVE-2019-9514. Backport-PR-URL: #29124 PR-URL: #29122 Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
1 parent 7de642b commit 11b4e2c

File tree

3 files changed

+19
-5
lines changed

3 files changed

+19
-5
lines changed
 

Diff for: ‎src/node_http2.cc

+9-4
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,8 @@
44
#include "node_http2.h"
55
#include "node_http2_state.h"
66
#include "node_perf.h"
7+
#include "node_revert.h"
8+
#include "util-inl.h"
79

810
#include <algorithm>
911

@@ -970,15 +972,18 @@ inline int Http2Session::OnBeginHeadersCallback(nghttp2_session* handle,
970972
if (LIKELY(session->CanAddStream())) {
971973
new Http2Stream(session, id, frame->headers.cat);
972974
} else {
975+
if (session->rejected_stream_count_++ > 100 &&
976+
!IsReverted(SECURITY_REVERT_CVE_2019_9514)) {
977+
return NGHTTP2_ERR_CALLBACK_FAILURE;
978+
}
973979
// Too many concurrent streams being opened
974980
nghttp2_submit_rst_stream(**session, NGHTTP2_FLAG_NONE, id,
975981
NGHTTP2_ENHANCE_YOUR_CALM);
976982
return NGHTTP2_ERR_TEMPORAL_CALLBACK_FAILURE;
977983
}
978-
} else {
979-
// If the stream has already been destroyed, ignore.
980-
if (stream->IsDestroyed())
981-
return 0;
984+
985+
session->rejected_stream_count_ = 0;
986+
} else if (!stream->IsDestroyed()) {
982987
stream->StartHeaders(frame->headers.cat);
983988
}
984989
return 0;

Diff for: ‎src/node_http2.h

+5
Original file line numberDiff line numberDiff line change
@@ -1071,6 +1071,11 @@ class Http2Session : public AsyncWrap {
10711071
std::vector<nghttp2_stream_write> outgoing_buffers_;
10721072
std::vector<uint8_t> outgoing_storage_;
10731073
std::vector<int32_t> pending_rst_streams_;
1074+
// Count streams that have been rejected while being opened. Exceeding a fixed
1075+
// limit will result in the session being destroyed, as an indication of a
1076+
// misbehaving peer. This counter is reset once new streams are being
1077+
// accepted again.
1078+
int32_t rejected_stream_count_ = 0;
10741079

10751080
void CopyDataIntoOutgoing(const uint8_t* src, size_t src_length);
10761081
void ClearOutgoing(int status);

Diff for: ‎src/node_revert.h

+5-1
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,11 @@
1616
namespace node {
1717

1818
#define SECURITY_REVERSIONS(XX) \
19-
XX(CVE_2018_12116, "CVE-2018-12116", "HTTP request splitting")
19+
XX(CVE_2018_12116, "CVE-2018-12116", "HTTP request splitting") \
20+
XX(CVE_2019_9514, "CVE-2019-9514", "HTTP/2 Reset Flood") \
21+
// XX(CVE_2016_PEND, "CVE-2016-PEND", "Vulnerability Title")
22+
// TODO(addaleax): Remove all of the above before Node.js 13 as the comment
23+
// at the start of the file indicates.
2024

2125
enum reversion {
2226
#define V(code, ...) SECURITY_REVERT_##code,

0 commit comments

Comments
 (0)
Please sign in to comment.