Skip to content

Commit 343a5c2

Browse files
trivikraduh95
authored andcommitted
ffi: fix use-after-free in PrepareFunction
PrepareFunction() looked up the function cache before parsing the signature. Parsing can run user getters, and a getter that calls lib.close() clears the cache, which invalidates the iterator. For a function that was already cached, reading it afterwards used freed memory and crashed the process. Look up the cache after the signature is parsed. If the library was closed during parsing, ResolveSymbol() now throws ERR_FFI_LIBRARY_CLOSED. Signed-off-by: Trivikram Kamat <16024985+trivikr@users.noreply.github.com> Assisted-by: claude:opus-5.5 PR-URL: #66368 Fixes: #66367 Reviewed-By: Daeyeon Jeong <daeyeon.dev@gmail.com> Reviewed-By: Paolo Insogna <paolo@cowtech.it> Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
1 parent 1ff8d5c commit 343a5c2

2 files changed

Lines changed: 18 additions & 1 deletion

File tree

‎src/node_ffi.cc‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -144,12 +144,14 @@ Maybe<void*> DynamicLibrary::ResolveSymbol(Environment* env,
144144
Maybe<DynamicLibrary::PreparedFunction> DynamicLibrary::PrepareFunction(
145145
Environment* env, const std::string& name, Local<Object> signature) {
146146
std::shared_ptr<FFIFunction> fn;
147-
auto existing = functions_.find(name);
148147
FunctionSignature parsed;
149148

150149
if (!ParseFunctionSignature(env, name, signature).To(&parsed)) {
151150
return {};
152151
}
152+
// Look up the cache only after parsing: the signature's getters run user
153+
// code that may close the library, which clears `functions_`.
154+
auto existing = functions_.find(name);
153155
auto [return_type, args, return_type_name, arg_type_names] =
154156
std::move(parsed);
155157

‎test/ffi/test-ffi-dynamic-library.js‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -290,6 +290,21 @@ test('closed libraries reject subsequent operations', () => {
290290
assert.throws(() => lib.getSymbols(), /Library is closed/);
291291
});
292292

293+
test('closing the library from a signature getter of a cached function', () => {
294+
const lib = new ffi.DynamicLibrary(libraryPath);
295+
lib.getFunction('add_i32', fixtureSymbols.add_i32);
296+
297+
assert.throws(() => {
298+
lib.getFunction('add_i32', {
299+
arguments: ['i32', 'i32'],
300+
get return() {
301+
lib.close();
302+
return 'i32';
303+
},
304+
});
305+
}, { code: 'ERR_FFI_LIBRARY_CLOSED' });
306+
});
307+
293308
test('optimized fast calls reject calls after the library is closed', () => {
294309
const { lib, functions } = ffi.dlopen(libraryPath, {
295310
multiply_f64: fixtureSymbols.multiply_f64,

0 commit comments

Comments
 (0)