Fix integer overflow in Chakra napi_create_dataview bounds check (#181)

## Summary
`napi_create_dataview` (Chakra backend) validated the requested view
with an unchecked `byte_length + byte_offset > bufferLength` comparison.
Both operands are caller-supplied `size_t` values, so on 64-bit builds
their sum can **overflow and wrap** past the limit. When that happens:
- the values are truncated to 32-bit for `JsCreateDataView` (yielding a
small, valid view), but
- the **original 64-bit** `byte_offset`/`byte_length` are stored in
`DataViewInfo` and later returned by `napi_get_dataview_info` alongside
the small backing buffer

…handing a calling native addon an out-of-bounds read/write primitive
(CWE-190 → CWE-125/787, also CWE-681).

## Fix
Validate `byte_offset` and `byte_length` against the buffer size
**individually, without adding them**:
`cpp
if (byte_offset > bufferLength || byte_length > bufferLength -
byte_offset) { /* range error */ }
`
After this check both values are `<= bufferLength` (a 32-bit quantity),
so the later 32-bit truncation and the values stored in `DataViewInfo`
are guaranteed in range.

- **JavaScriptCore** backend delegates to the JS `DataView` constructor
→ unaffected.
- **V8** backend carries the same upstream pattern but is vendored
verbatim from Node → left untouched.

## Test
This path is **not reachable from JS** (`new DataView` uses the engine
directly, not this N-API), so coverage is a native gtest. It crafts an
offset/length whose low 32 bits are valid for a 16-byte buffer but whose
64-bit sum wraps, and asserts the view is rejected (and never reports
the raw 64-bit extents). Guarded to the Chakra engine and 64-bit builds
(`JSRUNTIMEHOST_NAPI_ENGINE_CHAKRA` + `_WIN64`).

Verified the test **fails on the pre-fix code** and **passes after the
fix**; full suite green (5 native tests + 151 JS tests) on Win32-x64
Chakra Debug.

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

Author: 3 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,13 @@ 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()) {
+  // [BABYLON-NATIVE-ADDITION]: overflow-safe bounds check; not in Node.js upstream
+  // 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

@@ -17,6 +17,7 @@
 #include <arcana/threading/blocking_concurrent_queue.h>
 #include <atomic>
 #include <chrono>
+#include <cstdint>
 #include <future>
 #include <iostream>
 #include <thread>
@@ -278,6 +279,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();