Skip to content

Commit 2008c97

Browse files
kumarakBethGriggs
authored andcommitted
http2: update handling of rst_stream with error code NGHTTP2_CANCEL
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>
1 parent f2f7a45 commit 2008c97

File tree

1 file changed

+10
-9
lines changed

1 file changed

+10
-9
lines changed

src/node_http2.cc

Lines changed: 10 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -2151,18 +2151,19 @@ void Http2Stream::SubmitRstStream(const uint32_t code) {
21512151
CHECK(!this->IsDestroyed());
21522152
code_ = code;
21532153

2154-
// If RST_STREAM frame is received and stream is not writable
2155-
// because it is busy reading data, don't try force purging it.
2156-
// Instead add the stream to pending stream list and process
2157-
// the pending data when it is safe to do so. This is to avoid
2158-
// double free error due to unwanted behavior of nghttp2.
2159-
// Ref:https://github.com/nodejs/node/issues/38964
2160-
2161-
// Add stream to the pending list if it is received with scope
2154+
auto is_stream_cancel = [](const uint32_t code) {
2155+
return code == NGHTTP2_CANCEL;
2156+
};
2157+
2158+
// If RST_STREAM frame is received with error code NGHTTP2_CANCEL,
2159+
// add it to the pending list and don't force purge the data. It is
2160+
// to avoids the double free error due to unwanted behavior of nghttp2.
2161+
2162+
// Add stream to the pending list only if it is received with scope
21622163
// below in the stack. The pending list may not get processed
21632164
// if RST_STREAM received is not in scope and added to the list
21642165
// causing endpoint to hang.
2165-
if (session_->is_in_scope() && IsReading()) {
2166+
if (session_->is_in_scope() && is_stream_cancel(code)) {
21662167
session_->AddPendingRstStream(id_);
21672168
return;
21682169
}

0 commit comments

Comments
 (0)