From 893ed58ca0aa086a2cc8f7d93ae51805ab901e5b Mon Sep 17 00:00:00 2001 From: Sreehari Annam Date: Mon, 10 Aug 2026 10:58:05 -0400 Subject: [PATCH] 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. Fixes: https://github.com/nodejs/node/issues/65195 --- src/api/hooks.cc | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/src/api/hooks.cc b/src/api/hooks.cc index 09a79fb9da8..b46073b6b7c 100644 --- a/src/api/hooks.cc +++ b/src/api/hooks.cc @@ -145,8 +145,15 @@ static ExclusiveAccess cleanup_hook_registry; static void CleanupHookThunkRun(void* arg) { const CleanupHookThunk* thunk = static_cast(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,