From af7c0a108b3a19a0d2defe396358eae1c6e3908f Mon Sep 17 00:00:00 2001
From: Krzysztof Wicher <kwicher@microsoft.com>
Date: Wed, 24 Jan 2024 14:01:53 +0100
Subject: [PATCH] Do not use AllocHGlobal in Pkcs12Reader

---
 .../X509Certificates/UnixPkcs12Reader.cs      | 40 ++++---------------
 1 file changed, 8 insertions(+), 32 deletions(-)

diff --git a/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/X509Certificates/UnixPkcs12Reader.cs b/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/X509Certificates/UnixPkcs12Reader.cs
index a9a89f6769690..8b4e999750f8c 100644
--- a/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/X509Certificates/UnixPkcs12Reader.cs
+++ b/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/X509Certificates/UnixPkcs12Reader.cs
@@ -24,7 +24,7 @@ internal abstract class UnixPkcs12Reader : IDisposable
         private ContentInfoAsn[]? _safeContentsValues;
         private CertAndKey[]? _certs;
         private int _certCount;
-        private PointerMemoryManager<byte>? _tmpManager;
+        private Memory<byte> _tmpMemory;
         private bool _allowDoubleBind;
 
         protected abstract ICertificatePalCore ReadX509Der(ReadOnlyMemory<byte> data);
@@ -39,19 +39,13 @@ protected void ParsePkcs12(ReadOnlySpan<byte> data)
 
                 // Windows compatibility: Ignore trailing data.
                 ReadOnlySpan<byte> encodedData = reader.PeekEncodedValue();
+                byte[] dataWithoutTrailing = GC.AllocateUninitializedArray<byte>(encodedData.Length, pinned: true);
+                encodedData.CopyTo(dataWithoutTrailing);
+                _tmpMemory = MemoryMarshal.CreateFromPinnedArray(dataWithoutTrailing, 0, dataWithoutTrailing.Length);
 
-                unsafe
-                {
-                    IntPtr tmpPtr = Marshal.AllocHGlobal(encodedData.Length);
-                    Span<byte> tmpSpan = new Span<byte>((byte*)tmpPtr, encodedData.Length);
-                    encodedData.CopyTo(tmpSpan);
-                    _tmpManager = new PointerMemoryManager<byte>((void*)tmpPtr, encodedData.Length);
-                }
-
-                ReadOnlyMemory<byte> tmpMemory = _tmpManager.Memory;
-                reader = new AsnValueReader(tmpMemory.Span, AsnEncodingRules.BER);
+                reader = new AsnValueReader(_tmpMemory.Span, AsnEncodingRules.BER);
 
-                PfxAsn.Decode(ref reader, tmpMemory, out PfxAsn pfxAsn);
+                PfxAsn.Decode(ref reader, _tmpMemory, out PfxAsn pfxAsn);
 
                 if (pfxAsn.AuthSafe.ContentType != Oids.Pkcs7Data)
                 {
@@ -112,26 +106,8 @@ internal IEnumerable<CertAndKey> EnumerateAll()
 
         public void Dispose()
         {
-            // Generally, having a MemoryManager cleaned up in a Dispose is a bad practice.
-            // In this case, the UnixPkcs12Reader is only ever created in a using statement,
-            // never accessed by a second thread, and there isn't a manual call to Dispose
-            // mixed in anywhere.
-            if (_tmpManager != null)
-            {
-                unsafe
-                {
-                    Span<byte> tmp = _tmpManager.GetSpan();
-                    CryptographicOperations.ZeroMemory(tmp);
-
-                    fixed (byte* ptr = tmp)
-                    {
-                        Marshal.FreeHGlobal((IntPtr)ptr);
-                    }
-                }
-
-                ((IDisposable)_tmpManager).Dispose();
-                _tmpManager = null;
-            }
+            CryptographicOperations.ZeroMemory(_tmpMemory.Span);
+            _tmpMemory = Memory<byte>.Empty;
 
             ContentInfoAsn[]? rentedContents = Interlocked.Exchange(ref _safeContentsValues, null);
             CertAndKey[]? rentedCerts = Interlocked.Exchange(ref _certs, null);