Skip to content

Commit 58e5f70

Browse files
ffi: validate 'void' as parameter type in getFunction and getFunctions
Fixes: #63461 Signed-off-by: Anshikakalpana <anshikajain196872@gmail.com>
1 parent 1b04f16 commit 58e5f70

2 files changed

Lines changed: 51 additions & 2 deletions

File tree

lib/internal/ffi-shared-buffer.js

Lines changed: 19 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@ const {
3434
const {
3535
codes: {
3636
ERR_INTERNAL_ASSERTION,
37+
ERR_INVALID_ARG_VALUE,
3738
},
3839
} = require('internal/errors');
3940

@@ -555,6 +556,18 @@ function sigResult(sig) {
555556
return sig.result ?? sig.return ?? sig.returns ?? 'void';
556557
}
557558

559+
function validateParameters(parameters, prefix) {
560+
for (let i = 0; i < parameters.length; i++) {
561+
if (parameters[i] === 'void') {
562+
throw new ERR_INVALID_ARG_VALUE(
563+
`${prefix}[${i}]`,
564+
parameters[i],
565+
'must not be "void"; use an empty array for no-argument functions',
566+
);
567+
}
568+
}
569+
}
570+
558571
// The native invoker for SB-eligible symbols is `InvokeFunctionSB`, which
559572
// reads arguments from the shared buffer populated by
560573
// `wrapWithSharedBuffer`. These patches make sure every path that surfaces
@@ -566,8 +579,10 @@ const rawGetFunctions = DynamicLibrary.prototype.getFunctions;
566579
DynamicLibrary.prototype.getFunction = function getFunction(name, sig) {
567580
// Native `DynamicLibrary::GetFunction` validates `sig`, so by the time
568581
// we have `raw` we know `sig` is a valid object.
582+
const params = sigParams(sig);
583+
validateParameters(params, 'options.parameters');
569584
const raw = FunctionPrototypeCall(rawGetFunction, this, name, sig);
570-
return wrapWithSharedBuffer(raw, sigParams(sig), sigResult(sig));
585+
return wrapWithSharedBuffer(raw, params, sigResult(sig));
571586
};
572587

573588
DynamicLibrary.prototype.getFunctions = function getFunctions(definitions) {
@@ -589,7 +604,9 @@ DynamicLibrary.prototype.getFunctions = function getFunctions(definitions) {
589604
out[name] = wrapWithSharedBuffer(raw[name]);
590605
} else {
591606
const sig = definitions[name];
592-
out[name] = wrapWithSharedBuffer(raw[name], sigParams(sig), sigResult(sig));
607+
const params = sigParams(sig);
608+
validateParameters(params, `definitions.${name}.parameters`);
609+
out[name] = wrapWithSharedBuffer(raw[name], params, sigResult(sig));
593610
}
594611
}
595612
return out;
Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
'use strict';
2+
3+
const common = require('../common');
4+
5+
if (!process.config.variables.node_use_ffi) {
6+
common.skip('requires FFI support');
7+
}
8+
9+
const assert = require('node:assert');
10+
const { spawnSync } = require('node:child_process');
11+
12+
// Regression test for https://github.com/nodejs/node/issues/63461
13+
// 'void' as a parameter type should throw ERR_INVALID_ARG_VALUE
14+
// instead of triggering ERR_INTERNAL_ASSERTION.
15+
16+
{
17+
const { stderr, status } = spawnSync(process.execPath, [
18+
'--expose-internals',
19+
'-e',
20+
`
21+
const { DynamicLibrary } = internalBinding('ffi');
22+
const lib = new DynamicLibrary(null);
23+
lib.getFunction('f', { parameters: ['void'] });
24+
`,
25+
], {
26+
encoding: 'utf8',
27+
});
28+
29+
assert.strictEqual(status, 1);
30+
assert.match(stderr, /ERR_INVALID_ARG_VALUE/);
31+
assert.doesNotMatch(stderr, /ERR_INTERNAL_ASSERTION/);
32+
}

0 commit comments

Comments
 (0)