Skip to content

Commit 9bdab1e

Browse files
christianaurichzmaduh95
authored andcommitted
ffi: reject unsafe integers as length or offset
GetValidatedSize() checks the value against static_cast<double>(SIZE_MAX), which rounds up to 2^64 on 64-bit platforms. A length or offset of 2 ** 64 gets through, and the cast to size_t after it is undefined behavior. With GCC on x64 it gives 0, so ffi.setUint8(ptr, 2 ** 64, 42) writes to ptr instead of throwing. Anything above Number.MAX_SAFE_INTEGER may already have been rounded by the time it gets here, so reject those values too. The export*() helpers already cap their length there, and so does setInt64() for number values. SIZE_MAX is still the limit on 32-bit platforms. When buffer.constants.MAX_LENGTH is Number.MAX_SAFE_INTEGER, as on 64-bit builds without the V8 sandbox, toBuffer() and toArrayBuffer() now throw ERR_OUT_OF_RANGE for MAX_LENGTH + 1 instead of ERR_BUFFER_TOO_LARGE. Signed-off-by: Christian Aurich Zanettini Martins <christian.aurichzm@gmail.com> PR-URL: #66216 Reviewed-By: René <contact.9a5d6388@renegade334.me.uk>
1 parent e7121be commit 9bdab1e

2 files changed

Lines changed: 26 additions & 3 deletions

File tree

‎src/ffi/data.cc‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,10 @@ Maybe<size_t> GetValidatedSize(Environment* env,
5050
return Nothing<size_t>();
5151
}
5252

53-
if (length > static_cast<double>(std::numeric_limits<size_t>::max())) {
53+
// Values beyond Number.MAX_SAFE_INTEGER may already have been rounded
54+
// by the caller. On 32-bit platforms SIZE_MAX is the tighter bound.
55+
if (length > kMaxSafeJsInteger ||
56+
length > static_cast<double>(std::numeric_limits<size_t>::max())) {
5457
THROW_ERR_OUT_OF_RANGE(env, "The %s is too large", label);
5558
return Nothing<size_t>();
5659
}

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

Lines changed: 22 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -342,15 +342,35 @@ test('ffi validates memory access arguments', () => {
342342

343343
assert.throws(() => ffi.toBuffer(maxPointer, 8), /pointer and length exceed the platform address range/);
344344
assert.throws(() => ffi.toArrayBuffer(maxPointer, 8), /pointer and length exceed the platform address range/);
345-
assert.throws(() => ffi.toBuffer(1n, bufferConstants.MAX_LENGTH + 1), { code: 'ERR_BUFFER_TOO_LARGE' });
346-
assert.throws(() => ffi.toArrayBuffer(1n, bufferConstants.MAX_LENGTH + 1), { code: 'ERR_BUFFER_TOO_LARGE' });
345+
346+
// If MAX_LENGTH is Number.MAX_SAFE_INTEGER, MAX_LENGTH + 1 is an unsafe
347+
// integer and is rejected before the buffer length is checked.
348+
if (bufferConstants.MAX_LENGTH < Number.MAX_SAFE_INTEGER) {
349+
assert.throws(() => ffi.toBuffer(1n, bufferConstants.MAX_LENGTH + 1), { code: 'ERR_BUFFER_TOO_LARGE' });
350+
assert.throws(() => ffi.toArrayBuffer(1n, bufferConstants.MAX_LENGTH + 1), { code: 'ERR_BUFFER_TOO_LARGE' });
351+
}
347352

348353
if (process.arch === 'ia32' || process.arch === 'arm') {
349354
assert.throws(() => ffi.toBuffer(2n ** 32n, 0), /platform pointer range/);
350355
}
351356
}));
352357
});
353358

359+
test('ffi rejects unsafe integers as an offset or length', () => {
360+
withAllocations(common.mustCall((alloc) => {
361+
const ptr = alloc(8);
362+
const range = { code: 'ERR_OUT_OF_RANGE' };
363+
364+
// On 64-bit platforms SIZE_MAX rounds up to 2 ** 64 as a double.
365+
for (const value of [Number.MAX_SAFE_INTEGER + 1, 2 ** 64]) {
366+
assert.throws(() => ffi.getUint8(ptr, value), range);
367+
assert.throws(() => ffi.setUint8(ptr, value, 42), range);
368+
assert.throws(() => ffi.toBuffer(ptr, value), range);
369+
assert.throws(() => ffi.toArrayBuffer(ptr, value), range);
370+
}
371+
}));
372+
});
373+
354374
test('ffi memory helpers reject missing required arguments', () => {
355375
const widths = ['Int8', 'Uint8', 'Int16', 'Uint16', 'Int32', 'Uint32',
356376
'Int64', 'Uint64', 'Float32', 'Float64'];

0 commit comments

Comments
 (0)