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: #39622
Refs: #39423
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Beth Griggs <[email protected]>

Author: kumarak

Diff for: src/node_http2.cc

@@ -2196,21 +2196,21 @@ void Http2Stream::SubmitRstStream(const uint32_t code) {
   CHECK(!this->is_destroyed());
   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() &&
-      !is_writable() && is_reading()) {
-    session_->AddPendingRstStream(id_);
-    return;
+  if (session_->is_in_scope() && is_stream_cancel(code)) {
+      session_->AddPendingRstStream(id_);
+      return;
   }