Use a custom ReadOnlySpan marshaller to prevent null pointers when marshaling

src/libraries/Common/src/Interop/Windows/BCrypt/Interop.BCryptGenerateSymmetricKey.cs

@@ -3,29 +3,30 @@
 
 using System;
 using System.Runtime.InteropServices;
+using System.Runtime.InteropServices.Marshalling;
 using Microsoft.Win32.SafeHandles;
 
 internal static partial class Interop
 {
     internal static partial class BCrypt
     {
         [LibraryImport(Libraries.BCrypt)]
-        internal static unsafe partial NTSTATUS BCryptGenerateSymmetricKey(
+        internal static partial NTSTATUS BCryptGenerateSymmetricKey(
             SafeBCryptAlgorithmHandle hAlgorithm,
             out SafeBCryptKeyHandle phKey,
             IntPtr pbKeyObject,
             int cbKeyObject,
-            byte* pbSecret,
+            [MarshalUsing(typeof(NotNullReadOnlySpanMarshaller<,>))] ReadOnlySpan<byte> pbSecret,
             int cbSecret,
             uint dwFlags);
 
         [LibraryImport(Libraries.BCrypt)]
-        internal static unsafe partial NTSTATUS BCryptGenerateSymmetricKey(
+        internal static partial NTSTATUS BCryptGenerateSymmetricKey(
             nuint hAlgorithm,
             out SafeBCryptKeyHandle phKey,
             IntPtr pbKeyObject,
             int cbKeyObject,
-            byte* pbSecret,
+            [MarshalUsing(typeof(NotNullReadOnlySpanMarshaller<,>))] ReadOnlySpan<byte> pbSecret,
             int cbSecret,
             uint dwFlags);
     }

src/libraries/Common/src/System/Runtime/InteropServices/NotNullReadOnlySpanMarshaller.cs

@@ -0,0 +1,107 @@
+// Licensed to the .NET Foundation under one or more agreements.
+// The .NET Foundation licenses this file to you under the MIT license.
+
+using System;
+using System.Collections.Generic;
+using System.Runtime.CompilerServices;
+using System.Text;
+
+namespace System.Runtime.InteropServices.Marshalling
+{
+    [CustomMarshaller(typeof(ReadOnlySpan<>), MarshalMode.ManagedToUnmanagedIn, typeof(NotNullReadOnlySpanMarshaller<,>.ManagedToUnmanagedIn))]
+    [CustomMarshaller(typeof(ReadOnlySpan<>), MarshalMode.ManagedToUnmanagedOut, typeof(NotNullReadOnlySpanMarshaller<,>.ManagedToUnmanagedOut))]
+    [ContiguousCollectionMarshaller]
+    internal static unsafe class NotNullReadOnlySpanMarshaller<T, TUnmanagedElement>
+        where TUnmanagedElement : unmanaged
+    {
+        public ref struct ManagedToUnmanagedIn
+        {
+            /// <summary>
+            /// Gets the size of the caller-allocated buffer to allocate.
+            /// </summary>
+            // We'll keep the buffer size at a maximum of 512 bytes to avoid overflowing the stack.
+            public static int BufferSize => 0x200 / sizeof(TUnmanagedElement);
+
+            private ReadOnlySpan<T> _managedArray;
+            private TUnmanagedElement* _allocatedMemory;
+            private Span<TUnmanagedElement> _span;
+
+            public void FromManaged(ReadOnlySpan<T> managed, Span<TUnmanagedElement> buffer)
+            {
+                _managedArray = managed;
+
+                if (managed.Length <= buffer.Length)
+                {
+                    _span = buffer[0..managed.Length];
+                }
+                else
+                {
+                    int bufferSize = checked(managed.Length * sizeof(TUnmanagedElement));
+                    _allocatedMemory = (TUnmanagedElement*)NativeMemory.Alloc((nuint)bufferSize);
+                    _span = new Span<TUnmanagedElement>(_allocatedMemory, managed.Length);
+                }
+            }
+
+            public ReadOnlySpan<T> GetManagedValuesSource() => _managedArray;
+
+            public Span<TUnmanagedElement> GetUnmanagedValuesDestination() => _span;
+
+            public ref TUnmanagedElement GetPinnableReference() => ref MemoryMarshal.GetReference(_span);
+
+            public TUnmanagedElement* ToUnmanaged()
+            {
+                // Unsafe.AsPointer is safe since buffer must be pinned
+                return (TUnmanagedElement*)Unsafe.AsPointer(ref GetPinnableReference());
+            }
+
+            public void Free()
+            {
+                NativeMemory.Free(_allocatedMemory);
+            }
+
+            public static ref T GetPinnableReference(ReadOnlySpan<T> managed)
+            {
+                if (Unsafe.IsNullRef(ref MemoryMarshal.GetReference(managed)) && managed.IsEmpty)
+                {
+                    return ref MemoryMarshal.GetArrayDataReference(Array.Empty<T>());
+                }
+                else
+                {
+                    return ref MemoryMarshal.GetReference(managed);
+                }
+            }
+        }
+
+        public struct ManagedToUnmanagedOut
+        {
+            private TUnmanagedElement* _unmanagedArray;
+            private T[]? _managedValues;
+
+            public void FromUnmanaged(TUnmanagedElement* unmanaged)
+            {
+                _unmanagedArray = unmanaged;
+            }
+
+            public ReadOnlySpan<T> ToManaged()
+            {
+                return new ReadOnlySpan<T>(_managedValues!);
+            }
+
+            public ReadOnlySpan<TUnmanagedElement> GetUnmanagedValuesSource(int numElements)
+            {
+                return new ReadOnlySpan<TUnmanagedElement>(_unmanagedArray, numElements);
+            }
+
+            public Span<T> GetManagedValuesDestination(int numElements)
+            {
+                _managedValues = new T[numElements];
+                return _managedValues;
+            }
+
+            public void Free()
+            {
+                Marshal.FreeCoTaskMem((IntPtr)_unmanagedArray);
+            }
+        }
+    }
+}

src/libraries/Common/src/System/Security/Cryptography/SP800108HmacCounterKdfImplementationCng.cs

@@ -128,7 +128,7 @@ internal static void DeriveBytesOneShot(
             }
         }
 
-        private static unsafe SafeBCryptKeyHandle CreateSymmetricKey(byte* symmetricKey, int symmetricKeyLength)
+        private static SafeBCryptKeyHandle CreateSymmetricKey(ReadOnlySpan<byte> symmetricKey)
         {
             NTSTATUS generateKeyStatus;
             SafeBCryptKeyHandle keyHandle;
@@ -141,7 +141,7 @@ private static unsafe SafeBCryptKeyHandle CreateSymmetricKey(byte* symmetricKey,
                     pbKeyObject: IntPtr.Zero,
                     cbKeyObject: 0,
                     symmetricKey,
-                    symmetricKeyLength,
+                    symmetricKey.Length,
                     dwFlags: 0);
             }
             else
@@ -152,7 +152,7 @@ private static unsafe SafeBCryptKeyHandle CreateSymmetricKey(byte* symmetricKey,
                     pbKeyObject: IntPtr.Zero,
                     cbKeyObject: 0,
                     symmetricKey,
-                    symmetricKeyLength,
+                    symmetricKey.Length,
                     dwFlags: 0);
             }
 

src/libraries/Microsoft.Bcl.Cryptography/src/Microsoft.Bcl.Cryptography.csproj

@@ -27,6 +27,8 @@
   </ItemGroup>
 
   <ItemGroup Condition="'$(TargetFrameworkIdentifier)' != '.NETCoreApp'">
+    <Compile Include="$(CommonPath)System\Runtime\InteropServices\NotNullReadOnlySpanMarshaller.cs"
+             Link="Common\System\Runtime\InteropServices\NotNullReadOnlySpanMarshaller.cs" />
     <Compile Include="$(CommonPath)Microsoft\Win32\SafeHandles\SafeBCryptAlgorithmHandle.cs"
              Link="Microsoft\Win32\SafeHandles\SafeBCryptAlgorithmHandle.cs" />
     <Compile Include="$(CommonPath)Microsoft\Win32\SafeHandles\SafeBCryptHandle.cs"

src/libraries/Microsoft.Bcl.Cryptography/src/System/Security/Cryptography/SP800108HmacCounterKdfImplementationCng.cs

@@ -13,7 +13,6 @@ internal unsafe SP800108HmacCounterKdfImplementationCng(ReadOnlySpan<byte> key,
 
             scoped ReadOnlySpan<byte> symmetricKeyMaterial;
             scoped Span<byte> clearSpan = default;
-            int symmetricKeyMaterialLength;
             int hashAlgorithmBlockSize = GetHashBlockSize(hashAlgorithm.Name);
 
             if (key.Length > hashAlgorithmBlockSize)
@@ -28,26 +27,15 @@ internal unsafe SP800108HmacCounterKdfImplementationCng(ReadOnlySpan<byte> key,
                 }
 
                 symmetricKeyMaterial = clearSpan;
-                symmetricKeyMaterialLength = symmetricKeyMaterial.Length;
-            }
-            else if (!key.IsEmpty)
-            {
-                symmetricKeyMaterial = key;
-                symmetricKeyMaterialLength = key.Length;
             }
             else
             {
-                // CNG requires a non-null pointer even when the length is zero.
-                symmetricKeyMaterial = [0];
-                symmetricKeyMaterialLength = 0;
+                symmetricKeyMaterial = key;
             }
 
             try
             {
-                fixed (byte* pSymmetricKeyMaterial = symmetricKeyMaterial)
-                {
-                    _keyHandle = CreateSymmetricKey(pSymmetricKeyMaterial, symmetricKeyMaterialLength);
-                }
+                _keyHandle = CreateSymmetricKey(symmetricKeyMaterial);
             }
             finally
             {
@@ -65,33 +53,21 @@ internal unsafe SP800108HmacCounterKdfImplementationCng(byte[] key, HashAlgorith
 
             scoped ReadOnlySpan<byte> symmetricKeyMaterial;
             scoped Span<byte> clearSpan = default;
-            int symmetricKeyMaterialLength;
             int hashAlgorithmBlockSize = GetHashBlockSize(hashAlgorithm.Name);
 
             if (key.Length > hashAlgorithmBlockSize)
             {
                 clearSpan = HashOneShot(hashAlgorithm, key);
                 symmetricKeyMaterial = clearSpan;
-                symmetricKeyMaterialLength = symmetricKeyMaterial.Length;
-            }
-            else if (key.Length > 0)
-            {
-                symmetricKeyMaterial = key;
-                symmetricKeyMaterialLength = key.Length;
             }
             else
             {
-                // CNG requires a non-null pointer even when the length is zero.
-                symmetricKeyMaterial = [0];
-                symmetricKeyMaterialLength = 0;
+                symmetricKeyMaterial = key;
             }
 
             try
             {
-                fixed (byte* pSymmetricKeyMaterial = symmetricKeyMaterial)
-                {
-                    _keyHandle = CreateSymmetricKey(pSymmetricKeyMaterial, symmetricKeyMaterialLength);
-                }
+                _keyHandle = CreateSymmetricKey(symmetricKeyMaterial);
             }
             finally
             {

src/libraries/System.Security.Cryptography/src/System.Security.Cryptography.csproj

@@ -1514,6 +1514,8 @@
   </ItemGroup>
 
   <ItemGroup Condition="'$(TargetPlatformIdentifier)' == 'windows'">
+    <Compile Include="$(CommonPath)System\Runtime\InteropServices\NotNullReadOnlySpanMarshaller.cs"
+             Link="Common\System\Runtime\InteropServices\NotNullReadOnlySpanMarshaller.cs" />
     <Compile Include="$(CommonPath)Interop\Windows\Advapi32\Interop.CryptAcquireContext.cs"
              Link="Common\Interop\Windows\Advapi32\Interop.CryptAcquireContext.cs" />
     <Compile Include="$(CommonPath)Interop\Windows\Advapi32\Interop.CryptAcquireContext_IntPtr.cs"

src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/Pbkdf2Implementation.Windows.cs

@@ -55,20 +55,11 @@ private static unsafe void FillKeyDerivation(
             // stackalloc 0 to let compiler know this cannot escape.
             scoped Span<byte> clearSpan;
             scoped ReadOnlySpan<byte> symmetricKeyMaterial;
-            int symmetricKeyMaterialLength;
 
-            if (password.IsEmpty)
-            {
-                // CNG won't accept a null pointer for the password.
-                symmetricKeyMaterial = [0];
-                symmetricKeyMaterialLength = 0;
-                clearSpan = default;
-            }
-            else if (password.Length <= hashBlockSizeBytes)
+            if (password.Length <= hashBlockSizeBytes)
             {
                 // Password is small enough to use as-is.
                 symmetricKeyMaterial = password;
-                symmetricKeyMaterialLength = password.Length;
                 clearSpan = default;
             }
             else
@@ -108,26 +99,20 @@ private static unsafe void FillKeyDerivation(
 
                 clearSpan = hashBuffer.Slice(0, hashBufferSize);
                 symmetricKeyMaterial = clearSpan;
-                symmetricKeyMaterialLength = hashBufferSize;
             }
 
-            Debug.Assert(symmetricKeyMaterial.Length > 0);
-
             NTSTATUS generateKeyStatus;
 
             if (Interop.BCrypt.PseudoHandlesSupported)
             {
-                fixed (byte* pSymmetricKeyMaterial = symmetricKeyMaterial)
-                {
-                    generateKeyStatus = Interop.BCrypt.BCryptGenerateSymmetricKey(
-                        (nuint)BCryptAlgPseudoHandle.BCRYPT_PBKDF2_ALG_HANDLE,
-                        out keyHandle,
-                        pbKeyObject: IntPtr.Zero,
-                        cbKeyObject: 0,
-                        pSymmetricKeyMaterial,
-                        symmetricKeyMaterialLength,
-                        dwFlags: 0);
-                }
+                generateKeyStatus = Interop.BCrypt.BCryptGenerateSymmetricKey(
+                    (nuint)BCryptAlgPseudoHandle.BCRYPT_PBKDF2_ALG_HANDLE,
+                    out keyHandle,
+                    pbKeyObject: IntPtr.Zero,
+                    cbKeyObject: 0,
+                    symmetricKeyMaterial,
+                    symmetricKeyMaterial.Length,
+                    dwFlags: 0);
             }
             else
             {
@@ -151,17 +136,14 @@ private static unsafe void FillKeyDerivation(
                     Interlocked.CompareExchange(ref s_pbkdf2AlgorithmHandle, pbkdf2AlgorithmHandle, null);
                 }
 
-                fixed (byte* pSymmetricKeyMaterial = symmetricKeyMaterial)
-                {
-                    generateKeyStatus = Interop.BCrypt.BCryptGenerateSymmetricKey(
-                        s_pbkdf2AlgorithmHandle,
-                        out keyHandle,
-                        pbKeyObject: IntPtr.Zero,
-                        cbKeyObject: 0,
-                        pSymmetricKeyMaterial,
-                        symmetricKeyMaterialLength,
-                        dwFlags: 0);
-                }
+                generateKeyStatus = Interop.BCrypt.BCryptGenerateSymmetricKey(
+                    s_pbkdf2AlgorithmHandle,
+                    out keyHandle,
+                    pbKeyObject: IntPtr.Zero,
+                    cbKeyObject: 0,
+                    symmetricKeyMaterial,
+                    symmetricKeyMaterial.Length,
+                    dwFlags: 0);
             }
 
             CryptographicOperations.ZeroMemory(clearSpan);

src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/SP800108HmacCounterKdfImplementationCng.cs

@@ -7,40 +7,29 @@ namespace System.Security.Cryptography
 {
     internal sealed partial class SP800108HmacCounterKdfImplementationCng
     {
-        internal unsafe SP800108HmacCounterKdfImplementationCng(ReadOnlySpan<byte> key, HashAlgorithmName hashAlgorithm)
+        internal SP800108HmacCounterKdfImplementationCng(ReadOnlySpan<byte> key, HashAlgorithmName hashAlgorithm)
         {
             Debug.Assert(hashAlgorithm.Name is not null);
 
             scoped ReadOnlySpan<byte> symmetricKeyMaterial;
             scoped Span<byte> clearSpan = default;
-            int symmetricKeyMaterialLength;
             int hashAlgorithmBlockSize = GetHashBlockSize(hashAlgorithm.Name);
 
             if (key.Length > hashAlgorithmBlockSize)
             {
                 Span<byte> buffer = stackalloc byte[512 / 8]; // Largest supported digest is SHA512.
-                symmetricKeyMaterialLength = HashOneShot(hashAlgorithm, key, buffer);
+                int symmetricKeyMaterialLength = HashOneShot(hashAlgorithm, key, buffer);
                 clearSpan = buffer.Slice(0, symmetricKeyMaterialLength);
                 symmetricKeyMaterial = clearSpan;
             }
-            else if (!key.IsEmpty)
-            {
-                symmetricKeyMaterial = key;
-                symmetricKeyMaterialLength = key.Length;
-            }
             else
             {
-                // CNG requires a non-null pointer even when the length is zero.
-                symmetricKeyMaterial = [0];
-                symmetricKeyMaterialLength = 0;
+                symmetricKeyMaterial = key;
             }
 
             try
             {
-                fixed (byte* pSymmetricKeyMaterial = symmetricKeyMaterial)
-                {
-                    _keyHandle = CreateSymmetricKey(pSymmetricKeyMaterial, symmetricKeyMaterialLength);
-                }
+                _keyHandle = CreateSymmetricKey(symmetricKeyMaterial);
             }
             finally
             {