ffi: fix crash in refCallback and unrefCallback · nodejs/node@428e9bc · GitHub
Skip to content

Commit 428e9bc

Browse files
trivikraduh95
authored andcommitted
ffi: fix crash in refCallback and unrefCallback
Both handlers called fn.ClearWeak() and fn.SetWeak() without checking whether the persistent handle was still populated. After the callback function is garbage collected following an earlier unrefCallback() call, the handle is empty and both V8 methods dereference a null slot. InvokeCallback() already guarded against this. Add the same check to both handlers and throw ERR_INVALID_ARG_VALUE, matching the existing behavior for a pointer that is not in the callback map. Signed-off-by: Trivikram Kamat <16024985+trivikr@users.noreply.github.com> Assisted-by: claude:opus-5 PR-URL: #64881 Fixes: #64880 Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Aviv Keller <me@aviv.sh>
1 parent c96d76a commit 428e9bc

3 files changed

Lines changed: 55 additions & 0 deletions

File tree

doc/api/ffi.md

Lines changed: 7 additions & 0 deletions

src/node_ffi.cc

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1127,6 +1127,15 @@ void DynamicLibrary::RefCallback(const FunctionCallbackInfo<Value>& args) {
11271127
return;
11281128
}
11291129

1130+
// The callback function may already have been collected after a previous
1131+
// unrefCallback() call. The persistent handle is empty in that case, and
1132+
// ClearWeak() on an empty handle dereferences a null slot. There is also no
1133+
// function left to make strong again.
1134+
if (existing->second->fn.IsEmpty()) {
1135+
THROW_ERR_INVALID_ARG_VALUE(env, "Callback not found");
1136+
return;
1137+
}
1138+
11301139
existing->second->fn.ClearWeak();
11311140
}
11321141

@@ -1158,6 +1167,14 @@ void DynamicLibrary::UnrefCallback(const FunctionCallbackInfo<Value>& args) {
11581167
return;
11591168
}
11601169

1170+
// The callback function may already have been collected by a previous
1171+
// unrefCallback() call. The persistent handle is empty in that case, and
1172+
// SetWeak() on an empty handle dereferences a null slot.
1173+
if (existing->second->fn.IsEmpty()) {
1174+
THROW_ERR_INVALID_ARG_VALUE(env, "Callback not found");
1175+
return;
1176+
}
1177+
11611178
existing->second->fn.SetWeak();
11621179
}
11631180

test/ffi/test-ffi-weakref-calls.js

Lines changed: 31 additions & 0 deletions

0 commit comments

Comments
 (0)