Fix integer overflow in napi_create_dataview bounds check

napi_create_dataview validated the requested view with an unchecked
byte_length + byte_offset > bufferLength comparison. Both are caller-supplied
size_t values, so on 64-bit builds their sum can overflow and wrap past the
limit, slipping an out-of-range view past the check.

- Chakra: the truncated 32-bit values created a small valid view while the
  original 64-bit extents were stored in DataViewInfo and handed back by
  napi_get_dataview_info, giving an out-of-bounds access primitive.
- V8: the wrapped value passed the check and then tripped a fatal CHECK inside
  v8::DataView::New, aborting the process.

Both backends now validate byte_offset and byte_length against the buffer size
individually, without adding them. JavaScriptCore delegates to the JS DataView
constructor and is unaffected. The V8JSI Node-API shim does not implement
napi_create_dataview (its DataView::New throws "TODO"), so the regression test
is compiled out there; it runs on Chakra, V8, and JavaScriptCore on 64-bit.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Author: 2 people

Core/Node-API/Source/js_native_api_chakra.cc

@@ -2244,7 +2244,13 @@ napi_status napi_create_dataview(napi_env env,
     &unused,
     &bufferLength));
 
-  if (byte_length + byte_offset > bufferLength) {
+  // bufferLength is 32-bit; byte_offset and byte_length are caller-supplied
+  // size_t values. Validate each against the buffer size without adding them
+  // (byte_offset + byte_length could overflow size_t and wrap past the check,
+  // after which the values would be truncated to 32-bit for JsCreateDataView
+  // while the original 64-bit values get stored in DataViewInfo and later
+  // handed back by napi_get_dataview_info, enabling an out-of-bounds access).
+  if (byte_offset > bufferLength || byte_length > bufferLength - byte_offset) {
     napi_throw_range_error(
       env,
       "ERR_NAPI_INVALID_DATAVIEW_ARGS",

Core/Node-API/Source/js_native_api_v8.cc

@@ -3185,7 +3185,12 @@ napi_status NAPI_CDECL napi_create_dataview(napi_env env,
   RETURN_STATUS_IF_FALSE(env, value->IsArrayBuffer(), napi_invalid_arg);
 
   v8::Local<v8::ArrayBuffer> buffer = value.As<v8::ArrayBuffer>();
-  if (byte_length + byte_offset > buffer->ByteLength()) {
+  // byte_offset and byte_length are caller-supplied size_t values; computing
+  // byte_length + byte_offset can overflow and wrap past the buffer size, which
+  // would slip an out-of-range request through and trip a fatal CHECK inside
+  // v8::DataView::New. Validate each against the buffer size without adding them.
+  if (byte_offset > buffer->ByteLength() ||
+      byte_length > buffer->ByteLength() - byte_offset) {
     napi_throw_range_error(env,
                            "ERR_NAPI_INVALID_DATAVIEW_ARGS",
                            "byte_offset + byte_length should be less than or "

Tests/UnitTests/CMakeLists.txt

@@ -46,6 +46,12 @@ endif()
 add_executable(UnitTests ${SOURCES} ${SCRIPTS} ${TYPE_SCRIPTS} ${ASSETS})
 target_compile_definitions(UnitTests PRIVATE JSRUNTIMEHOST_PLATFORM="${JSRUNTIMEHOST_PLATFORM}")
 
+# The V8JSI Node-API shim does not implement napi_create_dataview, so the
+# CreateDataViewRejectsOverflowingRange test is compiled out on that backend.
+if(NAPI_JAVASCRIPT_ENGINE STREQUAL "JSI")
+    target_compile_definitions(UnitTests PRIVATE JSRUNTIMEHOST_NAPI_ENGINE_JSI)
+endif()
+
 target_link_libraries(UnitTests
     PRIVATE AppRuntime
     PRIVATE Console

Tests/UnitTests/Shared/Shared.cpp

@@ -16,6 +16,7 @@
 #include <arcana/threading/blocking_concurrent_queue.h>
 #include <atomic>
 #include <chrono>
+#include <cstdint>
 #include <future>
 #include <iostream>
 #include <thread>
@@ -276,6 +277,78 @@ TEST(AppRuntime, DestroyDoesNotDeadlock)
     testThread.join();
 }
 
+// The V8JSI Node-API shim does not implement napi_create_dataview /
+// napi_get_dataview_info (its DataView::New throws "TODO"), so this native test
+// only builds on the Chakra, V8, and JavaScriptCore backends. The size_t-width
+// guard is required because the overflow scenario below needs a 64-bit size_t.
+#if (SIZE_MAX > 0xFFFFFFFFu) && !defined(JSRUNTIMEHOST_NAPI_ENGINE_JSI)
+TEST(NodeApi, CreateDataViewRejectsOverflowingRange)
+{
+    // Regression: napi_create_dataview must reject a (byte_offset, byte_length)
+    // pair whose sum overflows size_t. The pre-fix code performed an unchecked
+    // `byte_offset + byte_length > bufferLength` comparison; with the inputs
+    // below the 64-bit sum wraps to 8 and slips past it. It then truncated the
+    // values to 32-bit (offset -> 0, length -> 8) and created a valid 8-byte
+    // DataView, but stored the ORIGINAL 64-bit offset/length in DataViewInfo,
+    // which napi_get_dataview_info hands back alongside the small real buffer --
+    // an out-of-bounds access primitive. This path is not reachable from JS
+    // `new DataView`, so it is covered natively here. The scenario requires a
+    // 64-bit size_t (where the 32-bit truncation diverged from the stored value),
+    // hence the size_t-width guard.
+    Babylon::AppRuntime runtime{};
+
+    std::promise<bool> overflowSafe;
+    std::promise<bool> validAccepted;
+
+    runtime.Dispatch([&overflowSafe, &validAccepted](Napi::Env env) {
+        napi_env nenv{env};
+
+        Napi::ArrayBuffer arrayBuffer{Napi::ArrayBuffer::New(env, 16)};
+        napi_value arrayBufferValue{arrayBuffer};
+
+        // Low 32 bits are individually valid for the 16-byte buffer (offset 0,
+        // length 8), but the full 64-bit values are enormous and their sum wraps
+        // around size_t to 8.
+        const size_t hugeOffset{0xFFFFFFFF00000000ull};
+        const size_t hugeLength{0x0000000100000008ull};
+
+        napi_value result{nullptr};
+        napi_status status{napi_create_dataview(nenv, hugeLength, arrayBufferValue, hugeOffset, &result)};
+
+        bool safe;
+        if (status != napi_ok || result == nullptr)
+        {
+            // Fixed path: the out-of-range request is rejected outright.
+            safe = true;
+        }
+        else
+        {
+            // If creation unexpectedly succeeds, the reported extents must still
+            // lie within the 16-byte backing buffer (i.e. not the raw 64-bit
+            // inputs). The pre-fix code reported the huge stored values here.
+            size_t reportedLength{0};
+            size_t reportedOffset{0};
+            void* data{nullptr};
+            napi_get_dataview_info(nenv, result, &reportedLength, &data, nullptr, &reportedOffset);
+            safe = reportedOffset <= 16 && reportedLength <= 16 && reportedOffset + reportedLength <= 16;
+        }
+
+        // Clear any pending range error so it doesn't surface as an unhandled error.
+        napi_value pendingException{nullptr};
+        napi_get_and_clear_last_exception(nenv, &pendingException);
+        overflowSafe.set_value(safe);
+
+        // A legitimate offset/length pair must still succeed.
+        napi_value validResult{nullptr};
+        napi_status validStatus{napi_create_dataview(nenv, 8, arrayBufferValue, 4, &validResult)};
+        validAccepted.set_value(validStatus == napi_ok && validResult != nullptr);
+    });
+
+    EXPECT_TRUE(overflowSafe.get_future().get());
+    EXPECT_TRUE(validAccepted.get_future().get());
+}
+#endif
+
 int RunTests()
 {
     testing::InitGoogleTest();