Skip to content

Commit 63a6ea2

Browse files
christianaurichzmpanva
authored andcommitted
ffi: reject non-boolean copy arguments
ffi.toBuffer() and ffi.toArrayBuffer() document copy as a boolean, but read it with BooleanValue(), which applies JavaScript truthiness. A falsy non-boolean such as null, 0 or '' therefore selects the zero-copy mode, which returns a writable view over foreign memory instead of a copy, while values such as 'false' or 1 select a copy. Throw ERR_INVALID_ARG_TYPE when copy is neither undefined nor a boolean. Omitting copy or passing undefined still makes a copy, and true and false keep their current behavior. Signed-off-by: Christian Aurich Zanettini Martins <christian.aurichzm@gmail.com> PR-URL: #66219 Reviewed-By: Daeyeon Jeong <daeyeon.dev@gmail.com> Reviewed-By: Filip Skokan <panva.ip@gmail.com>
1 parent 118f2a1 commit 63a6ea2

2 files changed

Lines changed: 24 additions & 5 deletions

File tree

‎src/ffi/data.cc‎

Lines changed: 12 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -546,7 +546,6 @@ static bool ZeroCopyUnavailable(Environment* env) {
546546

547547
void ToBuffer(const FunctionCallbackInfo<Value>& args) {
548548
Environment* env = Environment::GetCurrent(args);
549-
Isolate* isolate = env->isolate();
550549

551550
THROW_IF_INSUFFICIENT_PERMISSIONS(env, permission::PermissionScope::kFFI, "");
552551

@@ -589,8 +588,12 @@ void ToBuffer(const FunctionCallbackInfo<Value>& args) {
589588
return;
590589
}
591590

592-
bool copy = args.Length() < 3 || args[2]->IsUndefined() ||
593-
args[2]->BooleanValue(isolate);
591+
if (!args[2]->IsUndefined() && !args[2]->IsBoolean()) {
592+
THROW_ERR_INVALID_ARG_TYPE(env, "The copy argument must be a boolean");
593+
return;
594+
}
595+
596+
bool copy = !args[2]->IsFalse();
594597
if (!copy && ZeroCopyUnavailable(env)) return;
595598

596599
Local<Object> buf;
@@ -654,8 +657,12 @@ void ToArrayBuffer(const FunctionCallbackInfo<Value>& args) {
654657
return;
655658
}
656659

657-
bool copy = args.Length() < 3 || args[2]->IsUndefined() ||
658-
args[2]->BooleanValue(isolate);
660+
if (!args[2]->IsUndefined() && !args[2]->IsBoolean()) {
661+
THROW_ERR_INVALID_ARG_TYPE(env, "The copy argument must be a boolean");
662+
return;
663+
}
664+
665+
bool copy = !args[2]->IsFalse();
659666
if (!copy && ZeroCopyUnavailable(env)) return;
660667

661668
Local<ArrayBuffer> ab;

‎test/ffi/test-ffi-memory.js‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -132,6 +132,18 @@ test('ffi zero-copy views throw with the V8 sandbox', { skip: !common.hasV8Sandb
132132
}));
133133
});
134134

135+
test('ffi toBuffer and toArrayBuffer require a boolean copy argument', () => {
136+
withAllocations(common.mustCall((alloc) => {
137+
const ptr = alloc(4);
138+
const type = { code: 'ERR_INVALID_ARG_TYPE' };
139+
140+
for (const copy of [null, 0, '', 'false', 1, {}]) {
141+
assert.throws(() => ffi.toBuffer(ptr, 4, copy), type);
142+
assert.throws(() => ffi.toArrayBuffer(ptr, 4, copy), type);
143+
}
144+
}));
145+
});
146+
135147
test('ffi getRawPointer returns raw addresses for byte sources', () => {
136148
const buffer = Buffer.from([1, 2, 3]);
137149
const arrayBuffer = new Uint8Array([4, 5, 6, 7]).buffer;

0 commit comments

Comments
 (0)