From 211aa20378e44ad0d7daf1b41598ae70f906a71e Mon Sep 17 00:00:00 2001
From: "Kamat, Trivikram" <16024985+trivikr@users.noreply.github.com>
Date: Sun, 1 Nov 2026 12:47:26 -0800
Subject: [PATCH] ffi: reject signature conflicts from getFunctions getters

A signature getter could request an earlier symbol with a different
signature, making getFunctions() return a wrapper with that signature.
Recheck pending functions before caching them, and throw on a mismatch.

Signed-off-by: Kamat, Trivikram <16024985+trivikr@users.noreply.github.com>
Assisted-by: claude:opus-5.5
---
 src/node_ffi.cc                      | 24 ++++++++++++--
 test/ffi/test-ffi-dynamic-library.js | 48 ++++++++++++++++++++++++++++
 2 files changed, 69 insertions(+), 3 deletions(-)

diff --git a/src/node_ffi.cc b/src/node_ffi.cc
index b988120606c6..bab60081456b 100644
--- a/src/node_ffi.cc
+++ b/src/node_ffi.cc
@@ -286,9 +286,10 @@ MaybeLocal<Function> DynamicLibrary::CreateFunction(
 
   // Creating a callable emits a trampoline, allocates an FFIFunctionInfo, and
   // on the SharedBuffer path allocates an ArrayBuffer, so reuse the one already
-  // handed out for this symbol. `PrepareFunction()` rejects a request that uses
-  // a different signature, so a hit always describes the same signature. An
-  // empty handle means the wrapper was collected; fall through and rebuild.
+  // handed out for this symbol. `PrepareFunction()` and `GetFunctions()` reject
+  // a request that uses a different signature, so a hit always describes the
+  // same signature. An empty handle means the wrapper was collected; fall
+  // through and rebuild.
   auto cached = function_wrappers_.find(name);
   if (cached != function_wrappers_.end()) {
     if (!cached->second.IsEmpty()) {
@@ -964,6 +965,23 @@ void DynamicLibrary::GetFunctions(const FunctionCallbackInfo<Value>& args) {
       });
     }
 
+    // The signature getters above run user code that may have requested an
+    // earlier symbol with a different signature. Check before caching anything
+    // so that a rejected call leaves the library unchanged.
+    for (const auto& item : pending) {
+      auto existing = lib->functions_.find(item.name);
+      if (existing != lib->functions_.end() &&
+          !SignaturesMatch(
+              *existing->second, item.fn->return_type, item.fn->args)) {
+        THROW_ERR_INVALID_ARG_VALUE(
+            env,
+            "Function %s"
+            " was already requested with a different signature",
+            item.name);
+        return;
+      }
+    }
+
     for (const auto& item : pending) {
       if (item.should_cache_symbol) {
         lib->symbols_.emplace(item.name, item.fn->ptr);
diff --git a/test/ffi/test-ffi-dynamic-library.js b/test/ffi/test-ffi-dynamic-library.js
index 62cc7e0b4d49..bf9991efe57a 100644
--- a/test/ffi/test-ffi-dynamic-library.js
+++ b/test/ffi/test-ffi-dynamic-library.js
@@ -215,6 +215,54 @@ test('getFunction caches signatures consistently', () => {
   }
 });
 
+test('getFunctions rejects a signature changed by a getter', () => {
+  const lib = new ffi.DynamicLibrary(libraryPath);
+  const conflicting = { arguments: ['f64', 'f64'], return: 'f64' };
+
+  try {
+    let fn;
+    assert.throws(() => {
+      lib.getFunctions({
+        add_i32: fixtureSymbols.add_i32,
+        get add_i64() {
+          fn = lib.getFunction('add_i32', conflicting);
+          return fixtureSymbols.add_i64;
+        },
+      });
+    }, {
+      code: 'ERR_INVALID_ARG_VALUE',
+      message: /Function add_i32 was already requested with a different signature/,
+    });
+
+    // The rejected call must not cache any of its own entries.
+    assert.deepStrictEqual(Object.keys(lib.functions), ['add_i32']);
+    assert.strictEqual(lib.getFunction('add_i32', conflicting), fn);
+  } finally {
+    lib.close();
+  }
+});
+
+test('getFunctions accepts a matching signature requested by a getter', () => {
+  const lib = new ffi.DynamicLibrary(libraryPath);
+
+  try {
+    let fn;
+    const functions = lib.getFunctions({
+      add_i32: fixtureSymbols.add_i32,
+      get add_i64() {
+        fn = lib.getFunction('add_i32', fixtureSymbols.add_i32);
+        return fixtureSymbols.add_i64;
+      },
+    });
+
+    assert.strictEqual(functions.add_i32, fn);
+    assert.strictEqual(functions.add_i32(20, 22), 42);
+    assert.strictEqual(functions.add_i64(20n, 22n), 42n);
+  } finally {
+    lib.close();
+  }
+});
+
 test('resolving the same symbol reuses one function', () => {
   const lib = new ffi.DynamicLibrary(libraryPath);
   const definitions = { add_i32: fixtureSymbols.add_i32 };