Skip to content

Commit 9313bcf

Browse files
author
Your Name
committed
Fix #55422: Buffer.concat and Buffer.copy silently produce invalid r
1 parent f632f3f commit 9313bcf

File tree

2 files changed

+60
-8
lines changed

2 files changed

+60
-8
lines changed

src/node_buffer.cc

Lines changed: 16 additions & 8 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,23 @@ 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+
}
606610

607-
CopyImpl(source_obj, target_obj, target_start, source_start, to_copy);
611+
CopyImpl(source_obj,
612+
target_obj,
613+
static_cast<size_t>(target_start),
614+
static_cast<size_t>(source_start),
615+
static_cast<size_t>(to_copy));
608616

609-
args.GetReturnValue().Set(to_copy);
617+
args.GetReturnValue().Set(static_cast<double>(to_copy));
610618
}
611619

612620
// Assume caller has properly validated args.
Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,44 @@
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 involved were >= 2^32 due to 32-bit integer overflow in SlowCopy.
6+
7+
const common = require('../common');
8+
const assert = require('assert');
9+
10+
// This test exercises the native SlowCopy path in node_buffer.cc.
11+
// SlowCopy is invoked by _copyActual when the fast TypedArrayPrototypeSet
12+
// path cannot be used (i.e. when sourceStart !== 0 or nb !== sourceLen).
13+
14+
// Cannot test on 32-bit machines since buffers that large cannot exist there.
15+
common.skipIf32Bits();
16+
17+
const THRESHOLD = 2 ** 32; // 4 GiB
18+
19+
// Allocate a large target buffer (just over 4 GiB) to test that a targetStart
20+
// value >= 2^32 is not silently truncated to its lower 32 bits.
21+
let target;
22+
try {
23+
target = Buffer.alloc(THRESHOLD + 10, 0);
24+
} catch (e) {
25+
if (e.code === 'ERR_MEMORY_ALLOCATION_FAILED' ||
26+
/Array buffer allocation failed/.test(e.message)) {
27+
common.skip('insufficient memory for large buffer allocation');
28+
}
29+
throw e;
30+
}
31+
32+
const source = Buffer.alloc(10, 111);
33+
34+
// Copy only the first 5 bytes of source (nb=5, sourceLen=10 → nb !== sourceLen)
35+
// so _copyActual falls through to the native _copy (SlowCopy) instead of
36+
// using TypedArrayPrototypeSet. The targetStart is THRESHOLD (2^32), which
37+
// previously overflowed to 0 when cast to uint32_t.
38+
source.copy(target, THRESHOLD, 0, 5);
39+
40+
// The 5 copied bytes must appear at position THRESHOLD, not at position 0.
41+
assert.strictEqual(target[0], 0, 'position 0 must not have been overwritten');
42+
assert.strictEqual(target[THRESHOLD], 111, 'byte at THRESHOLD must be 111');
43+
assert.strictEqual(target[THRESHOLD + 4], 111, 'byte at THRESHOLD+4 must be 111');
44+
assert.strictEqual(target[THRESHOLD + 5], 0, 'byte at THRESHOLD+5 must be 0 (not copied)');

0 commit comments

Comments
 (0)