Skip to content

[v26.x backport] http2: avoid uaf while receiving and sending rst_stream - #65264

Open
mcollina wants to merge 2 commits into
nodejs:v26.x-stagingfrom
mcollina:backport-64166-v26.x-staging
Open

[v26.x backport] http2: avoid uaf while receiving and sending rst_stream#65264
mcollina wants to merge 2 commits into
nodejs:v26.x-stagingfrom
mcollina:backport-64166-v26.x-staging

Conversation

@mcollina

@mcollina mcollina commented Aug 13, 2026

Copy link
Copy Markdown
Member

Backport of #64166 to v26.x-staging.

Original PR: http2: avoid uaf while receiving and sending rst_stream
Fixes: #64113

Backport note

The upstream fix relies on the http2 JS close/destroy refactor from PR #63249 (http2: error for incomplete reads on RST, auto-drain, deprecate aborted), which is semver-major and cannot be backported to a release branch. On v26.x (which lacks that refactor) the C++ backport alone stalls test-http2-many-writes-and-destroy.js (graceful close never completes — the server's GOAWAY is not flushed because the SendPendingData() guard returns busy during nghttp2_session_mem_recv()).

So this backport includes a second commit that drops the send guard. The primary UAF protection (the RST_STREAM / Destroy / Close deferrals) remains intact, and the full http2 suite passes.

Eusgor and others added 2 commits August 8, 2026 15:51
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: nodejs#64113
Signed-off-by: Evgeniy Gorbanev <gorbanev.es@gmail.com>
PR-URL: nodejs#64166
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Tim Perry <pimterry@gmail.com>
Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com>
The http2 UAF backport (avoid uaf while receiving and sending
rst_stream) added a guard in SendPendingData() that returns busy while
added a guard in SendPendingData() that returns busy while mem_recv is
processing incoming data. On v26.x this stalls a graceful close: the
server's GOAWAY (submitted by server.close()) is not flushed because
sends during receive are blocked, so the client never completes its
graceful close and both sockets stay open.

The upstream fix relies on the http2 JS close/destroy refactor (PR
nodejs#63249), which is semver-major and cannot be backported to v26.x. Drop
the send guard instead; the RST_STREAM/Destroy/Close deferrals that are
the primary UAF protection remain intact.

Fixes the hang in test-http2-many-writes-and-destroy.js.

Signed-off-by: Matteo Collina <hello@matteocollina.com>
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

Review requested:

  • @nodejs/http2
  • @nodejs/net

@nodejs-github-bot nodejs-github-bot added c++ Issues and PRs that require attention from people who are familiar with C++. http2 Issues or PRs related to the http2 subsystem. needs-ci PRs that need a full CI run. v26.x Issues that can be reproduced on v26.x or PRs targeting the v26.x-staging branch. labels Aug 13, 2026
@mcollina
mcollina requested review from aduh95 and pimterry August 13, 2026 14:33
@aduh95 aduh95 changed the title http2: avoid uaf while receiving and sending rst_stream [v26.x backport] http2: avoid uaf while receiving and sending rst_stream Aug 13, 2026
@aduh95

aduh95 commented Aug 13, 2026

Copy link
Copy Markdown
Contributor

116 tests failed on the FIPS jobs, that's expected and already fixed on the staging branch. Closing and reopening to fix that

@aduh95 aduh95 closed this Aug 13, 2026
@aduh95 aduh95 reopened this Aug 13, 2026
@codecov

codecov Bot commented Aug 13, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 78.94737% with 12 lines in your changes missing coverage. Please review.
✅ Project coverage is 90.29%. Comparing base (54929af) to head (9b55403).
⚠️ Report is 324 commits behind head on v26.x-staging.

Files with missing lines Patch % Lines
src/node_http2.cc 77.35% 5 Missing and 7 partials ⚠️
Additional details and impacted files
@@                Coverage Diff                @@
##           v26.x-staging   #65264      +/-   ##
=================================================
+ Coverage          90.19%   90.29%   +0.09%     
=================================================
  Files                737      729       -8     
  Lines             240370   242763    +2393     
  Branches           45490    46920    +1430     
=================================================
+ Hits              216797   219191    +2394     
+ Misses             15107    15024      -83     
- Partials            8466     8548      +82     
Files with missing lines Coverage Δ
src/node_http2.h 91.86% <100.00%> (+0.24%) ⬆️
src/node_http2.cc 81.78% <77.35%> (-0.55%) ⬇️

... and 235 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@mcollina mcollina added the request-ci Add this label to start a Jenkins CI on a PR. label Aug 13, 2026
@github-actions github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Aug 13, 2026
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

c++ Issues and PRs that require attention from people who are familiar with C++. http2 Issues or PRs related to the http2 subsystem. needs-ci PRs that need a full CI run. v26.x Issues that can be reproduced on v26.x or PRs targeting the v26.x-staging branch.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants