Skip to content

Commit 0558424

Browse files
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>
1 parent 3cd2d6e commit 0558424

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
@@ -322,15 +322,35 @@ test('ffi validates memory access arguments', () => {
322322

323323
assert.throws(() => ffi.toBuffer(maxPointer, 8), /pointer and length exceed the platform address range/);
324324
assert.throws(() => ffi.toArrayBuffer(maxPointer, 8), /pointer and length exceed the platform address range/);
325-
assert.throws(() => ffi.toBuffer(1n, bufferConstants.MAX_LENGTH + 1), { code: 'ERR_BUFFER_TOO_LARGE' });
326-
assert.throws(() => ffi.toArrayBuffer(1n, bufferConstants.MAX_LENGTH + 1), { code: 'ERR_BUFFER_TOO_LARGE' });
325+
326+
// If MAX_LENGTH is Number.MAX_SAFE_INTEGER, MAX_LENGTH + 1 is an unsafe
327+
// integer and is rejected before the buffer length is checked.
328+
if (bufferConstants.MAX_LENGTH < Number.MAX_SAFE_INTEGER) {
329+
assert.throws(() => ffi.toBuffer(1n, bufferConstants.MAX_LENGTH + 1), { code: 'ERR_BUFFER_TOO_LARGE' });
330+
assert.throws(() => ffi.toArrayBuffer(1n, bufferConstants.MAX_LENGTH + 1), { code: 'ERR_BUFFER_TOO_LARGE' });
331+
}
327332

328333
if (process.arch === 'ia32' || process.arch === 'arm') {
329334
assert.throws(() => ffi.toBuffer(2n ** 32n, 0), /platform pointer range/);
330335
}
331336
}));
332337
});
333338

339+
test('ffi rejects unsafe integers as an offset or length', () => {
340+
withAllocations(common.mustCall((alloc) => {
341+
const ptr = alloc(8);
342+
const range = { code: 'ERR_OUT_OF_RANGE' };
343+
344+
// On 64-bit platforms SIZE_MAX rounds up to 2 ** 64 as a double.
345+
for (const value of [Number.MAX_SAFE_INTEGER + 1, 2 ** 64]) {
346+
assert.throws(() => ffi.getUint8(ptr, value), range);
347+
assert.throws(() => ffi.setUint8(ptr, value, 42), range);
348+
assert.throws(() => ffi.toBuffer(ptr, value), range);
349+
assert.throws(() => ffi.toArrayBuffer(ptr, value), range);
350+
}
351+
}));
352+
});
353+
334354
test('ffi memory helpers reject missing required arguments', () => {
335355
const widths = ['Int8', 'Uint8', 'Int16', 'Uint16', 'Int32', 'Uint32',
336356
'Int64', 'Uint64', 'Float32', 'Float64'];

0 commit comments

Comments
 (0)