From 59227474e9ccc98d9bc791e13c62ee38bfd1ba7b Mon Sep 17 00:00:00 2001 From: Eduardo Speroni Date: Wed, 21 Oct 2026 16:01:19 -0300 Subject: [PATCH] fix(runtime): stop handing freed or foreign buffers to C string parameters An empty JS string passed to a char* parameter was a use-after-free: WriteValue strdup'd it and wrapped the copy in a zero-length external string, but V8 disposes a zero-length resource as soon as it is handed one, so native code received a freed buffer. The resource length now counts the NUL terminator and is never zero. The IsExternalOneByte fast path passed any external one-byte string's resource data straight to native code. The runtime's own external strings are never visible to JS, so the path only fired for foreign resources, such as strings from node_api_create_external_string_latin1, whose data is Latin-1 with no NUL terminator: native code read past the buffer and saw non-ASCII characters as invalid UTF-8. Every string now takes the copying path. --- NativeScript/runtime/Interop.mm | 30 ++++++++----------- TestFixtures/NapiCoverageModule.mm | 22 +++++++++++++- .../app/tests/Marshalling/ReferenceTests.js | 7 +++++ TestRunner/app/tests/NapiCoverageTests.js | 10 +++++++ 4 files changed, 50 insertions(+), 19 deletions(-) diff --git a/NativeScript/runtime/Interop.mm b/NativeScript/runtime/Interop.mm index 5d326c298..ac140efad 100644 --- a/NativeScript/runtime/Interop.mm +++ b/NativeScript/runtime/Interop.mm @@ -362,24 +362,18 @@ static bool SnapshotArrayLike(Isolate* isolate, Local object, Localtype == BinaryTypeEncodingType::CStringEncoding) { if (arg->IsString()) { - const char* value = nullptr; - Local strArg = arg.As(); - if (strArg->IsExternalOneByte()) { - const v8::String::ExternalOneByteStringResource* resource = - strArg->GetExternalOneByteStringResource(); - value = resource->data(); - } else { - v8::String::Utf8Value utf8Value(isolate, arg); - value = strdup(*utf8Value); - // The external string only ties the strdup'd buffer's lifetime to the - // GC; it is never read as a string, and its one-byte (Latin-1) content - // matches the UTF-8 buffer only for ASCII. Length is the buffer's byte - // count, excluding the NUL. - OneByteStringResource* resource = - new OneByteStringResource(value, (size_t)utf8Value.length()); - bool success = v8::String::NewExternalOneByte(isolate, resource).ToLocal(&arg); - tns::Assert(success, isolate); - } + // Always copy: an external one-byte string's resource (e.g. one made by + // node_api_create_external_string_latin1) holds Latin-1 with no NUL + // terminator, so its data can't be handed to native code as a C string. + v8::String::Utf8Value utf8Value(isolate, arg); + const char* value = strdup(*utf8Value); + // The external string only ties the strdup'd buffer's lifetime to the + // GC; it is never read as a string. Its length counts the NUL so it is + // never zero: V8 disposes a zero-length resource immediately. + OneByteStringResource* resource = + new OneByteStringResource(value, (size_t)utf8Value.length() + 1); + bool success = v8::String::NewExternalOneByte(isolate, resource).ToLocal(&arg); + tns::Assert(success, isolate); Interop::SetValue(dest, value); } else { BaseDataWrapper* wrapper = tns::GetValue(isolate, arg); diff --git a/TestFixtures/NapiCoverageModule.mm b/TestFixtures/NapiCoverageModule.mm index 45fd11eaf..0b9b0be01 100644 --- a/TestFixtures/NapiCoverageModule.mm +++ b/TestFixtures/NapiCoverageModule.mm @@ -1,7 +1,7 @@ // Addons see NAPI_VERSION 8 unless they ask for more; asking must come before // the headers. The env still reports module API version 8 either way — this // only decides which declarations exist. -#define NAPI_VERSION 9 +#define NAPI_VERSION 10 #include #include @@ -146,6 +146,25 @@ static napi_value CreateStrings(napi_env env, napi_callback_info info) { return result; } +// The shape addons hand to node_api_create_external_string_latin1: bytes that +// are Latin-1 (0xE9 is 'é') and not NUL-terminated. +static napi_value CreateExternalLatin1(napi_env env, napi_callback_info info) { + (void)info; + + static char kBytes[] = {'c', 'a', 'f', (char)0xE9, '!'}; + + napi_value text = NULL; + bool copied = true; + NAPI_CALL(env, + node_api_create_external_string_latin1(env, kBytes, 4, NULL, NULL, &text, &copied)); + + napi_value result = NULL; + NAPI_CALL(env, napi_create_object(env, &result)); + NAPI_CALL(env, napi_set_named_property(env, result, "text", text)); + NAPI_CALL(env, napi_set_named_property(env, result, "copied", NapiBool(env, copied))); + return result; +} + static napi_value StringStatus(napi_env env, napi_callback_info info) { size_t argc = 1; napi_value args[1]; @@ -1344,6 +1363,7 @@ static napi_value InitNapiCoverageModule(napi_env env, napi_value exports) { NAPI_METHOD("stringLengths", StringLengths), NAPI_METHOD("copyString", CopyString), NAPI_METHOD("createStrings", CreateStrings), + NAPI_METHOD("createExternalLatin1", CreateExternalLatin1), NAPI_METHOD("stringStatus", StringStatus), NAPI_METHOD("numberParts", NumberParts), NAPI_METHOD("createNumbers", CreateNumbers), diff --git a/TestRunner/app/tests/Marshalling/ReferenceTests.js b/TestRunner/app/tests/Marshalling/ReferenceTests.js index c1810c3bd..f468a99ec 100644 --- a/TestRunner/app/tests/Marshalling/ReferenceTests.js +++ b/TestRunner/app/tests/Marshalling/ReferenceTests.js @@ -148,6 +148,13 @@ describe(module.id, function () { expect(TNSGetOutput()).toBe("abc"); }); + it("marshals an empty string as an empty C string", function () { + TNSClearOutput(); + var actual = functionWithCharPtr(""); + expect(TNSGetOutput()).toBe(""); + expect(actual[0]).toBe(0); + }); + it("functionWithInt8ArrayBuffer", function () { let array = Int8Array.from([ 97, 98, 99, 0 ]).buffer; let actual = functionWithCharPtr(array); diff --git a/TestRunner/app/tests/NapiCoverageTests.js b/TestRunner/app/tests/NapiCoverageTests.js index 05c777964..bbe4696c7 100644 --- a/TestRunner/app/tests/NapiCoverageTests.js +++ b/TestRunner/app/tests/NapiCoverageTests.js @@ -46,6 +46,16 @@ describe("Node-API value surface", function () { expect(created.empty).toBe(""); }); + it("hands an external latin1 string to C as UTF-8", function () { + var external = napi.createExternalLatin1(); + expect(external.copied).toBe(false); + expect(external.text).toBe("café"); + + TNSClearOutput(); + functionWithCharPtr(external.text); + expect(TNSGetOutput()).toBe("café"); + }); + it("honours an explicit length when creating", function () { expect(napi.createStrings().utf8Sized).toBe("abc"); });