Harden UnixPkcs12Reader AllocHGlobal #98331
Conversation
|
Tagging subscribers to this area: @dotnet/area-system-security, @bartonjs, @vcsjones Issue Details
Replaces PRs #93647 and #97447
|
* Do not do the allocation and parsing in the constructor, since that prevents Dispose from running * If something goes wrong between AllocHGlobal and moving the pointer into a PointerMemoryManager, call FreeHGlobal. * Past that point it will be correctly freed by Dispose.
....Security.Cryptography/src/System/Security/Cryptography/X509Certificates/UnixPkcs12Reader.cs
Outdated
Show resolved
Hide resolved
...em.Security.Cryptography/src/System/Security/Cryptography/X509Certificates/StorePal.macOS.cs
Show resolved
Hide resolved
....Security.Cryptography/src/System/Security/Cryptography/X509Certificates/UnixPkcs12Reader.cs
Outdated
Show resolved
Hide resolved
....Security.Cryptography/src/System/Security/Cryptography/X509Certificates/UnixPkcs12Reader.cs
Outdated
Show resolved
Hide resolved
| Span<byte> tmp = _tmpManager.GetSpan(); | ||
| CryptographicOperations.ZeroMemory(tmp); | ||
|
|
||
| fixed (byte* ptr = tmp) |
There was a problem hiding this comment.
Should we just expose the pointer directly? It doesn't really matter; it's just a bit of a headscratcher seeing this code pin a span and then free the pointer out from under it.
There was a problem hiding this comment.
I believe this is the only place that combines the Pointer Memory Manager and an alloc/free; so it seems "better" to me to just have a weird line of code here than to de-encapsulate the pointer.
There was a problem hiding this comment.
And/or use Unsafe.AsPointer to get the pointer instead of pinning:
Unsafe.AsPointer(ref MemoryMarshal.GetReference(tmp))
....Security.Cryptography/src/System/Security/Cryptography/X509Certificates/UnixPkcs12Reader.cs
Outdated
Show resolved
Hide resolved
3 participants
Replaces PRs #93647 and #97447