Skip to content

Commit 24b9ca9

Browse files
ffi: avoid memcpy() with a null pointer
Zero-length FFI memory operations accept a null pointer, but ToArrayBuffer() and ExportBytes() call memcpy() unconditionally. Passing a null pointer to memcpy() is undefined behavior even when the size is zero. Skip the copy when the length is zero, matching what Buffer::Copy() already does for the same case. An isolated reproduction compiled with -fsanitize=undefined reports "null pointer passed as argument 1, which is declared to never be null". The added test covers the reachable zero-length paths, but it passes without the fix: libc does not fault on memcpy(NULL, NULL, 0), and --enable-ubsan does not set -fno-sanitize-recover, so UBSan reports the call without failing the process. Signed-off-by: Christian Aurich Zanettini Martins <christian.aurichzm@gmail.com> PR-URL: #66200 Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com> Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com>
1 parent bba3422 commit 24b9ca9

2 files changed

Lines changed: 12 additions & 2 deletions

File tree

‎src/ffi/data.cc‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -662,7 +662,7 @@ void ToArrayBuffer(const FunctionCallbackInfo<Value>& args) {
662662
if (copy) {
663663
std::unique_ptr<BackingStore> store =
664664
ArrayBuffer::NewBackingStore(isolate, len);
665-
memcpy(store->Data(), reinterpret_cast<void*>(ptr), len);
665+
if (len > 0) memcpy(store->Data(), reinterpret_cast<void*>(ptr), len);
666666
ab = ArrayBuffer::New(isolate, std::move(store));
667667
} else {
668668
std::unique_ptr<BackingStore> store = ArrayBuffer::NewBackingStore(
@@ -740,7 +740,9 @@ void ExportBytes(const FunctionCallbackInfo<Value>& args) {
740740
return;
741741
}
742742

743-
std::memcpy(reinterpret_cast<void*>(ptr), view.data(), view.length());
743+
if (view.length() > 0) {
744+
std::memcpy(reinterpret_cast<void*>(ptr), view.data(), view.length());
745+
}
744746
}
745747

746748
void GetRawPointer(const FunctionCallbackInfo<Value>& args) {

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

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -243,6 +243,14 @@ test('ffi toString returns null for a null pointer', () => {
243243
assert.strictEqual(ffi.toString(0n), null);
244244
});
245245

246+
test('ffi accepts zero-length copies through a null pointer', () => {
247+
assert.strictEqual(ffi.toBuffer(0n, 0).length, 0);
248+
assert.strictEqual(ffi.toArrayBuffer(0n, 0).byteLength, 0);
249+
assert.strictEqual(ffi.exportBuffer(Buffer.alloc(0), 0n, 0), undefined);
250+
assert.strictEqual(ffi.exportArrayBuffer(new ArrayBuffer(0), 0n, 0), undefined);
251+
assert.strictEqual(ffi.exportArrayBufferView(new Uint8Array(0), 0n, 0), undefined);
252+
});
253+
246254
test('ffi validates memory access arguments', () => {
247255
withAllocations(common.mustCall((alloc) => {
248256
const ptr = alloc(8);

0 commit comments

Comments
 (0)