Fix integer overflow in Chakra napi_create_dataview bounds check

napi_create_dataview validated the requested view with an unchecked
�yte_length + byte_offset > bufferLength comparison. byte_length and
byte_offset 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 then
truncated to 32-bit for JsCreateDataView (creating a small, valid view) while
the ORIGINAL 64-bit values are stored in DataViewInfo and later returned by
napi_get_dataview_info alongside the small backing buffer, giving a calling
addon an out-of-bounds read/write primitive.

Validate byte_offset and byte_length against the buffer size individually
without adding them, so neither overflow nor the subsequent 32-bit truncation
can slip an out-of-range view past the check. After the check both values are
<= bufferLength (a 32-bit quantity), so the truncation and the stored values
are guaranteed in range.

The JavaScriptCore backend delegates to the JS DataView constructor and is
unaffected. The V8 backend carries the same upstream pattern but is vendored
verbatim from Node and left untouched.

Add a native regression test (the path is not reachable from JS
ew DataView)
that crafts an offset/length whose low 32 bits are valid but whose 64-bit sum
wraps, and asserts the view is rejected (or at least never reports the raw
64-bit extents). Guarded to the Chakra engine and 64-bit builds.

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",

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,74 @@ TEST(AppRuntime, DestroyDoesNotDeadlock)
     testThread.join();
 }
 
+#if SIZE_MAX > 0xFFFFFFFFu
+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();