Skip to content

Commit 3870dee

Browse files
committed
buffer: avoid 32-bit truncation in copy beyond 2 GiB
`Buffer.prototype.copy` and `copyArrayBuffer` use Uint32Value() to read offset and length arguments in C++. Values larger than 2**32 are silently truncated, so a 4 GiB+ copy quietly drops bytes. Switch to IntegerValue() and widen internal types to size_t so the slow path handles the full kMaxLength range. The fast path keeps its uint32_t signature; values beyond UINT32_MAX naturally fall through to the slow path. Add a regression test gated on NODE_TEST_LARGE_BUFFER (matches test-buffer-tostring-4gb.js gating) so default CI does not need to allocate >4 GiB. Fixes: #55422 Signed-off-by: Maruthan G <[email protected]>
1 parent 4744070 commit 3870dee

2 files changed

Lines changed: 117 additions & 16 deletions

File tree

src/node_buffer.cc

Lines changed: 41 additions & 16 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,15 +598,29 @@ 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();
606-
607-
CopyImpl(source_obj, target_obj, target_start, source_start, to_copy);
608-
609-
args.GetReturnValue().Set(to_copy);
604+
// Use IntegerValue() rather than Uint32Value() so that values larger than
605+
// 2**32 are not silently truncated. The JS layer is responsible for
606+
// ensuring these are non-negative safe integers.
607+
const int64_t target_start =
608+
args[2]->IntegerValue(env->context()).ToChecked();
609+
const int64_t source_start =
610+
args[3]->IntegerValue(env->context()).ToChecked();
611+
const int64_t to_copy = args[4]->IntegerValue(env->context()).ToChecked();
612+
613+
CHECK_GE(target_start, 0);
614+
CHECK_GE(source_start, 0);
615+
CHECK_GE(to_copy, 0);
616+
617+
CopyImpl(source_obj,
618+
target_obj,
619+
static_cast<size_t>(target_start),
620+
static_cast<size_t>(source_start),
621+
static_cast<size_t>(to_copy));
622+
623+
args.GetReturnValue().Set(static_cast<double>(to_copy));
610624
}
611625

