[v24.x] http2: avoid UAF while receiving and sending RST_STREAM - #65093
Open
mcollina wants to merge 103 commits into
Open
[v24.x] http2: avoid UAF while receiving and sending RST_STREAM#65093mcollina wants to merge 103 commits into
mcollina wants to merge 103 commits into
Conversation
Collaborator
|
Review requested:
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## v24.x-staging #65093 +/- ##
=================================================
+ Coverage 90.31% 90.33% +0.02%
=================================================
Files 711 714 +3
Lines 228425 231848 +3423
Branches 43160 43954 +794
=================================================
+ Hits 206293 209437 +3144
- Misses 14090 14156 +66
- Partials 8042 8255 +213
🚀 New features to boost your workflow:
|
Contributor
|
The change would need to first be backported to Node.js 26 (or wait until Node.js 27 is Current), see #64166 (comment) |
Fixes: nodejs#64214 Signed-off-by: y1d7ng <y1d7ng@yeah.net> PR-URL: nodejs#64227 Reviewed-By: Chemi Atlow <chemi@atlow.co.il> Reviewed-By: Claudio Wunder <cwunder@gnome.org>
Signed-off-by: ulofiai <monsterking@tutamail.com> PR-URL: nodejs#65095 Reviewed-By: Aviv Keller <me@aviv.sh> Reviewed-By: Stefan Stojanovic <stefan.stojanovic@janeasystems.com>
An expired timer can run before the first complete event loop iteration, disabling the histogram before it records any samples. Drive a known number of iterations with setImmediate before checking the histograms, and share the chain between resolution variants. Signed-off-by: Kamat, Trivikram <16024985+trivikr@users.noreply.github.com> Assisted-by: codex:gpt-5.6-sol PR-URL: nodejs#64728 Refs: https://github.com/nodejs/reliability/issues?q=sort%3Aupdated-desc%20test-performance-eventloopdelay Reviewed-By: Filip Skokan <panva.ip@gmail.com>
PR-URL: nodejs#65317 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Chemi Atlow <chemi@atlow.co.il>
Add return types for Blob methods and legacy Base64 helpers so doc-kit does not render them as `void`. Refs: nodejs/doc-kit#953 Signed-off-by: inoway46 <inoueyuya416@gmail.com> PR-URL: nodejs#65308 Refs: nodejs/doc-kit#953 Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
Three entries in the fs documentation described their return
value only in prose, or not at all, so doc-kit could not parse
a return type and fell back to `void`:
* `filehandle[Symbol.asyncDispose]()` and
`dir[Symbol.asyncDispose]()` both return a promise, matching
the existing `Returns: {Promise}` annotations on other async
dispose methods.
* `new fs.Utf8Stream([options])` is a constructor and returns
an instance of the class.
Verified at runtime and by rendering the page locally with
doc-kit.
Refs: nodejs/doc-kit#953
Signed-off-by: Chxxeton <43923165+Chxxeton@users.noreply.github.com>
PR-URL: nodejs#65307
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Aviv Keller <me@aviv.sh>
Reviewed-By: Ulises Gascón <ulisesgascongonzalez@gmail.com>
Fixes: nodejs#63852 Signed-off-by: Erik Demaine <edemaine@mit.edu> PR-URL: nodejs#63856 Reviewed-By: Stefan Stojanovic <stefan.stojanovic@janeasystems.com> Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
Signed-off-by: T <cookesan@users.noreply.github.com> PR-URL: nodejs#62941 Fixes: nodejs#61518 Reviewed-By: Aviv Keller <me@aviv.sh>
Signed-off-by: RafaelGSS <rafael.nunu@hotmail.com> PR-URL: nodejs#64699 Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com> Reviewed-By: Juan José Arboleda <soyjuanarbol@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
The file type of a directory entry returned by fs.readdir() or fs.opendir() is the type reported by the operating system's directory listing. Node.js falls back to lstat() only when the reported type is unknown, and some file systems may report a type that differs from what lstat() would return. Document this in the fs.Dirent class introduction, with fs.lstat() as the accurate alternative. Fixes: nodejs#30646 Signed-off-by: Julian Soreavis <julian.soreavis@gmail.com> PR-URL: nodejs#64532 Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Jason Zhang <xzha4350@gmail.com>
StringDecoder used v8::String::NewFromUtf8() for UTF-8, while
Buffer#toString() goes through StringBytes::Encode(), which has
simdutf-backed ASCII, Latin-1 and UTF-16 paths and only falls back
to NewFromUtf8() for input that contains invalid sequences. Route
the decoder through the same function, so streams with
setEncoding('utf8') and readline decode at the same speed as
Buffer#toString(). U+FFFD replacement is unchanged because invalid
input still ends up in NewFromUtf8(), and the ERR_STRING_TOO_LONG
check is kept explicit so over-long input fails as before.
benchmark/string_decoder/string-decoder.js (encoding=utf8) and a
readline-over-pipe workload improve by 2-3x for chunks >= 1 KiB;
64 KiB newline-delimited JSON round trips over child stdio improve
by ~30% on the reading side alone.
Signed-off-by: Shelley Vohr <shelley.vohr@gmail.com>
PR-URL: nodejs#65324
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: Gürgün Dayıoğlu <hey@gurgun.day>
Reviewed-By: Daniel Lemire <daniel@lemire.me>
Reviewed-By: James M Snell <jasnell@gmail.com>
An asynchronous transform can abort the pipeline while its final flush resolves to null. With no final batch yielded, the pipeline previously completed without observing the abort. Check the signal after transform iteration completes so the consumer rejects with the abort reason. Signed-off-by: Kamat, Trivikram <16024985+trivikr@users.noreply.github.com> Assisted-by: codex:gpt-5.6-sol PR-URL: nodejs#65346 Fixes: nodejs#65345 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
This reverts commit 8488e13. It was advised that the fix in question is broken since it will unref the handle multiple times. Signed-off-by: Mike McCready <66998419+MikeMcC399@users.noreply.github.com> PR-URL: nodejs#65387 Refs: nodejs#64227 Refs: nodejs#64214 Reviewed-By: Claudio Wunder <cwunder@gnome.org> Reviewed-By: Chemi Atlow <chemi@atlow.co.il> Reviewed-By: Robert Nagy <ronagy@icloud.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reject session.close() while generating a changeset or patchset. Closing the session at that point frees it while SQLite is still using it, causing a use-after-free. Signed-off-by: Kamat, Trivikram <16024985+trivikr@users.noreply.github.com> Assisted-by: codex:gpt-5.6-sol PR-URL: nodejs#65349 Fixes: nodejs#65347 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Stephen Belanger <admin@stephenbelanger.com>
getNearestParentPackageJSON() memoized its answer per file, so every module loaded still made one native call, and TraverseParent() builds several std::filesystem::path temporaries per directory level and serializes the whole package.json, which the JS side then usually discarded because it already had that package.json deserialized. The native traversal starts at the directory of the given path, so the answer only depends on that directory: key the memo by it (following NormalizePath()'s trailing-separator rule), so that all modules in a directory share one native call. When the permission model is enabled the traversal also depends on the read permissions in effect at call time, so that configuration keeps the per-file cache. Loading a 1000-module tree spread over ~240 directories goes from 1000 to 236 native calls. Signed-off-by: Shelley Vohr <shelley.vohr@gmail.com> PR-URL: nodejs#65326 Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Jordan Harband <ljharb@gmail.com>
Reject offsets outside the destination buffer before subtracting from its length in SlowWriteString. Normalize wrapper arguments once so validated values reach the native binding. Signed-off-by: Matteo Collina <hello@matteocollina.com> PR-URL: nodejs#65043 Reviewed-By: Robert Nagy <ronagy@icloud.com>
Signed-off-by: Lazizbek Ergashev <lazerg2@gmail.com> PR-URL: nodejs#65357 Fixes: nodejs#65355 Reviewed-By: Chemi Atlow <chemi@atlow.co.il> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
Signed-off-by: Vedant Kulkarni <gamemaker0042@gmail.com> PR-URL: nodejs#62117 Reviewed-By: Ethan Arrowood <ethan@arrowood.dev> Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
Normalize each stateless transform result before passing it to the next transform in a fused run. This ensures that subsequent transforms always receive Uint8Array[] batches in both synchronous and asynchronous pipelines. Signed-off-by: Kamat, Trivikram <16024985+trivikr@users.noreply.github.com> Assisted-by: codex:gpt-5.6-sol PR-URL: nodejs#65367 Fixes: nodejs#65366 Reviewed-By: James M Snell <jasnell@gmail.com>
Originally added in 16b11cd, it first had three callers. Now there's only one branch requireing it. Signed-off-by: Szymon Łągiewka <slagiewka@users.noreply.github.com> PR-URL: nodejs#64473 Reviewed-By: Chemi Atlow <chemi@atlow.co.il>
While more readable, the removed closure overhead unlocks 10-20% in eventtarget.js benchmark. Signed-off-by: Szymon Łągiewka <slagiewka@users.noreply.github.com> PR-URL: nodejs#64473 Reviewed-By: Chemi Atlow <chemi@atlow.co.il>
PR-URL: nodejs#65542 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Filip Skokan <panva.ip@gmail.com> Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com>
PR-URL: nodejs#65542 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Filip Skokan <panva.ip@gmail.com> Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com>
Signed-off-by: Filip Skokan <panva.ip@gmail.com> PR-URL: nodejs#65542 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Filip Skokan <panva.ip@gmail.com> Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com>
AIX can report lo0 as the scope ID for a datagram sent to a local interface. Validate that the received address includes a non-empty scope ID and verify that the address can be used to reply, instead of requiring the scope ID to match the destination interface. Refs: nodejs#46792 Assisted-by: Codex Signed-off-by: Filip Skokan <panva.ip@gmail.com> PR-URL: nodejs#65629 Refs: nodejs#46792 Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
PR-URL: nodejs#65789 Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Filip Skokan <panva.ip@gmail.com> Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
PR-URL: nodejs#65653 Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com> Reviewed-By: Filip Skokan <panva.ip@gmail.com> Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com> Reviewed-By: Mike McCready <66998419+MikeMcC399@users.noreply.github.com>
aduh95
force-pushed
the
backport-64166-to-v24.x
branch
from
September 7, 2026 09:36
007c0bf to
176146b
Compare
Node.js 24 does not include the later stream lifecycle changes that the original fix relies on. Preserve its reset ordering, avoid JavaScript callbacks after a deferred session close, and let destroyed streams finish without requesting trailers. Signed-off-by: Matteo Collina <hello@matteocollina.com> Refs: nodejs#64166
aduh95
force-pushed
the
backport-64166-to-v24.x
branch
from
September 7, 2026 09:38
176146b to
b53cfdb
Compare
aduh95
approved these changes
Sep 7, 2026
aduh95
pushed a commit
that referenced
this pull request
Sep 7, 2026
Node.js 24 does not include the later stream lifecycle changes that the original fix relies on. Preserve its reset ordering, avoid JavaScript callbacks after a deferred session close, and let destroyed streams finish without requesting trailers. Signed-off-by: Matteo Collina <hello@matteocollina.com> Refs: #64166 PR-URL: #65093 Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Collaborator
aduh95
pushed a commit
that referenced
this pull request
Sep 7, 2026
Node.js 24 does not include the later stream lifecycle changes that the original fix relies on. Preserve its reset ordering, avoid JavaScript callbacks after a deferred session close, and let destroyed streams finish without requesting trailers. Signed-off-by: Matteo Collina <hello@matteocollina.com> Refs: #64166 PR-URL: #65093 Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
pimterry
approved these changes
Sep 7, 2026
Collaborator
aduh95
force-pushed
the
v24.x-staging
branch
3 times, most recently
from
September 9, 2026 21:47
7fc125b to
e711d3f
Compare
Contributor
|
This pull request has conflicts with its base branch, removing the |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
20 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Backports #64166 to
v24.x-staging.The direct cherry-pick relies on newer HTTP/2 stream lifecycle behavior that is not present in Node.js 24. The second commit preserves v24's reset ordering, avoids JavaScript callbacks after a session close is deferred out of
nghttp2_session_mem_recv(), and lets destroyed streams finish without requesting trailers.This addresses the memory corruption reported in #64841, where the corrupted state generally surfaced later in V8's concurrent Maglev compiler.
The release build passed all 278
parallel/test-http2-*tests. The Undici reproducer also completed 96/96 concurrent reduced runs and 16/16 concurrent originalnode:testruns without an abnormal exit.