René's URL Explorer Experiment

{"@context":"https://schema.org","@type":"DiscussionForumPosting","headline":"Use-after-free in `CleanupHookThunkRun` for every `node::ObjectWrap` alive at teardown","articleBody":"### Version\n\nv26.7.0\n\n### Platform\n\n```text\nLinux 6.8.0-100-generic aarch64\n```\n\n### Subsystem\n\n_No response_\n\n### What steps will reproduce the bug?\n\nBug can be reproduced with `test/addons/worker-addon-exit` from node source (it creates one instance that is still live when the worker\nEnvironment tears down):\n```console\n$ cd test/addons/worker-addon-exit && npx node-gyp configure build\n$ valgrind --undef-value-errors=no node test.js\n...\n==359== Thread 8 WorkerThread:\n==359== Invalid read of size 8\n==359==    at 0x89C990: node::CleanupHookThunkRun(void*) (in /usr/local/bin/node)\n==359==    by 0x8C6DF7: node::CleanupQueue::Drain() (in /usr/local/bin/node)\n==359==    by 0x91F31B: node::Environment::RunCleanup() (in /usr/local/bin/node)\n==359==    by 0x8986B3: node::FreeEnvironment(node::Environment*) (in /usr/local/bin/node)\n==359==    by 0xAF4AE7: node::worker::Worker::Run() (in /usr/local/bin/node)\n==359==    by 0xAF99CF: node::worker::Worker::StartThread(v8::FunctionCallbackInfo\u003cv8::Value> const&)::$_0::__invoke(void*) (in /usr/local/bin/node)\n==359==    by 0x7CB202F: start_thread (pthread_create.c:442)\n==359==    by 0x7D1BF5B: thread_start (clone.S:79)\n==359==  Address 0x338aba88 is 8 bytes inside a block of size 48 free'd\n==359==    at 0x7858360: operator delete(void*, unsigned long) (vg_replace_malloc.c:935)\n==359==    by 0x89CB47: node::RemoveEnvironmentCleanupHook(v8::Isolate*, void (*)(void*), void*) (in /usr/local/bin/node)\n==359==    by 0x54C91FB3: MyObject::~MyObject() (in /app/test/addons/worker-addon-exit/build/Release/binding.node)\n==359==    by 0x54C91FF3: MyObject::~MyObject() (in /app/test/addons/worker-addon-exit/build/Release/binding.node)\n==359==    by 0x89C98F: node::CleanupHookThunkRun(void*) (in /usr/local/bin/node)\n==359==    by 0x8C6DF7: node::CleanupQueue::Drain() (in /usr/local/bin/node)\n==359==    by 0x91F31B: node::Environment::RunCleanup() (in /usr/local/bin/node)\n==359==    by 0x8986B3: node::FreeEnvironment(node::Environment*) (in /usr/local/bin/node)\n==359==    by 0xAF4AE7: node::worker::Worker::Run() (in /usr/local/bin/node)\n==359==    by 0xAF99CF: node::worker::Worker::StartThread(v8::FunctionCallbackInfo\u003cv8::Value> const&)::$_0::__invoke(void*) (in /usr/local/bin/node)\n==359==    by 0x7CB202F: start_thread (pthread_create.c:442)\n==359==    by 0x7D1BF5B: thread_start (clone.S:79)\n==359==  Block was alloc'd at\n==359==    at 0x7855828: operator new(unsigned long) (vg_replace_malloc.c:422)\n==359==    by 0x89DB73: std::pair\u003cstd::__detail::_Node_iterator\u003cnode::CleanupHookThunk, true, true>, bool> std::_Hashtable\u003cnode::CleanupHookThunk, node::CleanupHookThunk, std::allocator\u003cnode::CleanupHookThunk>, std::__detail::_Identity, std::equal_to\u003cnode::CleanupHookThunk>, node::CleanupHookThunkHash, std::__detail::_Mod_range_hashing, std::__detail::_Default_ranged_hash, std::__detail::_Prime_rehash_policy, std::__detail::_Hashtable_traits\u003ctrue, true, true> >::_M_insert_unique\u003cnode::CleanupHookThunk, node::CleanupHookThunk, std::__detail::_AllocNode\u003cstd::allocator\u003cstd::__detail::_Hash_node\u003cnode::CleanupHookThunk, true> > > >(node::CleanupHookThunk&&, node::CleanupHookThunk&&, std::__detail::_AllocNode\u003cstd::allocator\u003cstd::__detail::_Hash_node\u003cnode::CleanupHookThunk, true> > > const&) (in /usr/local/bin/node)\n==359==    by 0x89C8DB: node::AddEnvironmentCleanupHook(v8::Isolate*, void (*)(void*), void*) (in /usr/local/bin/node)\n==359==    by 0x54C92053: MyObject::MyObject(double) (in /app/test/addons/worker-addon-exit/build/Release/binding.node)\n==359==    by 0x54C921BF: MyObject::New(v8::FunctionCallbackInfo\u003cv8::Value> const&) (in /app/test/addons/worker-addon-exit/build/Release/binding.node)\n==359==    by 0xCAAE2F: v8::internal::FunctionCallbackArguments::CallOrConstruct(v8::internal::Isolate*, v8::internal::Tagged\u003cv8::internal::FunctionTemplateInfo>, bool) (in /usr/local/bin/node)\n==359==    by 0xCAA4E3: v8::internal::Builtin_HandleApiConstruct(int, unsigned long*, v8::internal::Isolate*) (in /usr/local/bin/node)\n==359==    by 0x1A32BCB: Builtins_CEntry_Return1_ArgvOnStack_BuiltinExit (in /usr/local/bin/node)\n```\n\n### How often does it reproduce? Is there a required condition?\n\n100%, deterministic\n\n### What is the expected behavior? Why is that the expected behavior?\n\nDeleting an `ObjectWrap` from the environment cleanup drain should not read freed memory. Since #63642, `~ObjectWrap` calls `RemoveEnvironmentCleanupHook`, so this is now the ordinary teardown path for every `ObjectWrap`-based addon, not an edge case.\n\n### What do you see instead?\n\n```\n==359== Thread 8 WorkerThread:\n==359== Invalid read of size 8\n==359==    at 0x89C990: node::CleanupHookThunkRun(void*) (in /usr/local/bin/node)\n==359==    by 0x8C6DF7: node::CleanupQueue::Drain() (in /usr/local/bin/node)\n```\n\nThe process still exits 0, so this is silent unless you run under valgrind.\n\n### Additional information\n\n#### Cause\n\n`src/api/hooks.cc`:\n\n```cpp\nstatic void CleanupHookThunkRun(void* arg) {\n  const CleanupHookThunk* thunk = static_cast\u003cCleanupHookThunk*>(arg);\n  thunk->fun(thunk->arg);                                               // (1)\n  RemoveEnvironmentCleanupHook(thunk->isolate, thunk->fun, thunk->arg); // (2)\n}\n```\n\n(1) is `ObjectWrap::CleanupHook`, i.e. `delete wrap`. `~ObjectWrap` calls `RemoveEnvironmentCleanupHook` itself, which does `registry->erase(result)` and destroys the `CleanupHookThunk` node that `thunk` points into. (2) then reads `thunk->isolate` / `thunk->fun` / `thunk->arg` out of that freed node. The second removal is also redundant.\n\nIntroduced by #63985, which added the registry, in combination with #63642, which made `~ObjectWrap` remove its own hook. \n\n#### Suggested fix\n\nCache the fields before the callback can invalidate the node:\n\n```cpp\nstatic void CleanupHookThunkRun(void* arg) {\n  const CleanupHookThunk* thunk = static_cast\u003cCleanupHookThunk*>(arg);\n  Isolate* isolate = thunk->isolate;\n  CleanupHook fun = thunk->fun;\n  void* fun_arg = thunk->arg;\n  fun(fun_arg);\n  RemoveEnvironmentCleanupHook(isolate, fun, fun_arg);\n}\n```\n","author":{"url":"https://github.com/nsavoire","@type":"Person","name":"nsavoire"},"datePublished":"2026-08-10T14:48:42Z","url":"https://github.com/nodejs/node/issues/65195"}