[release/10.0] Use a SafeHandle when duplicating a certificate context. #119372
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
CertDuplicateCertificateContext does not ensure the CERT_CONTEXT pointer it is incrementing has not been freed. If the duplicate and dispose race, duplicating a disposed handle will lead to unspecified behavior. For .NET 10, we can use the SafeHandle around the certificate before we duplicate the handle. For OOB, we don't have access to the internals, so we will continue to use the IntPtr Handle property.
Contributor
|
Tagging subscribers to this area: @dotnet/area-system-security, @bartonjs, @vcsjones |
bartonjs
approved these changes
Sep 4, 2025
artl93
approved these changes
Sep 4, 2025
This was referenced Sep 5, 2025
Open
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Backport of #119362 to release/10.0
/cc @vcsjones
Customer Impact
Customers occasionally use an
X509Certificate2in a way that results in concurrent use andDispose. A change was recently made that cause this to have an access violation or corruption and take down the process.Regression
Introduced by PR #117907.
Testing
We have a smoke test for this,
RaceDisposeAndKeyAccessthat attempts to use and dispose a certificate concurrently. Since the race window is fairly narrow, the test does not hit the conditions required to create a crash reliably. However this test did start failing in CI which identified the issue.The fix was verified by running the same test with significantly more attempts and not crashing in 60 minutes, whereas a crash would previously happen in less than 5.
Risk
Low. The change is straightforward and uses a
SafeHandleto ensure the native interop does not use a freed handle. There is significant test coverage in this area to test that existing scenarios continue to work as expected.