Skip to content

Commit 7576d37

Browse files
kumarakfoxxyz
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 PR-URL: nodejs#39622 Refs: nodejs#39423 Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Beth Griggs <[email protected]>
1 parent 2429b7f commit 7576d37

File tree

1 file changed

+12
-12
lines changed

1 file changed

+12
-12
lines changed

src/node_http2.cc

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -2119,21 +2119,21 @@ void Http2Stream::SubmitRstStream(const uint32_t code) {
21192119
CHECK(!this->is_destroyed());
21202120
code_ = code;
21212121

2122-
// If RST_STREAM frame is received and stream is not writable
2123-
// because it is busy reading data, don't try force purging it.
2124-
// Instead add the stream to pending stream list and process
2125-
// the pending data when it is safe to do so. This is to avoid
2126-
// double free error due to unwanted behavior of nghttp2.
2127-
// Ref:https://github.com/nodejs/node/issues/38964
2128-
2129-
// Add stream to the pending list if it is received with scope
2122+
auto is_stream_cancel = [](const uint32_t code) {
2123+
return code == NGHTTP2_CANCEL;
2124+
};
2125+
2126+
// If RST_STREAM frame is received with error code NGHTTP2_CANCEL,
2127+
// add it to the pending list and don't force purge the data. It is
2128+
// to avoids the double free error due to unwanted behavior of nghttp2.
2129+
2130+
// Add stream to the pending list only if it is received with scope
21302131
// below in the stack. The pending list may not get processed
21312132
// if RST_STREAM received is not in scope and added to the list
21322133
// causing endpoint to hang.
2133-
if (session_->is_in_scope() &&
2134-
!is_writable() && is_reading()) {
2135-
session_->AddPendingRstStream(id_);
2136-
return;
2134+
if (session_->is_in_scope() && is_stream_cancel(code)) {
2135+
session_->AddPendingRstStream(id_);
2136+
return;
21372137
}
21382138

21392139

0 commit comments

Comments
 (0)