Skip to content

Commit 4a9feca

Browse files
committed
buffer: fix silent data corruption with indices >= 2^32
SlowCopy and FastCopy in node_buffer.cc used uint32_t for target_start, source_start, and to_copy parameters. When these values exceeded 2^32, they silently wrapped around, causing Buffer.copy and Buffer.concat to produce incorrect results with large buffers. Change CopyImpl parameters from uint32_t to size_t, SlowCopy to use IntegerValue (int64_t) instead of Uint32, and FastCopy to use uint64_t. Add a guard against negative int64_t values before casting to size_t to prevent potential out-of-bounds access. Note: other functions in node_buffer.cc (CopyArrayBuffer, StringWrite, FastByteLengthUtf8) have similar uint32_t limitations but are not addressed here to keep this change focused on the reported regression. Fixes: #55422
1 parent 5b5f069 commit 4a9feca

File tree

2 files changed

+84
-13
lines changed

2 files changed

+84
-13
lines changed

src/node_buffer.cc

Lines changed: 30 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -587,9 +587,9 @@ void StringSlice(const FunctionCallbackInfo<Value>& args) {
587587

588588
void CopyImpl(Local<Value> source_obj,
589589
Local<Value> target_obj,
590-
const uint32_t target_start,
591-
const uint32_t source_start,
592-
const uint32_t to_copy) {
590+
const size_t target_start,
591+
const size_t source_start,
592+
const size_t to_copy) {
593593
ArrayBufferViewContents<char> source(source_obj);
594594
SPREAD_BUFFER_ARG(target_obj, target);
595595

@@ -598,29 +598,46 @@ void CopyImpl(Local<Value> source_obj,
598598

599599
// Assume caller has properly validated args.
600600
void SlowCopy(const FunctionCallbackInfo<Value>& args) {
601+
Environment* env = Environment::GetCurrent(args);
601602
Local<Value> source_obj = args[0];
602603
Local<Value> target_obj = args[1];
603-
const uint32_t target_start = args[2].As<Uint32>()->Value();
604-
const uint32_t source_start = args[3].As<Uint32>()->Value();
605-
const uint32_t to_copy = args[4].As<Uint32>()->Value();
604+
int64_t target_start, source_start, to_copy;
605+
if (!args[2]->IntegerValue(env->context()).To(&target_start) ||
606+
!args[3]->IntegerValue(env->context()).To(&source_start) ||
607+
!args[4]->IntegerValue(env->context()).To(&to_copy)) {
608+
return;
609+
}
610+
611+
// Guard against negative values that would wrap to huge size_t.
612+
if (target_start < 0 || source_start < 0 || to_copy < 0) {
613+
return;
614+
}
606615

607-
CopyImpl(source_obj, target_obj, target_start, source_start, to_copy);
616+
CopyImpl(source_obj,
617+
target_obj,
618+
static_cast<size_t>(target_start),
619+
static_cast<size_t>(source_start),
620+
static_cast<size_t>(to_copy));
608621

609-
args.GetReturnValue().Set(to_copy);
622+
args.GetReturnValue().Set(static_cast<double>(to_copy));
610623
}
611624

612625
// Assume caller has properly validated args.
613-
uint32_t FastCopy(Local<Value> receiver,
626+
uint64_t FastCopy(Local<Value> receiver,
614627
Local<Value> source_obj,
615628
Local<Value> target_obj,
616-
uint32_t target_start,
617-
uint32_t source_start,
618-
uint32_t to_copy,
629+
uint64_t target_start,
630+
uint64_t source_start,
631+
uint64_t to_copy,
619632
// NOLINTNEXTLINE(runtime/references)
620633
FastApiCallbackOptions& options) {
621634
HandleScope scope(options.isolate);
622635

623-
CopyImpl(source_obj, target_obj, target_start, source_start, to_copy);
636+
CopyImpl(source_obj,
637+
target_obj,
638+
static_cast<size_t>(target_start),
639+
static_cast<size_t>(source_start),
640+
static_cast<size_t>(to_copy));
624641

625642
return to_copy;
626643
}
Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,54 @@
1+
'use strict';
2+
3+
// Regression test for https://github.com/nodejs/node/issues/55422
4+
// Buffer.copy and Buffer.concat silently produced incorrect results when
5+
// indices >= 2^32 due to uint32_t overflow in the native SlowCopy path.
6+
7+
const common = require('../common');
8+
const assert = require('assert');
9+
10+
// Cannot test on 32-bit platforms since buffers that large cannot exist.
11+
common.skipIf32Bits();
12+
13+
const THRESHOLD = 2 ** 32; // 4 GiB
14+
15+
// Allocate a large target buffer (just over 4 GiB). Skip if there is not
16+
// enough memory available in the current environment (e.g. CI).
17+
let target;
18+
try {
19+
target = Buffer.alloc(THRESHOLD + 10, 0);
20+
} catch (e) {
21+
if (e.code === 'ERR_MEMORY_ALLOCATION_FAILED' ||
22+
/Array buffer allocation failed/.test(e.message)) {
23+
common.skip('insufficient memory for large buffer allocation');
24+
}
25+
throw e;
26+
}
27+
28+
const source = Buffer.alloc(10, 111);
29+
30+
// Test 1: Buffer.copy with targetStart >= 2^32
31+
// Copy only the first 5 bytes so _copyActual falls through to the native
32+
// _copy (SlowCopy) instead of using TypedArrayPrototypeSet.
33+
source.copy(target, THRESHOLD, 0, 5);
34+
35+
assert.strictEqual(target[0], 0, 'position 0 must not have been overwritten');
36+
assert.strictEqual(target[THRESHOLD], 111, 'byte at THRESHOLD must be 111');
37+
assert.strictEqual(target[THRESHOLD + 4], 111, 'byte at THRESHOLD+4 must be 111');
38+
assert.strictEqual(target[THRESHOLD + 5], 0, 'byte at THRESHOLD+5 must be 0');
39+
40+
// Test 2: Buffer.copy at the 2^32 - 1 boundary (crossing the 32-bit edge)
41+
target.fill(0);
42+
source.copy(target, THRESHOLD - 2, 0, 5);
43+
assert.strictEqual(target[THRESHOLD - 2], 111, 'byte at boundary start');
44+
assert.strictEqual(target[THRESHOLD + 2], 111, 'byte crossing boundary');
45+
assert.strictEqual(target[THRESHOLD + 3], 0, 'byte after copied range');
46+
47+
// Test 3: Buffer.concat producing a result with total length > 2^32
48+
// concat uses TypedArrayPrototypeSet internally, which should handle large
49+
// offsets correctly. Verify this explicitly.
50+
const small = Buffer.alloc(2, 115);
51+
const result = Buffer.concat([target, small]);
52+
assert.strictEqual(result.length, THRESHOLD + 12);
53+
assert.strictEqual(result[THRESHOLD + 10], 115, 'concat: byte from second buffer');
54+
assert.strictEqual(result[THRESHOLD + 11], 115, 'concat: second byte from second buffer');

0 commit comments

Comments
 (0)