Skip to content

Commit

Permalink
http2: update handling of rst_stream with error code NGHTTP2_CANCEL
Browse files Browse the repository at this point in the history
The PR updates the handling of rst_stream frames and adds all streams
to the pending list on receiving rst frames with the error code
NGHTTP2_CANCEL.

The changes will remove dependency on the stream state that may allow
bypassing the checks in certain cases. I think a better solution is to
delay streams in all cases if rst_stream is received for the cancel
events.

The rst_stream frames can be received for protocol/connection error as
well it should be handled immediately. Adding streams to the pending
list in such cases may cause errors.

CVE-ID: CVE-2021-22930
Refs: https://nvd.nist.gov/vuln/detail/CVE-2021-22930
Backport-PR-URL: #39659
PR-URL: #39622
Refs: #39423
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Beth Griggs <bgriggs@redhat.com>
  • Loading branch information
kumarak authored and BethGriggs committed Aug 9, 2021
1 parent f2f7a45 commit 2008c97
Showing 1 changed file with 10 additions and 9 deletions.
19 changes: 10 additions & 9 deletions src/node_http2.cc
Original file line number Diff line number Diff line change
Expand Up @@ -2151,18 +2151,19 @@ void Http2Stream::SubmitRstStream(const uint32_t code) {
CHECK(!this->IsDestroyed());
code_ = code;

// If RST_STREAM frame is received and stream is not writable
// because it is busy reading data, don't try force purging it.
// Instead add the stream to pending stream list and process
// the pending data when it is safe to do so. This is to avoid
// double free error due to unwanted behavior of nghttp2.
// Ref:https://github.com/nodejs/node/issues/38964

// Add stream to the pending list if it is received with scope
auto is_stream_cancel = [](const uint32_t code) {
return code == NGHTTP2_CANCEL;
};

// If RST_STREAM frame is received with error code NGHTTP2_CANCEL,
// add it to the pending list and don't force purge the data. It is
// to avoids the double free error due to unwanted behavior of nghttp2.

// Add stream to the pending list only if it is received with scope
// below in the stack. The pending list may not get processed
// if RST_STREAM received is not in scope and added to the list
// causing endpoint to hang.
if (session_->is_in_scope() && IsReading()) {
if (session_->is_in_scope() && is_stream_cancel(code)) {
session_->AddPendingRstStream(id_);
return;
}
Expand Down

0 comments on commit 2008c97

Please sign in to comment.