diff --git a/src/NodeApi/NodeApi.csproj b/src/NodeApi/NodeApi.csproj index b6aa76a6..3727e35e 100644 --- a/src/NodeApi/NodeApi.csproj +++ b/src/NodeApi/NodeApi.csproj @@ -28,6 +28,10 @@ + + + + diff --git a/src/NodeApi/Runtime/Utf8StringArray.cs b/src/NodeApi/Runtime/Utf8StringArray.cs index f3b7b36c..1ea25e3b 100644 --- a/src/NodeApi/Runtime/Utf8StringArray.cs +++ b/src/NodeApi/Runtime/Utf8StringArray.cs @@ -46,9 +46,13 @@ public unsafe Utf8StringArray(ReadOnlySpan strings) fixed (char* src = strings[i]) { Utf8Strings[i] = stringBufferPtr + offset; - offset += Encoding.UTF8.GetBytes( - src, strings[i].Length, (byte*)(stringBufferPtr + offset), byteLength - offset) - + 1; // +1 for the string Null-terminator. + int encodedLength = Encoding.UTF8.GetBytes( + src, strings[i].Length, (byte*)(stringBufferPtr + offset), byteLength - offset); + + // Rented pool buffers are not zero-initialized, so write the terminator explicitly + // rather than relying on the reserved byte already being zero (#481). + *(byte*)(stringBufferPtr + offset + encodedLength) = 0; + offset += encodedLength + 1; } } } diff --git a/test/Utf8StringArrayTests.cs b/test/Utf8StringArrayTests.cs new file mode 100644 index 00000000..f7c64e20 --- /dev/null +++ b/test/Utf8StringArrayTests.cs @@ -0,0 +1,71 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +// The bug only affects the .NET path, where the buffer is pool-rented (not zeroed); on .NET +// Framework it is a zeroed new byte[] and Marshal.PtrToStringUTF8 is unavailable. +#if !(NETFRAMEWORK || NETSTANDARD) + +using System.Buffers; +using System.Collections.Generic; +using System.Runtime.InteropServices; +using Microsoft.JavaScript.NodeApi.Runtime; +using Xunit; + +namespace Microsoft.JavaScript.NodeApi.Test; + +public class Utf8StringArrayTests +{ + // Regression test for #481: a dirty (non-zeroed) pooled buffer must not leak bytes into the + // marshaled strings, which requires writing the null terminator explicitly. + [Fact] + public void MarshalsStringsCorrectlyWithDirtyPooledBuffer() + { + string[] input = + { + "node", + "--disable-wasm-trap-handler", + "--max-old-space-size=4096", + }; + + PoisonSharedByteArrayPool(); + + string[] roundTripped; + using (var utf8Array = new Utf8StringArray(input)) + { + nint[] pointers = utf8Array.Utf8Strings; + roundTripped = new string[input.Length]; + for (int i = 0; i < input.Length; i++) + { + roundTripped[i] = Marshal.PtrToStringUTF8(pointers[i])!; + } + } + + Assert.Equal(input, roundTripped); + } + + // Fill pooled buffers with non-zero bytes and return them, so the next rent starts dirty. + private static void PoisonSharedByteArrayPool() + { + var rented = new List(); + for (int size = 8; size <= 1024; size *= 2) + { + for (int n = 0; n < 16; n++) + { + byte[] buffer = ArrayPool.Shared.Rent(size); + for (int k = 0; k < buffer.Length; k++) + { + buffer[k] = 0xEF; + } + + rented.Add(buffer); + } + } + + foreach (byte[] buffer in rented) + { + ArrayPool.Shared.Return(buffer); + } + } +} + +#endif