From 834cdea8781cc90018a25125dba541d11216c31e Mon Sep 17 00:00:00 2001 From: INTFRAME Date: Tue, 22 Sep 2026 03:24:29 +0700 Subject: [PATCH] buffer: fix 32-bit truncation in Buffer.prototype.copy `SlowCopy` read `targetStart`, `sourceStart` and the byte count with `As()`, and `FastCopy` took them as `uint32_t`, so a value at or above 2**32 wrapped around: the copy silently read from or wrote to the wrong offset, or moved only `count % 2**32` bytes. Read them as doubles into `size_t` instead, as `main` does since 4383f67279. That commit also routes the copy through a new V8 API and is labeled dont-land-on-v24.x, so this keeps the existing memmove path and only widens the argument types. Fixes: https://github.com/nodejs/node/issues/55422 Refs: https://github.com/nodejs/node/pull/63828 Signed-off-by: INTFRAME --- src/node_buffer.cc | 41 ++++++++++-------- test/pummel/test-buffer-large-size-copy.js | 48 ++++++++++++++++++++++ 2 files changed, 73 insertions(+), 16 deletions(-) create mode 100644 test/pummel/test-buffer-large-size-copy.js diff --git a/src/node_buffer.cc b/src/node_buffer.cc index bc0f03fdc8f3..17350817016e 100644 --- a/src/node_buffer.cc +++ b/src/node_buffer.cc @@ -599,9 +599,9 @@ void StringSlice(const FunctionCallbackInfo& args) { void CopyImpl(Local source_obj, Local target_obj, - const uint32_t target_start, - const uint32_t source_start, - const uint32_t to_copy) { + const size_t target_start, + const size_t source_start, + const size_t to_copy) { ArrayBufferViewContents source(source_obj); SPREAD_BUFFER_ARG(target_obj, target); @@ -612,27 +612,36 @@ void CopyImpl(Local source_obj, void SlowCopy(const FunctionCallbackInfo& args) { Local source_obj = args[0]; Local target_obj = args[1]; - const uint32_t target_start = args[2].As()->Value(); - const uint32_t source_start = args[3].As()->Value(); - const uint32_t to_copy = args[4].As()->Value(); + // Byte offsets and lengths can exceed uint32 for buffers larger than 4 GiB, + // so they are passed and returned as doubles (exact for integers < 2^53). + const size_t target_start = + static_cast(args[2].As()->Value()); + const size_t source_start = + static_cast(args[3].As()->Value()); + const size_t to_copy = static_cast(args[4].As()->Value()); CopyImpl(source_obj, target_obj, target_start, source_start, to_copy); - args.GetReturnValue().Set(to_copy); + args.GetReturnValue().Set(static_cast(to_copy)); } // Assume caller has properly validated args. -uint32_t FastCopy(Local receiver, - Local source_obj, - Local target_obj, - uint32_t target_start, - uint32_t source_start, - uint32_t to_copy, - // NOLINTNEXTLINE(runtime/references) - FastApiCallbackOptions& options) { +double FastCopy(Local receiver, + Local source_obj, + Local target_obj, + double target_start, + double source_start, + double to_copy, + // NOLINTNEXTLINE(runtime/references) + FastApiCallbackOptions& options) { + TRACK_V8_FAST_API_CALL("buffer.copy"); HandleScope scope(options.isolate); - CopyImpl(source_obj, target_obj, target_start, source_start, to_copy); + CopyImpl(source_obj, + target_obj, + static_cast(target_start), + static_cast(source_start), + static_cast(to_copy)); return to_copy; } diff --git a/test/pummel/test-buffer-large-size-copy.js b/test/pummel/test-buffer-large-size-copy.js new file mode 100644 index 000000000000..5db818992270 --- /dev/null +++ b/test/pummel/test-buffer-large-size-copy.js @@ -0,0 +1,48 @@ +'use strict'; +const common = require('../common'); + +// Buffer.prototype.copy with offsets and a byte count > 2 ** 32. Regression +// test for the .copy() side of https://github.com/nodejs/node/issues/55422, +// where the byte count and the offsets were truncated to 32 bits. +common.skipIf32Bits(); + +const assert = require('node:assert'); + +// A little past the 2 ** 32 boundary, with headroom for the shift below. +const size = 2 ** 32 + 16; + +let buf; +try { + buf = Buffer.alloc(size); +} catch (e) { + if ( + e.code === 'ERR_MEMORY_ALLOCATION_FAILED' || + /Array buffer allocation failed/.test(e.message) + ) { + common.skip('insufficient memory for Buffer.alloc'); + } + + throw e; +} + +// Place a marker near the end of the source range, at an index > 2 ** 32. +const marker = 0x42; +buf[size - 9] = marker; + +// Shift the whole buffer right by 8 bytes within itself. `to_copy` is +// size - 8 (> 2 ** 32), so a truncated count would copy only 8 bytes. The +// source byte at size - 9 must land at size - 1. +const copied = buf.copy(buf, 8, 0, size - 8); + +// The return value must not be truncated to 32 bits ... +assert.strictEqual(copied, size - 8); +// ... and the byte must actually have moved across the 2 ** 32 boundary. +assert.strictEqual(buf[size - 1], marker); + +// `sourceStart` and `targetStart` past 2 ** 32 must not be truncated either: +// copy one byte from size - 2 to size - 1, both of which are > 2 ** 32. +const marker2 = 0x43; +buf[size - 2] = marker2; +buf[size - 1] = 0; +assert.strictEqual(buf.copy(buf, size - 1, size - 2, size - 1), 1); +assert.strictEqual(buf[size - 1], marker2);