Skip to content

src: fix use-after-free in CleanupHookThunkRun #65630

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

Merged
nodejs-github-bot merged 2 commits into from
Sep 4, 2026
+49 −2
Merged

src: fix use-after-free in CleanupHookThunkRun #65630

Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
dd2fa31
src: fix use-after-free in CleanupHookThunkRun
everett1992 Aug 10, 2026
fd325a7
test: add regression test for cleanup hook UAF
everett1992 Aug 19, 2026
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
Next Next commit
src: fix use-after-free in CleanupHookThunkRun 
CleanupHookThunkRun() read thunk->isolate/fun/arg from the
CleanupHookThunk after invoking thunk->fun(). For every
node::ObjectWrap alive at teardown, thunk->fun is
ObjectWrap::CleanupHook, which deletes the wrap; ~ObjectWrap() calls
RemoveEnvironmentCleanupHook() itself, erasing the CleanupHookThunk
from the registry and freeing the node it lives in. The subsequent
read of thunk->isolate/fun/arg to make the (now redundant) second
RemoveEnvironmentCleanupHook() call was therefore a use-after-free.

Cache the fields before running the hook so nothing is read from
`thunk` once it may have been freed.

Taken over from #65196, which has been inactive; the original change is
unmodified apart from the added comment.

This also unblocks #65042, the backport of the cleanup hook registry to
v24.x. Without that registry ~ObjectWrap() asserts during garbage
collection, so every 24.x runtime aborts for ObjectWrap addons
(#65446), as do 26.x runtimes before 26.4.0 when used with newer
headers (#65262).

Fixes: #65195
Refs: #65196
Refs: #65042
Refs: #65446
Refs: #65262
Assisted-by: a closed-source coding agent
Co-authored-by: Sreehari Annam <sreehari.annam@gmail.com>
Signed-off-by: Caleb Everett <everett.caleb@gmail.com>
  • Loading branch information
@everett1992 @sreehariannam
everett1992 and sreehariannam committed Sep 1, 2026
commit dd2fa31b1d99e5a6710e08ce4139da7c2e344598
11 changes: 9 additions & 2 deletions src/api/hooks.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -145,8 +145,15 @@ static ExclusiveAccess<CleanupHookRegistry> cleanup_hook_registry;

static void CleanupHookThunkRun(void* arg) {
const CleanupHookThunk* thunk = static_cast<CleanupHookThunk*>(arg);
thunk->fun(thunk->arg);
RemoveEnvironmentCleanupHook(thunk->isolate, thunk->fun, thunk->arg);
// `thunk->fun` may itself remove and free this CleanupHookThunk (e.g. via
// ~ObjectWrap(), which calls RemoveEnvironmentCleanupHook()), so cache the
// fields we still need before invoking it rather than reading them from
// `thunk` afterwards.
Isolate* isolate = thunk->isolate;
CleanupHook fun = thunk->fun;
void* fun_arg = thunk->arg;
fun(fun_arg);
RemoveEnvironmentCleanupHook(isolate, fun, fun_arg);
}

void AddEnvironmentCleanupHook(Isolate* isolate,
Expand Down