Repository navigation
Description
PooledBuffer.FromStringUtf8 and PooledBuffer.FromSpanUtf8 (src/NodeApi/Runtime/PooledBuffer.cs) reserve byteLength + 1 bytes but never write the terminating \0:
PooledBuffer buffer = new(byteLength, byteLength + 1); // reserves the terminator byte...
Encoding.UTF8.GetBytes(value, 0, value!.Length, buffer.Buffer, 0); // ...but never writes itOn .NET the array comes from ArrayPool<byte>.Shared.Rent, which is not zeroed. (On .NET Framework / netstandard it is a fresh new byte[], so the bug only shows on .NET.) Several call sites pass the pointer to Node-API functions that take a NUL-terminated const char* without a length:
| Managed API | Native function | Argument |
|---|---|---|
JSError.ThrowError / ThrowTypeError / ThrowRangeError / ThrowSyntaxError (string overloads) |
napi_throw_error, napi_throw_type_error, napi_throw_range_error, node_api_throw_syntax_error |
code and msg |
NodeEmbeddingRuntimeSettings.Modules |
node_embedding_runtime_config_add_module |
module_name |
exceptions in the embedding callback adapters (TaskRunCallbackAdapter, …) |
node_embedding_last_error_message_set |
message |
Once the pool is warm, native code reads stale bytes after the string, or past the end of the array if it contains no zero byte (out-of-bounds read).
Call sites that pass an explicit length (CreateFunction, DefineClass, GetSymbolFor, FatalError) are not affected.
What we saw
- In production (0.9.21, net10.0): a module registered via
NodeEmbeddingRuntimeSettings.Modulesasgaiawas linked under a corrupted name, andprocess._linkedBinding('gaia')failed withNo such binding was linked: gaia. It happened intermittently, only after the app had been running for a while. JSError.ThrowError("x")from a .NET callback → in JS,e.messageis"x"followed by 15 garbage bytes (x���������������).
Repro
Make it deterministic by dirtying the shared pool on the JS thread first:
static void FillArrayPoolWithGarbage()
{
List<byte[]> rented = new();
for (int size = 1; size <= 1024; size *= 2)
for (int i = 0; i < 8; i++)
{
byte[] array = ArrayPool<byte>.Shared.Rent(size);
array.AsSpan().Fill(0xAA);
rented.Add(array);
}
foreach (byte[] array in rented) ArrayPool<byte>.Shared.Return(array);
}
nodejs.Run(() =>
{
JSValue.Global.SetProperty("throwError", JSValue.CreateFunction("throwError", (args) =>
{
FillArrayPoolWithGarbage();
JSError.ThrowError((string)args[0]);
return JSValue.Undefined;
}));
string message = (string)JSValue.RunScript(
"(() => { try { throwError('x'); } catch (e) { return e.message; } })()");
// Expected "x"; actual "x" + garbage.
});Related
- An empty string returns
PooledBuffer.Empty, which pins to a NULL pointer. SoJSError.ThrowError("")passesmsg = NULL,napi_throw_errorreturnsnapi_invalid_arg, and no error is thrown at all. An empty module name would likewise passmodule_name = NULL. NodejsRuntime.EmbeddingRuntimeConfigAddModulenever disposes itsPooledBuffer, so the rented array is not returned to the pool.- Same class of bug as Embedding args occasionally get trailing garbage on .NET (missing null terminator on a pooled buffer) #481 (
Utf8StringArray, fix in PR Write explicit null terminator in Utf8StringArray #482).
Suggested fix
Write the terminator explicitly in both helpers (buffer.Buffer[byteLength] = 0). Encode "" as a 1-byte buffer holding only the terminator, and keep null → null pointer (used for the optional code). Add using in EmbeddingRuntimeConfigAddModule; Node.js copies the name into a std::string, so disposing right after the call is safe. I have a PR ready.
Versions: reproduced on main (54d2b74) and in 0.9.21; the code is unchanged in 0.9.28. Windows x64, net10.0, Microsoft.JavaScript.LibNode 20.1800.203.
Activity