Skip to content

Commit dd2fa31

Browse files
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>
1 parent f9ab994 commit dd2fa31

1 file changed

Lines changed: 9 additions & 2 deletions

File tree

‎src/api/hooks.cc‎

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -145,8 +145,15 @@ static ExclusiveAccess<CleanupHookRegistry> cleanup_hook_registry;
145145

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

152159
void AddEnvironmentCleanupHook(Isolate* isolate,

0 commit comments

Comments
 (0)