612626
// Assume caller has properly validated args.
@@ -1480,11 +1494,12 @@ void CopyArrayBuffer(const FunctionCallbackInfo<Value>& args) {
14801494
// args[3] == Source ArrayBuffer Offset
14811495
// args[4] == bytesToCopy
14821496

1497+
Environment* env = Environment::GetCurrent(args);
14831498
CHECK(args[0]->IsArrayBuffer() || args[0]->IsSharedArrayBuffer());
1484-
CHECK(args[1]->IsUint32());
1499+
CHECK(args[1]->IsNumber());
14851500
CHECK(args[2]->IsArrayBuffer() || args[2]->IsSharedArrayBuffer());
1486-
CHECK(args[3]->IsUint32());
1487-
CHECK(args[4]->IsUint32());
1501+
CHECK(args[3]->IsNumber());
1502+
CHECK(args[4]->IsNumber());
14881503

14891504
void* destination;
14901505
size_t destination_byte_length;
@@ -1495,9 +1510,19 @@ void CopyArrayBuffer(const FunctionCallbackInfo<Value>& args) {
14951510
size_t source_byte_length;
14961511
std::tie(source, source_byte_length) = DecomposeBufferToParts(args[2]);
14971512

1498-
uint32_t destination_offset = args[1].As<Uint32>()->Value();
1499-
uint32_t source_offset = args[3].As<Uint32>()->Value();
1500-
size_t bytes_to_copy = args[4].As<Uint32>()->Value();
1513+
// Use IntegerValue() so offsets larger than 2**32 are not truncated.
1514+
const int64_t destination_offset_signed =
1515+
args[1]->IntegerValue(env->context()).ToChecked();
1516+
const int64_t source_offset_signed =
1517+
args[3]->IntegerValue(env->context()).ToChecked();
1518+
const int64_t bytes_to_copy_signed =
1519+
args[4]->IntegerValue(env->context()).ToChecked();
1520+
CHECK_GE(destination_offset_signed, 0);
1521+
CHECK_GE(source_offset_signed, 0);
1522+
CHECK_GE(bytes_to_copy_signed, 0);
1523+
size_t destination_offset = static_cast<size_t>(destination_offset_signed);
1524+
size_t source_offset = static_cast<size_t>(source_offset_signed);
1525+
size_t bytes_to_copy = static_cast<size_t>(bytes_to_copy_signed);
15011526

15021527
CHECK_GE(destination_byte_length - destination_offset, bytes_to_copy);
15031528
CHECK_GE(source_byte_length - source_offset, bytes_to_copy);
Lines changed: 76 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,76 @@
1+
'use strict';
2+
3+
// This tests that Buffer.prototype.copy correctly handles copies whose
4+
// source/target offsets or byte counts exceed 2**32. Refs:
5+
// https://github.com/nodejs/node/issues/55422
6+
const common = require('../common');
7+
8+
// Cannot test on 32-bit machines because the test relies on creating
9+
// buffers larger than 4 GiB.
10+
common.skipIf32Bits();
11+
12+
// Allocating 4+ GiB buffers in CI environments is expensive and may also
13+
// fail under sanitizer / memory-constrained builds. Gate the test behind
14+
// an explicit opt-in env var to avoid timeouts/OOMs in normal CI runs.
15+
if (!process.env.NODE_TEST_LARGE_BUFFER) {
16+
common.skip('Skipping: requires NODE_TEST_LARGE_BUFFER=1 (allocates >4GiB)');
17+
}
18+
19+
const assert = require('assert');
20+
21+
const threshold = 0xFFFFFFFF; // 2**32 - 1
22+
const overflow = threshold + 5; // 2**32 + 4 — exercises offsets > 2**32
23+
24+
let largeBuffer;
25+
try {
26+
// Allocate a buffer slightly larger than 2**32 so we can copy to and from
27+
// offsets above the uint32 boundary.
28+
largeBuffer = Buffer.alloc(overflow);
29+
} catch (e) {
30+
if (e.code === 'ERR_MEMORY_ALLOCATION_FAILED' ||
31+
/Array buffer allocation failed/.test(e.message)) {
32+
common.skip('insufficient memory for >4GiB Buffer.alloc');
33+
}
34+
throw e;
35+
}
36+
37+
// Sentinel byte at an index above 2**32. Before the fix, copy() truncates the
38+
// length to 32 bits and the sentinel never gets written, so reading it back
39+
// would yield 0.
40+
const sentinelIndex = threshold + 2;
41+
largeBuffer[sentinelIndex] = 0xAB;
42+
43+
// Test 1: Buffer.prototype.copy with sourceEnd > 2**32 should copy the bytes
44+
// past the 4 GiB boundary instead of silently truncating.
45+
{
46+
const target = Buffer.alloc(overflow);
47+
const copied = largeBuffer.copy(target, 0, 0, overflow);
48+
assert.strictEqual(copied, overflow,
49+
'copy() should report copying the full byte range');
50+
assert.strictEqual(target[sentinelIndex], 0xAB,
51+
'byte beyond 2**32 must be copied, not truncated');
52+
}
53+
54+
// Test 2: Buffer.prototype.copy with targetStart > 2**32 should write at the
55+
// large offset rather than wrapping back to a low address.
56+
{
57+
const target = Buffer.alloc(overflow);
58+
const src = Buffer.from([0xCD]);
59+
const copied = src.copy(target, threshold + 1, 0, 1);
60+
assert.strictEqual(copied, 1);
61+
assert.strictEqual(target[threshold + 1], 0xCD,
62+
'targetStart > 2**32 must not wrap to a low offset');
63+
// The low offset that uint32 truncation would have written to must remain
64+
// untouched.
65+
assert.strictEqual(target[(threshold + 1) >>> 0], 0);
66+
}
67+
68+
// Test 3: Buffer.concat with a single >4 GiB buffer should preserve the
69+
// trailing bytes, exercising the original repro from the issue.
70+
{
71+
largeBuffer.fill(0x6F); // 'o'
72+
const result = Buffer.concat([largeBuffer]);
73+
assert.strictEqual(result.length, overflow);
74+
assert.strictEqual(result[overflow - 1], 0x6F,
75+
'final byte beyond 2**32 must survive concat');
76+
}

0 commit comments

Comments
 (0)