Skip to content

Commit dec48cc

Browse files
committed
http2: avoid uaf while receiving and sending rst_stream
Mark the session as receiving around nghttp2_session_mem_recv() and defer RST_STREAM handling while receive is in progress. This prevents closing a stream while nghttp2 still processes it and avoids heap-use-after-free in nghttp2_session_mem_recv2(). Fixes: #64113 Signed-off-by: Evgeniy Gorbanev <gorbanev.es@gmail.com>
1 parent 8cb8312 commit dec48cc

2 files changed

Lines changed: 16 additions & 0 deletions

File tree

src/node_http2.cc

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -958,11 +958,13 @@ void Http2Session::ConsumeHTTP2Data() {
958958
nghttp2_session_want_read(session_.get()));
959959
set_receive_paused(false);
960960
custom_recv_error_code_ = nullptr;
961+
set_receiving();
961962
ssize_t ret =
962963
nghttp2_session_mem_recv(session_.get(),
963964
reinterpret_cast<uint8_t*>(stream_buf_.base) +
964965
stream_buf_offset_,
965966
read_len);
967+
set_receiving(false);
966968
CHECK_NE(ret, NGHTTP2_ERR_NOMEM);
967969
CHECK_IMPLIES(custom_recv_error_code_ != nullptr, ret < 0);
968970

@@ -2534,6 +2536,18 @@ void Http2Stream::SubmitRstStream(const uint32_t code) {
25342536
return code == NGHTTP2_CANCEL;
25352537
};
25362538

2539+
// Do not call `nghttp2_session_mem_send()` while nghttp2 is processing
2540+
// incoming data. Sending may close the stream and free nghttp2 state
2541+
// that is still in use by `nghttp2_session_mem_recv()`.
2542+
if (session_->is_receiving() && available_outbound_length_ == 0) {
2543+
if (is_stream_cancel(code)) {
2544+
session_->AddPendingRstStream(id_);
2545+
return;
2546+
}
2547+
FlushRstStream();
2548+
return;
2549+
}
2550+
25372551
// If RST_STREAM frame is received with error code NGHTTP2_CANCEL,
25382552
// add it to the pending list and don't force purge the data. It is
25392553
// to avoids the double free error due to unwanted behavior of nghttp2.

src/node_http2.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -77,6 +77,7 @@ constexpr int kSessionStateSending = 0x10;
7777
constexpr int kSessionStateWriteInProgress = 0x20;
7878
constexpr int kSessionStateReadingStopped = 0x40;
7979
constexpr int kSessionStateReceivePaused = 0x80;
80+
constexpr int kSessionStateReceiving = 0x100;
8081

8182
// The Padding Strategy determines the method by which extra padding is
8283
// selected for HEADERS and DATA frames. These are configurable via the
@@ -669,6 +670,7 @@ class Http2Session : public AsyncWrap,
669670
IS_FLAG(write_in_progress, kSessionStateWriteInProgress)
670671
IS_FLAG(reading_stopped, kSessionStateReadingStopped)
671672
IS_FLAG(receive_paused, kSessionStateReceivePaused)
673+
IS_FLAG(receiving, kSessionStateReceiving)
672674

673675
#undef IS_FLAG
674676

0 commit comments

Comments
 (0)