Skip to content

http2: avoid uaf while receiving and sending rst_stream #64166

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
Eusgor wants to merge 1 commit into
base: main
Choose a base branch
from
+16 −0
Open

http2: avoid uaf while receiving and sending rst_stream #64166

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 14 additions & 0 deletions src/node_http2.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -958,11 +958,13 @@ void Http2Session::ConsumeHTTP2Data() {
nghttp2_session_want_read(session_.get()));
set_receive_paused(false);
custom_recv_error_code_ = nullptr;
set_receiving();
ssize_t ret =
nghttp2_session_mem_recv(session_.get(),
reinterpret_cast<uint8_t*>(stream_buf_.base) +
stream_buf_offset_,
read_len);
set_receiving(false);
CHECK_NE(ret, NGHTTP2_ERR_NOMEM);
CHECK_IMPLIES(custom_recv_error_code_ != nullptr, ret < 0);

Expand Down Expand Up @@ -2534,6 +2536,18 @@ void Http2Stream::SubmitRstStream(const uint32_t code) {
return code == NGHTTP2_CANCEL;
};

// Do not call `nghttp2_session_mem_send()` while nghttp2 is processing
// incoming data. Sending may close the stream and free nghttp2 state
// that is still in use by `nghttp2_session_mem_recv()`.
if (session_->is_receiving() && available_outbound_length_ == 0) {

@pimterry pimterry Jul 1, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think available_outbound_length_ == 0 still leaves the exact same UAF issue for the pending-writes case. E.g. if you write any data during a receive callback and then reset, this check would be false, and we'll miss this fix but hit the old UAF behaviour regardless.

I haven't tested this but looks very plausible, let me know if I've missed something.

If you move & invert (>0) the check into the inner if (so we always reset here in all receive cases, but we defer the reset for both cancel codes and pending-writes) then that'd cover this case as well.

if (is_stream_cancel(code)) {
session_->AddPendingRstStream(id_);
return;
}
FlushRstStream();
return;
}

// 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.
Expand Down
2 changes: 2 additions & 0 deletions src/node_http2.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,7 @@ constexpr int kSessionStateSending = 0x10;
constexpr int kSessionStateWriteInProgress = 0x20;
constexpr int kSessionStateReadingStopped = 0x40;
constexpr int kSessionStateReceivePaused = 0x80;
constexpr int kSessionStateReceiving = 0x100;

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

#undef IS_FLAG

Expand Down
Loading