Skip to content
Closed
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
http2: consider 0-length non-end DATA frames an error
This is intended to mitigate CVE-2019-9518.

Backport-PR-URL: #29123
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
commit 17357d37a9eba4ac1cbafe8628bf12cc900bc642
12 changes: 8 additions & 4 deletions src/node_http2.cc
Original file line number Diff line number Diff line change
Expand Up @@ -980,8 +980,7 @@ int Http2Session::OnFrameReceive(nghttp2_session* handle,
frame->hd.type);
switch (frame->hd.type) {
case NGHTTP2_DATA:
session->HandleDataFrame(frame);
break;
return session->HandleDataFrame(frame);
case NGHTTP2_PUSH_PROMISE:
// Intentional fall-through, handled just like headers frames
case NGHTTP2_HEADERS:
Expand Down Expand Up @@ -1398,13 +1397,18 @@ void Http2Session::HandlePriorityFrame(const nghttp2_frame* frame) {
// Called by OnFrameReceived when a complete DATA frame has been received.
// If we know that this was the last DATA frame (because the END_STREAM flag
// is set), then we'll terminate the readable side of the StreamBase.
void Http2Session::HandleDataFrame(const nghttp2_frame* frame) {
int Http2Session::HandleDataFrame(const nghttp2_frame* frame) {
int32_t id = GetFrameID(frame);
Debug(this, "handling data frame for stream %d", id);
Http2Stream* stream = FindStream(id);

if (!stream->IsDestroyed() && frame->hd.flags & NGHTTP2_FLAG_END_STREAM)
if (!stream->IsDestroyed() && frame->hd.flags & NGHTTP2_FLAG_END_STREAM) {
stream->EmitRead(UV_EOF);
} else if (frame->hd.length == 0 &&
!IsReverted(SECURITY_REVERT_CVE_2019_9518)) {
return 1; // Consider 0-length frame without END_STREAM an error.
}
return 0;
}


Expand Down
2 changes: 1 addition & 1 deletion src/node_http2.h
Original file line number Diff line number Diff line change
Expand Up @@ -889,7 +889,7 @@ class Http2Session : public AsyncWrap, public StreamListener {
size_t maxPayloadLen);

// Frame Handler
void HandleDataFrame(const nghttp2_frame* frame);
int HandleDataFrame(const nghttp2_frame* frame);
void HandleGoawayFrame(const nghttp2_frame* frame);
void HandleHeadersFrame(const nghttp2_frame* frame);
void HandlePriorityFrame(const nghttp2_frame* frame);
Expand Down
1 change: 1 addition & 0 deletions src/node_revert.h
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ namespace node {
#define SECURITY_REVERSIONS(XX) \
XX(CVE_2019_9514, "CVE-2019-9514", "HTTP/2 Reset Flood") \
XX(CVE_2019_9516, "CVE-2019-9516", "HTTP/2 0-Length Headers Leak") \
XX(CVE_2019_9518, "CVE-2019-9518", "HTTP/2 Empty DATA Frame Flooding") \
// XX(CVE_2016_PEND, "CVE-2016-PEND", "Vulnerability Title")
// TODO(addaleax): Remove all of the above before Node.js 13 as the comment
// at the start of the file indicates.
Expand Down