Skip to content

Commit

Permalink
Do not use AllocHGlobal in Pkcs12Reader
Browse files Browse the repository at this point in the history
  • Loading branch information
krwq committed Jan 24, 2024
1 parent 3d59a54 commit af7c0a1
Showing 1 changed file with 8 additions and 32 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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)
{
Expand Down Expand Up @@ -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);
Expand Down

0 comments on commit af7c0a1

Please sign in to comment.