diff --git a/src/node_buffer.cc b/src/node_buffer.cc index d4a63cf610ca7f..79fd433b712355 100644 --- a/src/node_buffer.cc +++ b/src/node_buffer.cc @@ -587,9 +587,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); @@ -598,24 +598,32 @@ void CopyImpl(Local source_obj, // Assume caller has properly validated args. void SlowCopy(const FunctionCallbackInfo& args) { + Environment* env = Environment::GetCurrent(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(); + int64_t target_start, source_start, to_copy; + if (!args[2]->IntegerValue(env->context()).To(&target_start) || + !args[3]->IntegerValue(env->context()).To(&source_start) || + !args[4]->IntegerValue(env->context()).To(&to_copy)) { + return; + } - 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)); - args.GetReturnValue().Set(to_copy); + args.GetReturnValue().Set(static_cast(to_copy)); } // Assume caller has properly validated args. -uint32_t FastCopy(Local receiver, +uint64_t FastCopy(Local receiver, Local source_obj, Local target_obj, - uint32_t target_start, - uint32_t source_start, - uint32_t to_copy, + uint64_t target_start, + uint64_t source_start, + uint64_t to_copy, // NOLINTNEXTLINE(runtime/references) FastApiCallbackOptions& options) { HandleScope scope(options.isolate); diff --git a/test/parallel/test-buffer-copy-large.js b/test/parallel/test-buffer-copy-large.js new file mode 100644 index 00000000000000..5580fc455fcbda --- /dev/null +++ b/test/parallel/test-buffer-copy-large.js @@ -0,0 +1,44 @@ +'use strict'; + +// Regression test for https://github.com/nodejs/node/issues/55422 +// Buffer.copy and Buffer.concat silently produced incorrect results when +// indices involved were >= 2^32 due to 32-bit integer overflow in SlowCopy. + +const common = require('../common'); +const assert = require('assert'); + +// This test exercises the native SlowCopy path in node_buffer.cc. +// SlowCopy is invoked by _copyActual when the fast TypedArrayPrototypeSet +// path cannot be used (i.e. when sourceStart !== 0 or nb !== sourceLen). + +// Cannot test on 32-bit machines since buffers that large cannot exist there. +common.skipIf32Bits(); + +const THRESHOLD = 2 ** 32; // 4 GiB + +// Allocate a large target buffer (just over 4 GiB) to test that a targetStart +// value >= 2^32 is not silently truncated to its lower 32 bits. +let target; +try { + target = Buffer.alloc(THRESHOLD + 10, 0); +} catch (e) { + if (e.code === 'ERR_MEMORY_ALLOCATION_FAILED' || + /Array buffer allocation failed/.test(e.message)) { + common.skip('insufficient memory for large buffer allocation'); + } + throw e; +} + +const source = Buffer.alloc(10, 111); + +// Copy only the first 5 bytes of source (nb=5, sourceLen=10 → nb !== sourceLen) +// so _copyActual falls through to the native _copy (SlowCopy) instead of +// using TypedArrayPrototypeSet. The targetStart is THRESHOLD (2^32), which +// previously overflowed to 0 when cast to uint32_t. +source.copy(target, THRESHOLD, 0, 5); + +// The 5 copied bytes must appear at position THRESHOLD, not at position 0. +assert.strictEqual(target[0], 0, 'position 0 must not have been overwritten'); +assert.strictEqual(target[THRESHOLD], 111, 'byte at THRESHOLD must be 111'); +assert.strictEqual(target[THRESHOLD + 4], 111, 'byte at THRESHOLD+4 must be 111'); +assert.strictEqual(target[THRESHOLD + 5], 0, 'byte at THRESHOLD+5 must be 0 (not copied)');