Skip to content

Commit d4d9f39

Browse files
ChALkeRRafaelGSSjoyeecheung
committed
src,lib: refactor unsafe buffer creation to remove zero-fill toggle
This removes the zero-fill toggle mechanism that allowed JavaScript to control ArrayBuffer initialization via shared memory. Instead, unsafe buffer creation now uses a dedicated C++ API. Refs: https://hackerone.com/reports/3405778 Co-Authored-By: Rafael Gonzaga <[email protected]> Co-authored-by: Joyee Cheung <[email protected]> Signed-off-by: RafaelGSS <[email protected]> PR-URL: nodejs-private/node-private#759 Backport-PR-URL: nodejs-private/node-private#798 CVE-ID: CVE-2025-55131
1 parent 6badf4e commit d4d9f39

File tree

5 files changed

+78
-55
lines changed

5 files changed

+78
-55
lines changed

‎deps/v8/include/v8-array-buffer.h‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -244,6 +244,13 @@ class V8_EXPORT ArrayBuffer : public Object {
244244
*/
245245
static std::unique_ptr<BackingStore> NewBackingStore(Isolate* isolate,
246246
size_t byte_length);
247+
/**
248+
* Returns a new standalone BackingStore with uninitialized memory and
249+
* return nullptr on failure.
250+
* This variant is for not breaking ABI on Node.js LTS. DO NOT USE.
251+
*/
252+
static std::unique_ptr<BackingStore> NewBackingStoreForNodeLTS(
253+
Isolate* isolate, size_t byte_length);
247254
/**
248255
* Returns a new standalone BackingStore that takes over the ownership of
249256
* the given buffer. The destructor of the BackingStore invokes the given

‎deps/v8/src/api/api.cc‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8930,6 +8930,23 @@ std::unique_ptr<v8::BackingStore> v8::ArrayBuffer::NewBackingStore(
89308930
static_cast<v8::BackingStore*>(backing_store.release()));
89318931
}
89328932

8933+
std::unique_ptr<v8::BackingStore> v8::ArrayBuffer::NewBackingStoreForNodeLTS(
8934+
Isolate* v8_isolate, size_t byte_length) {
8935+
i::Isolate* i_isolate = reinterpret_cast<i::Isolate*>(v8_isolate);
8936+
API_RCS_SCOPE(i_isolate, ArrayBuffer, NewBackingStore);
8937+
CHECK_LE(byte_length, i::JSArrayBuffer::kMaxByteLength);
8938+
ENTER_V8_NO_SCRIPT_NO_EXCEPTION(i_isolate);
8939+
std::unique_ptr<i::BackingStoreBase> backing_store =
8940+
i::BackingStore::Allocate(i_isolate, byte_length,
8941+
i::SharedFlag::kNotShared,
8942+
i::InitializedFlag::kUninitialized);
8943+
if (!backing_store) {
8944+
return nullptr;
8945+
}
8946+
return std::unique_ptr<v8::BackingStore>(
8947+
static_cast<v8::BackingStore*>(backing_store.release()));
8948+
}
8949+
89338950
std::unique_ptr<v8::BackingStore> v8::ArrayBuffer::NewBackingStore(
89348951
void* data, size_t byte_length, v8::BackingStore::DeleterCallback deleter,
89358952
void* deleter_data) {

‎lib/internal/buffer.js‎

Lines changed: 5 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ const {
3030
hexWrite,
3131
ucs2Write,
3232
utf8WriteStatic,
33-
getZeroFillToggle,
33+
createUnsafeArrayBuffer,
3434
} = internalBinding('buffer');
3535

3636
const {
@@ -1079,26 +1079,14 @@ function isMarkedAsUntransferable(obj) {
10791079
return obj[untransferable_object_private_symbol] !== undefined;
10801080
}
10811081

1082-
// A toggle used to access the zero fill setting of the array buffer allocator
1083-
// in C++.
1084-
// |zeroFill| can be undefined when running inside an isolate where we
1085-
// do not own the ArrayBuffer allocator. Zero fill is always on in that case.
1086-
let zeroFill = getZeroFillToggle();
10871082
function createUnsafeBuffer(size) {
1088-
zeroFill[0] = 0;
1089-
try {
1083+
if (size <= 64) {
1084+
// Allocated in heap, doesn't call backing store anyway
1085+
// This is the same that the old impl did implicitly, but explicit now
10901086
return new FastBuffer(size);
1091-
} finally {
1092-
zeroFill[0] = 1;
10931087
}
1094-
}
10951088

1096-
// The connection between the JS land zero fill toggle and the
1097-
// C++ one in the NodeArrayBufferAllocator gets lost if the toggle
1098-
// is deserialized from the snapshot, because V8 owns the underlying
1099-
// memory of this toggle. This resets the connection.
1100-
function reconnectZeroFillToggle() {
1101-
zeroFill = getZeroFillToggle();
1089+
return new FastBuffer(createUnsafeArrayBuffer(size));
11021090
}
11031091

11041092
module.exports = {
@@ -1109,5 +1097,4 @@ module.exports = {
11091097
createUnsafeBuffer,
11101098
readUInt16BE,
11111099
readUInt32BE,
1112-
reconnectZeroFillToggle,
11131100
};

‎lib/internal/process/pre_execution.js‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,6 @@ const {
2323
refreshOptions,
2424
getEmbedderOptions,
2525
} = require('internal/options');
26-
const { reconnectZeroFillToggle } = require('internal/buffer');
2726
const {
2827
exposeInterface,
2928
exposeLazyInterfaces,
@@ -98,7 +97,6 @@ function prepareExecution(options) {
9897
const { expandArgv1, initializeModules, isMainThread } = options;
9998

10099
refreshRuntimeOptions();
101-
reconnectZeroFillToggle();
102100

103101
// Patch the process object and get the resolved main entry point.
104102
const mainEntry = patchProcessObject(expandArgv1);

‎src/node_buffer.cc‎

Lines changed: 49 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -77,7 +77,6 @@ using v8::Object;
7777
using v8::SharedArrayBuffer;
7878
using v8::String;
7979
using v8::Uint32;
80-
using v8::Uint32Array;
8180
using v8::Uint8Array;
8281
using v8::Value;
8382

@@ -1229,37 +1228,6 @@ void SetBufferPrototype(const FunctionCallbackInfo<Value>& args) {
12291228
realm->set_buffer_prototype_object(proto);
12301229
}
12311230

1232-
void GetZeroFillToggle(const FunctionCallbackInfo<Value>& args) {
1233-
Environment* env = Environment::GetCurrent(args);
1234-
NodeArrayBufferAllocator* allocator = env->isolate_data()->node_allocator();
1235-
Local<ArrayBuffer> ab;
1236-
// It can be a nullptr when running inside an isolate where we
1237-
// do not own the ArrayBuffer allocator.
1238-
if (allocator == nullptr) {
1239-
// Create a dummy Uint32Array - the JS land can only toggle the C++ land
1240-
// setting when the allocator uses our toggle. With this the toggle in JS
1241-
// land results in no-ops.
1242-
ab = ArrayBuffer::New(env->isolate(), sizeof(uint32_t));
1243-
} else {
1244-
uint32_t* zero_fill_field = allocator->zero_fill_field();
1245-
std::unique_ptr<BackingStore> backing =
1246-
ArrayBuffer::NewBackingStore(zero_fill_field,
1247-
sizeof(*zero_fill_field),
1248-
[](void*, size_t, void*) {},
1249-
nullptr);
1250-
ab = ArrayBuffer::New(env->isolate(), std::move(backing));
1251-
}
1252-
1253-
if (ab->SetPrivate(env->context(),
1254-
env->untransferable_object_private_symbol(),
1255-
True(env->isolate()))
1256-
.IsNothing()) {
1257-
return;
1258-
}
1259-
1260-
args.GetReturnValue().Set(Uint32Array::New(ab, 0, 1));
1261-
}
1262-
12631231
static void Btoa(const FunctionCallbackInfo<Value>& args) {
12641232
CHECK_EQ(args.Length(), 1);
12651233
Environment* env = Environment::GetCurrent(args);
@@ -1433,6 +1401,52 @@ void CopyArrayBuffer(const FunctionCallbackInfo<Value>& args) {
14331401
memcpy(dest, src, bytes_to_copy);
14341402
}
14351403

1404+
// Converts a number parameter to size_t suitable for ArrayBuffer sizes
1405+
// Could be larger than uint32_t
1406+
// See v8::internal::TryNumberToSize and v8::internal::NumberToSize
1407+
inline size_t CheckNumberToSize(Local<Value> number) {
1408+
CHECK(number->IsNumber());
1409+
double value = number.As<Number>()->Value();
1410+
// See v8::internal::TryNumberToSize on this (and on < comparison)
1411+
double maxSize = static_cast<double>(std::numeric_limits<size_t>::max());
1412+
CHECK(value >= 0 && value < maxSize);
1413+
size_t size = static_cast<size_t>(value);
1414+
#ifdef V8_ENABLE_SANDBOX
1415+
CHECK_LE(size, kMaxSafeBufferSizeForSandbox);
1416+
#endif
1417+
return size;
1418+
}
1419+
1420+
void CreateUnsafeArrayBuffer(const FunctionCallbackInfo<Value>& args) {
1421+
Environment* env = Environment::GetCurrent(args);
1422+
if (args.Length() != 1) {
1423+
env->ThrowRangeError("Invalid array buffer length");
1424+
return;
1425+
}
1426+
1427+
size_t size = CheckNumberToSize(args[0]);
1428+
1429+
Isolate* isolate = env->isolate();
1430+
1431+
Local<ArrayBuffer> buf;
1432+
1433+
NodeArrayBufferAllocator* allocator = env->isolate_data()->node_allocator();
1434+
// 0-length, or zero-fill flag is set, or building snapshot
1435+
if (size == 0 || per_process::cli_options->zero_fill_all_buffers ||
1436+
allocator == nullptr) {
1437+
buf = ArrayBuffer::New(isolate, size);
1438+
} else {
1439+
std::unique_ptr<BackingStore> store =
1440+
ArrayBuffer::NewBackingStoreForNodeLTS(isolate, size);
1441+
if (!store) {
1442+
return env->ThrowRangeError("Array buffer allocation failed");
1443+
}
1444+
buf = ArrayBuffer::New(isolate, std::move(store));
1445+
}
1446+
1447+
args.GetReturnValue().Set(buf);
1448+
}
1449+
14361450
template <encoding encoding>
14371451
uint32_t WriteOneByteString(const char* src,
14381452
uint32_t src_len,
@@ -1550,6 +1564,8 @@ void Initialize(Local<Object> target,
15501564
SetMethodNoSideEffect(context, target, "indexOfString", IndexOfString);
15511565

15521566
SetMethod(context, target, "copyArrayBuffer", CopyArrayBuffer);
1567+
SetMethodNoSideEffect(
1568+
context, target, "createUnsafeArrayBuffer", CreateUnsafeArrayBuffer);
15531569

15541570
SetMethod(context, target, "swap16", Swap16);
15551571
SetMethod(context, target, "swap32", Swap32);
@@ -1599,8 +1615,6 @@ void Initialize(Local<Object> target,
15991615
"utf8WriteStatic",
16001616
SlowWriteString<UTF8>,
16011617
&fast_write_string_utf8);
1602-
1603-
SetMethod(context, target, "getZeroFillToggle", GetZeroFillToggle);
16041618
}
16051619

16061620
} // anonymous namespace
@@ -1649,9 +1663,9 @@ void RegisterExternalReferences(ExternalReferenceRegistry* registry) {
16491663
registry->Register(StringWrite<HEX>);
16501664
registry->Register(StringWrite<UCS2>);
16511665
registry->Register(StringWrite<UTF8>);
1652-
registry->Register(GetZeroFillToggle);
16531666

16541667
registry->Register(CopyArrayBuffer);
1668+
registry->Register(CreateUnsafeArrayBuffer);
16551669

16561670
registry->Register(Atob);
16571671
registry->Register(Btoa);

0 commit comments

Comments
 (0)