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

Author: 3 people

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

@@ -244,6 +244,13 @@ class V8_EXPORT ArrayBuffer : public Object {
    */
   static std::unique_ptr<BackingStore> NewBackingStore(Isolate* isolate,
                                                        size_t byte_length);
+  /**
+   * Returns a new standalone BackingStore with uninitialized memory and
+   * return nullptr on failure.
+   * This variant is for not breaking ABI on Node.js LTS. DO NOT USE.
+   */
+  static std::unique_ptr<BackingStore> NewBackingStoreForNodeLTS(
+      Isolate* isolate, size_t byte_length);
   /**
    * Returns a new standalone BackingStore that takes over the ownership of
    * the given buffer. The destructor of the BackingStore invokes the given

deps/v8/src/api/api.cc

@@ -8930,6 +8930,23 @@ std::unique_ptr<v8::BackingStore> v8::ArrayBuffer::NewBackingStore(
       static_cast<v8::BackingStore*>(backing_store.release()));
 }
 
+std::unique_ptr<v8::BackingStore> v8::ArrayBuffer::NewBackingStoreForNodeLTS(
+    Isolate* v8_isolate, size_t byte_length) {
+  i::Isolate* i_isolate = reinterpret_cast<i::Isolate*>(v8_isolate);
+  API_RCS_SCOPE(i_isolate, ArrayBuffer, NewBackingStore);
+  CHECK_LE(byte_length, i::JSArrayBuffer::kMaxByteLength);
+  ENTER_V8_NO_SCRIPT_NO_EXCEPTION(i_isolate);
+  std::unique_ptr<i::BackingStoreBase> backing_store =
+      i::BackingStore::Allocate(i_isolate, byte_length,
+                                i::SharedFlag::kNotShared,
+                                i::InitializedFlag::kUninitialized);
+  if (!backing_store) {
+    return nullptr;
+  }
+  return std::unique_ptr<v8::BackingStore>(
+      static_cast<v8::BackingStore*>(backing_store.release()));
+}
+
 std::unique_ptr<v8::BackingStore> v8::ArrayBuffer::NewBackingStore(
     void* data, size_t byte_length, v8::BackingStore::DeleterCallback deleter,
     void* deleter_data) {

lib/internal/buffer.js

@@ -30,7 +30,7 @@ const {
   hexWrite,
   ucs2Write,
   utf8WriteStatic,
-  getZeroFillToggle,
+  createUnsafeArrayBuffer,
 } = internalBinding('buffer');
 
 const {
@@ -1079,26 +1079,14 @@ function isMarkedAsUntransferable(obj) {
   return obj[untransferable_object_private_symbol] !== undefined;
 }
 
-// A toggle used to access the zero fill setting of the array buffer allocator
-// in C++.
-// |zeroFill| can be undefined when running inside an isolate where we
-// do not own the ArrayBuffer allocator.  Zero fill is always on in that case.
-let zeroFill = getZeroFillToggle();
 function createUnsafeBuffer(size) {
-  zeroFill[0] = 0;
-  try {
+  if (size <= 64) {
+    // Allocated in heap, doesn't call backing store anyway
+    // This is the same that the old impl did implicitly, but explicit now
     return new FastBuffer(size);
-  } finally {
-    zeroFill[0] = 1;
   }
-}
 
-// The connection between the JS land zero fill toggle and the
-// C++ one in the NodeArrayBufferAllocator gets lost if the toggle
-// is deserialized from the snapshot, because V8 owns the underlying
-// memory of this toggle. This resets the connection.
-function reconnectZeroFillToggle() {
-  zeroFill = getZeroFillToggle();
+  return new FastBuffer(createUnsafeArrayBuffer(size));
 }
 
 module.exports = {
@@ -1109,5 +1097,4 @@ module.exports = {
   createUnsafeBuffer,
   readUInt16BE,
   readUInt32BE,
-  reconnectZeroFillToggle,
 };

lib/internal/process/pre_execution.js

@@ -23,7 +23,6 @@ const {
   refreshOptions,
   getEmbedderOptions,
 } = require('internal/options');
-const { reconnectZeroFillToggle } = require('internal/buffer');
 const {
   exposeInterface,
   exposeLazyInterfaces,
@@ -98,7 +97,6 @@ function prepareExecution(options) {
   const { expandArgv1, initializeModules, isMainThread } = options;
 
   refreshRuntimeOptions();
-  reconnectZeroFillToggle();
 
   // Patch the process object and get the resolved main entry point.
   const mainEntry = patchProcessObject(expandArgv1);

src/node_buffer.cc

@@ -77,7 +77,6 @@ using v8::Object;
 using v8::SharedArrayBuffer;
 using v8::String;
 using v8::Uint32;
-using v8::Uint32Array;
 using v8::Uint8Array;
 using v8::Value;
 
@@ -1229,37 +1228,6 @@ void SetBufferPrototype(const FunctionCallbackInfo<Value>& args) {
   realm->set_buffer_prototype_object(proto);
 }
 
-void GetZeroFillToggle(const FunctionCallbackInfo<Value>& args) {
-  Environment* env = Environment::GetCurrent(args);
-  NodeArrayBufferAllocator* allocator = env->isolate_data()->node_allocator();
-  Local<ArrayBuffer> ab;
-  // It can be a nullptr when running inside an isolate where we
-  // do not own the ArrayBuffer allocator.
-  if (allocator == nullptr) {
-    // Create a dummy Uint32Array - the JS land can only toggle the C++ land
-    // setting when the allocator uses our toggle. With this the toggle in JS
-    // land results in no-ops.
-    ab = ArrayBuffer::New(env->isolate(), sizeof(uint32_t));
-  } else {
-    uint32_t* zero_fill_field = allocator->zero_fill_field();
-    std::unique_ptr<BackingStore> backing =
-        ArrayBuffer::NewBackingStore(zero_fill_field,
-                                     sizeof(*zero_fill_field),
-                                     [](void*, size_t, void*) {},
-                                     nullptr);
-    ab = ArrayBuffer::New(env->isolate(), std::move(backing));
-  }
-
-  if (ab->SetPrivate(env->context(),
-                     env->untransferable_object_private_symbol(),
-                     True(env->isolate()))
-          .IsNothing()) {
-    return;
-  }
-
-  args.GetReturnValue().Set(Uint32Array::New(ab, 0, 1));
-}
-
 static void Btoa(const FunctionCallbackInfo<Value>& args) {
   CHECK_EQ(args.Length(), 1);
   Environment* env = Environment::GetCurrent(args);
@@ -1433,6 +1401,52 @@ void CopyArrayBuffer(const FunctionCallbackInfo<Value>& args) {
   memcpy(dest, src, bytes_to_copy);
 }
 
+// Converts a number parameter to size_t suitable for ArrayBuffer sizes
+// Could be larger than uint32_t
+// See v8::internal::TryNumberToSize and v8::internal::NumberToSize
+inline size_t CheckNumberToSize(Local<Value> number) {
+  CHECK(number->IsNumber());
+  double value = number.As<Number>()->Value();
+  // See v8::internal::TryNumberToSize on this (and on < comparison)
+  double maxSize = static_cast<double>(std::numeric_limits<size_t>::max());
+  CHECK(value >= 0 && value < maxSize);
+  size_t size = static_cast<size_t>(value);
+#ifdef V8_ENABLE_SANDBOX
+  CHECK_LE(size, kMaxSafeBufferSizeForSandbox);
+#endif
+  return size;
+}
+
+void CreateUnsafeArrayBuffer(const FunctionCallbackInfo<Value>& args) {
+  Environment* env = Environment::GetCurrent(args);
+  if (args.Length() != 1) {
+    env->ThrowRangeError("Invalid array buffer length");
+    return;
+  }
+
+  size_t size = CheckNumberToSize(args[0]);
+
+  Isolate* isolate = env->isolate();
+
+  Local<ArrayBuffer> buf;
+
+  NodeArrayBufferAllocator* allocator = env->isolate_data()->node_allocator();
+  // 0-length, or zero-fill flag is set, or building snapshot
+  if (size == 0 || per_process::cli_options->zero_fill_all_buffers ||
+      allocator == nullptr) {
+    buf = ArrayBuffer::New(isolate, size);
+  } else {
+    std::unique_ptr<BackingStore> store =
+        ArrayBuffer::NewBackingStoreForNodeLTS(isolate, size);
+    if (!store) {
+      return env->ThrowRangeError("Array buffer allocation failed");
+    }
+    buf = ArrayBuffer::New(isolate, std::move(store));
+  }
+
+  args.GetReturnValue().Set(buf);
+}
+
 template <encoding encoding>
 uint32_t WriteOneByteString(const char* src,
                             uint32_t src_len,
@@ -1550,6 +1564,8 @@ void Initialize(Local<Object> target,
   SetMethodNoSideEffect(context, target, "indexOfString", IndexOfString);
 
   SetMethod(context, target, "copyArrayBuffer", CopyArrayBuffer);
+  SetMethodNoSideEffect(
+      context, target, "createUnsafeArrayBuffer", CreateUnsafeArrayBuffer);
 
   SetMethod(context, target, "swap16", Swap16);
   SetMethod(context, target, "swap32", Swap32);
@@ -1599,8 +1615,6 @@ void Initialize(Local<Object> target,
                 "utf8WriteStatic",
                 SlowWriteString<UTF8>,
                 &fast_write_string_utf8);
-
-  SetMethod(context, target, "getZeroFillToggle", GetZeroFillToggle);
 }
 
 }  // anonymous namespace
@@ -1649,9 +1663,9 @@ void RegisterExternalReferences(ExternalReferenceRegistry* registry) {
   registry->Register(StringWrite<HEX>);
   registry->Register(StringWrite<UCS2>);
   registry->Register(StringWrite<UTF8>);
-  registry->Register(GetZeroFillToggle);
 
   registry->Register(CopyArrayBuffer);
+  registry->Register(CreateUnsafeArrayBuffer);
 
   registry->Register(Atob);
   registry->Register(Btoa);