Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Commit d4d9f39

Browse filesBrowse 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 <rafael.nunu@hotmail.com> Co-authored-by: Joyee Cheung <joyeec9h3@gmail.com> Signed-off-by: RafaelGSS <rafael.nunu@hotmail.com> 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
Copy full SHA for d4d9f39

File tree

Expand file treeCollapse file tree

5 files changed

+78
-55
lines changed
Open diff view settings
Filter options
Expand file treeCollapse file tree

5 files changed

+78
-55
lines changed
Open diff view settings
Collapse file

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

Copy file name to clipboardExpand all lines: deps/v8/include/v8-array-buffer.h
+7Lines 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
Collapse file

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

Copy file name to clipboardExpand all lines: deps/v8/src/api/api.cc
+17Lines 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) {
Collapse file

‎lib/internal/buffer.js‎

Copy file name to clipboardExpand all lines: lib/internal/buffer.js
+5-18Lines 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
};
Collapse file

‎lib/internal/process/pre_execution.js‎

Copy file name to clipboardExpand all lines: lib/internal/process/pre_execution.js
-2Lines 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);
Collapse file

‎src/node_buffer.cc‎

Copy file name to clipboardExpand all lines: src/node_buffer.cc
+49-35Lines 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 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.