diff --git a/Core/Node-API/Source/js_native_api_chakra.cc b/Core/Node-API/Source/js_native_api_chakra.cc index 2e95ce09..53ef2af9 100644 --- a/Core/Node-API/Source/js_native_api_chakra.cc +++ b/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", diff --git a/Tests/UnitTests/Shared/Shared.cpp b/Tests/UnitTests/Shared/Shared.cpp index 9ca01bc9..d2d52b5a 100644 --- a/Tests/UnitTests/Shared/Shared.cpp +++ b/Tests/UnitTests/Shared/Shared.cpp @@ -16,6 +16,7 @@ #include #include #include +#include #include #include #include @@ -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 overflowSafe; + std::promise 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();