mirror of
https://github.com/nodejs/node.git
synced 2026-10-28 18:25:13 +08:00
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: https://github.com/nodejs/node/pull/66368 Fixes: https://github.com/nodejs/node/issues/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>
This commit is contained in:
1 parent
6b8bc0a6af
commit
f71d6441ce
2 files changed
+18
-1
No files matched your search
+3
-1
@@ -144,12 +144,14 @@ Maybe<void*> DynamicLibrary::ResolveSymbol(Environment* env,
|
||||
Maybe<DynamicLibrary::PreparedFunction> DynamicLibrary::PrepareFunction(
|
||||
Environment* env, const std::string& name, Local<Object> signature) {
|
||||
std::shared_ptr<FFIFunction> fn;
|
||||
auto existing = functions_.find(name);
|
||||
FunctionSignature parsed;
|
||||
|
||||
if (!ParseFunctionSignature(env, name, signature).To(&parsed)) {
|
||||
return {};
|
||||
}
|
||||
// Look up the cache only after parsing: the signature's getters run user
|
||||
// code that may close the library, which clears `functions_`.
|
||||
auto existing = functions_.find(name);
|
||||
auto [return_type, args, return_type_name, arg_type_names] =
|
||||
std::move(parsed);
|
||||
|
||||
|
||||
@@ -290,6 +290,21 @@ test('closed libraries reject subsequent operations', () => {
|
||||
assert.throws(() => lib.getSymbols(), /Library is closed/);
|
||||
});
|
||||
|
||||
test('closing the library from a signature getter of a cached function', () => {
|
||||
const lib = new ffi.DynamicLibrary(libraryPath);
|
||||
lib.getFunction('add_i32', fixtureSymbols.add_i32);
|
||||
|
||||
assert.throws(() => {
|
||||
lib.getFunction('add_i32', {
|
||||
arguments: ['i32', 'i32'],
|
||||
get return() {
|
||||
lib.close();
|
||||
return 'i32';
|
||||
},
|
||||
});
|
||||
}, { code: 'ERR_FFI_LIBRARY_CLOSED' });
|
||||
});
|
||||
|
||||
test('optimized fast calls reject calls after the library is closed', () => {
|
||||
const { lib, functions } = ffi.dlopen(libraryPath, {
|
||||
multiply_f64: fixtureSymbols.multiply_f64,
|
||||
|
||||
Reference in new issue
Block a user