From b7119404f70015d61b9808cd5aae20924aa891be Mon Sep 17 00:00:00 2001 From: Stephen Toub Date: Mon, 11 Jul 2022 23:59:55 -0400 Subject: [PATCH 1/4] More reliably clean up more SafeHandle instances This calls Dispose on SafeHandles (or things wrapping SafeHandles) that were otherwise being left for finalization: 1. Some of these are fixing finalization happening even on success paths (typically where the implementation isn't disposing of something that directly or indirectly wraps a SafeHandle). 2. Some of these are fixing finalization happening for invalid SafeHandles. 3. Some of these are fixing tests to finalize less. My goal with fixing the tests was to eliminate the noise in order to find instances of the other two cases. These were found primarily via two means: - Debug-instrumentation in SafeHandle to log when one is finalized. This instrumentation is built into debug/checked builds of SafeHandle and requires setting the DEBUG_SAFEHANDLE_FINALIZATION environment variable to "1". - Auditing use of SafeHandle.IsInvalid There's a lot more that can be cleaned up using the SafeHandle instrumentation, but I'm pausing here for now. The System.IO.Pipes, System.IO.FileSystem, and System.Security.Cryptography tests are clean on Windows. --- src/coreclr/vm/corelib.h | 1 + src/coreclr/vm/object.h | 3 + .../Interop.Err.cs | 4 +- .../Interop.SecKeyRef.macOS.cs | 5 +- .../src/Interop/Unix/Interop.IOErrors.cs | 4 +- .../Interop.ERR.cs | 4 +- .../Interop.EcDsa.ImportExport.cs | 3 +- .../Interop.X509.cs | 4 +- .../Advapi32/Interop.OpenThreadToken.cs | 1 + .../Windows/BCrypt/BCryptAlgorithmCache.cs | 75 +++----- .../Interop.CryptEncodeDecodeWrappers.cs | 12 +- ...p.DuplicateHandle_SafeAccessTokenHandle.cs | 2 +- .../Interop.NCryptDeriveSecretAgreement.cs | 1 + .../System/Net/ContextAwareResult.Windows.cs | 1 - .../Security/Cryptography/DSAOpenSsl.cs | 8 +- .../Cryptography/ECAndroid.ImportExport.cs | 4 + .../System/Security/Cryptography/ECAndroid.cs | 2 + .../Cryptography/ECCng.ImportExport.cs | 1 + .../Cryptography/ECDiffieHellmanCng.cs | 12 +- .../ECDiffieHellmanOpenSslPublicKey.cs | 3 +- .../System/Security/Cryptography/ECDsaCng.cs | 12 +- .../Cryptography/ECOpenSsl.ImportExport.cs | 10 +- .../System/Security/Cryptography/ECOpenSsl.cs | 1 + .../Security/Cryptography/RSAAndroid.cs | 3 + .../CompressionStreamUnitTestBase.cs | 19 +- .../tests/System/IO/ReparsePointUtilities.cs | 4 +- .../System/Net/Http/DefaultCredentialsTest.cs | 6 +- .../Net/Sockets/SocketTestServerAsync.cs | 6 +- .../AES/AesCipherTests.cs | 27 ++- .../AES/AesContractTests.cs | 6 +- .../AES/AesCornerTests.cs | 10 +- .../DES/DESCipherTests.cs | 6 +- .../ECDiffieHellmanTests.ImportExport.cs | 7 +- .../ECDsa/ECDsaImportExport.cs | 7 +- .../ECDsa/ECDsaTests.cs | 18 +- .../TripleDES/TripleDESCipherTests.cs | 6 +- .../System/Buffers/BoundedMemory.Windows.cs | 4 +- .../Microsoft/Win32/RegistryKey.Windows.cs | 10 +- .../System.Console/src/System/TermInfo.cs | 1 + .../System/Diagnostics/EventLogInternal.cs | 2 + .../Diagnostics/Reader/NativeWrapper.cs | 37 ++++ .../Diagnostics/PerformanceDataRegistryKey.cs | 16 +- .../src/System/Diagnostics/Process.Win32.cs | 17 +- .../src/System/Diagnostics/Process.Windows.cs | 11 ++ .../Diagnostics/ProcessManager.Windows.cs | 3 + .../AccountManagement/AD/SidList.cs | 5 + .../AccountManagement/AuthZSet.cs | 11 +- .../AccountManagement/Utils.cs | 1 + .../ActiveDirectory/DirectoryContext.cs | 2 +- .../ActiveDirectory/Utils.cs | 1 + .../IO/Compression/DeflateZLib/Deflater.cs | 6 +- .../IO/Compression/DeflateZLib/Inflater.cs | 6 +- .../src/System/IO/FileSystemWatcher.Linux.cs | 1 + .../src/System/IO/FileSystemWatcher.OSX.cs | 4 +- .../tests/Directory/Delete.Windows.cs | 4 +- .../System.IO.FileSystem/tests/FSAssert.cs | 3 +- .../tests/File/OpenHandle.cs | 13 +- .../MemoryMappedFile.Windows.cs | 1 + .../IO/Pipes/AnonymousPipeClientStream.cs | 1 + .../Pipes/AnonymousPipeServerStream.Unix.cs | 19 +- .../AnonymousPipeServerStream.Windows.cs | 17 +- .../IO/Pipes/NamedPipeClientStream.Windows.cs | 4 + .../IO/Pipes/NamedPipeServerStream.Windows.cs | 1 + .../AnonymousPipeTest.CreateClient.cs | 4 +- .../AnonymousPipeTest.CreateServer.cs | 3 +- .../NamedPipeTest.CreateClient.cs | 2 +- .../NamedPipeTest.CreateServer.cs | 2 +- .../IO/Ports/SafeSerialDeviceHandle.Unix.cs | 2 + .../System/IO/Ports/SerialStream.Windows.cs | 4 +- .../src/System/Net/Http/WinHttpHandler.cs | 4 + .../SocketsHttpHandler/HttpWindowsProxy.cs | 1 + .../FunctionalTests/ImpersonatedAuthTests.cs | 12 +- .../Net/Windows/HttpListener.Windows.cs | 1 + .../Windows/WebSockets/SafeWebSocketHandle.cs | 6 +- .../Net/Windows/WebSockets/ServerWebSocket.cs | 15 +- .../WebSockets/WebSocketProtocolComponent.cs | 69 +++---- .../Net/NetworkInformation/Ping.Windows.cs | 2 + .../Net/CertificateValidationPal.Windows.cs | 14 +- .../Net/Security/SslStreamPal.Windows.cs | 6 +- .../IdentityValidator.Windows.cs | 3 +- .../src/System/Net/Sockets/Socket.cs | 1 + .../src/System/Net/Sockets/SocketPal.Unix.cs | 4 + .../FunctionalTests/StartupTests.Windows.cs | 4 +- .../tests/FunctionalTests/UdpClientTest.cs | 4 +- .../Compression/WebSocketDeflater.cs | 3 +- .../Compression/WebSocketInflater.cs | 3 +- .../src/Internal/Win32/RegistryKey.cs | 2 + .../SafeHandles/SafeFileHandle.Windows.cs | 1 + .../src/System/IO/FileSystem.Windows.cs | 1 + .../Runtime/InteropServices/SafeHandle.cs | 24 ++- .../Threading/EventWaitHandle.Windows.cs | 2 + .../src/System/Threading/Mutex.Windows.cs | 3 + .../src/System/Threading/Semaphore.Windows.cs | 4 + .../PEReader.EmbeddedPortablePdb.cs | 2 +- .../src/System/Security/Principal/Win32.cs | 8 +- .../Pal/Windows/DecryptorPalWindows.Decode.cs | 67 ++++--- .../Pal/Windows/HelpersWindows.cs | 5 + .../Pal/Windows/PkcsPalWindows.Encrypt.cs | 6 +- .../Security/Cryptography/Xml/EncryptedXml.cs | 4 +- .../Cryptography/AsymmetricAlgorithm.cs | 2 +- .../Cryptography/CapiHelper.Windows.cs | 3 + .../Security/Cryptography/CngHelpers.cs | 1 + .../Security/Cryptography/CngKey.Create.cs | 47 +++-- .../Security/Cryptography/CngKey.Import.cs | 60 +++--- .../Security/Cryptography/CngKey.Open.cs | 4 + .../Cryptography/CryptoConfigForwarder.cs | 14 +- .../Cryptography/DSACng.ImportExport.cs | 23 ++- .../Cryptography/ECDiffieHellmanCng.cs | 32 +++- .../ECDsa.Create.SecurityTransforms.cs | 22 ++- .../System/Security/Cryptography/ECDsaCng.cs | 32 +++- .../src/System/Security/Cryptography/HMAC.cs | 2 +- .../Security/Cryptography/HashAlgorithm.cs | 2 +- .../Cryptography/KeyedHashAlgorithm.cs | 2 +- .../Cryptography/RSACng.ImportExport.cs | 23 ++- .../Cryptography/SafeEvpPKeyHandle.OpenSsl.cs | 4 +- .../Cryptography/SymmetricAlgorithm.cs | 2 +- .../CertificatePal.Windows.Import.cs | 11 +- .../CertificatePal.Windows.PrivateKey.cs | 9 +- .../CertificatePal.Windows.cs | 6 +- .../X509Certificates/ChainPal.Windows.cs | 3 + .../X509Certificates/FindPal.Windows.cs | 4 +- .../OpenSslCertificateAssetDownloader.cs | 4 + .../StorePal.Windows.Import.cs | 47 ++++- .../X509Certificates/StorePal.Windows.cs | 3 + .../WindowsInterop.crypt32.cs | 5 +- .../X509Certificates/X509Pal.Apple.ECKey.cs | 2 + .../X509Pal.Windows.PublicKey.cs | 7 +- .../tests/CryptoConfigTests.cs | 11 +- .../tests/CryptoStream.cs | 6 +- .../tests/DSATests.cs | 7 +- .../tests/ECDsaTests.cs | 6 + .../tests/HmacTests.cs | 6 +- .../tests/Rfc2898Tests.cs | 2 +- .../tests/SignatureDescriptionTests.cs | 47 ++--- .../tests/TripleDesTests.cs | 6 +- .../src/System/Security/Principal/Win32.cs | 5 +- .../Security/Principal/WindowsIdentity.cs | 181 +++++++++++------- .../Security/Principal/WindowsPrincipal.cs | 47 +++-- .../tests/SecurityIdentifierTests.cs | 5 +- .../tests/WellKnownSidTypeTests.cs | 3 +- ...owsIdentityImpersonatedTests.netcoreapp.cs | 31 ++- .../tests/WindowsIdentityTests.cs | 8 +- .../tests/WindowsPrincipalTests.cs | 2 +- .../ServiceProcess/ServiceController.cs | 2 + .../tests/TestServiceProvider.cs | 7 +- .../Internal/ObjectToken/RegistryDataKey.cs | 1 + .../System/Threading/EventWaitHandleAcl.cs | 1 + .../src/System/Threading/MutexAcl.cs | 1 + .../src/System/Threading/SemaphoreAcl.cs | 3 + .../X509Certificates/X509Certificate2UI.cs | 9 +- .../X509Certificates/X509Utils.cs | 6 +- 151 files changed, 1073 insertions(+), 503 deletions(-) diff --git a/src/coreclr/vm/corelib.h b/src/coreclr/vm/corelib.h index e183ffc8451e7..3537c57d8cf05 100644 --- a/src/coreclr/vm/corelib.h +++ b/src/coreclr/vm/corelib.h @@ -792,6 +792,7 @@ DEFINE_CLASS(CALLCONV_SUPPRESSGCTRANSITION, CompilerServices, CallConvSup DEFINE_CLASS(CALLCONV_MEMBERFUNCTION, CompilerServices, CallConvMemberFunction) DEFINE_CLASS_U(Interop, SafeHandle, SafeHandle) +DEFINE_FIELD_U(_ctorStackTrace, SafeHandle, m_ctorStackTrace) DEFINE_FIELD_U(handle, SafeHandle, m_handle) DEFINE_FIELD_U(_state, SafeHandle, m_state) DEFINE_FIELD_U(_ownsHandle, SafeHandle, m_ownsHandle) diff --git a/src/coreclr/vm/object.h b/src/coreclr/vm/object.h index 0c4535209ce9a..bdfce25846059 100644 --- a/src/coreclr/vm/object.h +++ b/src/coreclr/vm/object.h @@ -1913,6 +1913,9 @@ class SafeHandle : public Object // Modifying the order or fields of this object may require // other changes to the classlib class definition of this // object or special handling when loading this system class. +#if DEBUG + STRINGREF m_ctorStackTrace; // Debug-only stack trace captured when the SafeHandle was constructed +#endif Volatile m_handle; Volatile m_state; // Combined ref count and closed/disposed state (for atomicity) Volatile m_ownsHandle; diff --git a/src/libraries/Common/src/Interop/Android/System.Security.Cryptography.Native.Android/Interop.Err.cs b/src/libraries/Common/src/Interop/Android/System.Security.Cryptography.Native.Android/Interop.Err.cs index 42071325b5417..237a6ac1b880a 100644 --- a/src/libraries/Common/src/Interop/Android/System.Security.Cryptography.Native.Android/Interop.Err.cs +++ b/src/libraries/Common/src/Interop/Android/System.Security.Cryptography.Native.Android/Interop.Err.cs @@ -94,7 +94,9 @@ internal static void CheckValidOpenSslHandle(SafeHandle handle) { if (handle == null || handle.IsInvalid) { - throw CreateOpenSslCryptographicException(); + Exception e = CreateOpenSslCryptographicException(); + handle?.Dispose(); + throw e; } } diff --git a/src/libraries/Common/src/Interop/OSX/System.Security.Cryptography.Native.Apple/Interop.SecKeyRef.macOS.cs b/src/libraries/Common/src/Interop/OSX/System.Security.Cryptography.Native.Apple/Interop.SecKeyRef.macOS.cs index b6c1abaf362ab..e4bca886045c9 100644 --- a/src/libraries/Common/src/Interop/OSX/System.Security.Cryptography.Native.Apple/Interop.SecKeyRef.macOS.cs +++ b/src/libraries/Common/src/Interop/OSX/System.Security.Cryptography.Native.Apple/Interop.SecKeyRef.macOS.cs @@ -54,10 +54,13 @@ internal static SafeSecKeyRefHandle ImportEphemeralKey(ReadOnlySpan keyBlo if (ret == 0) { - throw CreateExceptionForOSStatus(osStatus); + Exception e = CreateExceptionForOSStatus(osStatus); + keyHandle.Dispose(); + throw e; } Debug.Fail($"SecKeyImportEphemeral returned {ret}"); + keyHandle.Dispose(); throw new CryptographicException(); } diff --git a/src/libraries/Common/src/Interop/Unix/Interop.IOErrors.cs b/src/libraries/Common/src/Interop/Unix/Interop.IOErrors.cs index 80a737fea3f8b..137b39405962e 100644 --- a/src/libraries/Common/src/Interop/Unix/Interop.IOErrors.cs +++ b/src/libraries/Common/src/Interop/Unix/Interop.IOErrors.cs @@ -88,7 +88,9 @@ internal static TSafeHandle CheckIo(TSafeHandle handle, string? pat { if (handle.IsInvalid) { - ThrowExceptionForIoErrno(Sys.GetLastErrorInfo(), path, isDirectory); + Exception e = Interop.GetExceptionForIoErrno(Sys.GetLastErrorInfo(), path, isDirectory); + handle.Dispose(); + throw e; } return handle; diff --git a/src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.ERR.cs b/src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.ERR.cs index ff307db15e395..bfb17982624dd 100644 --- a/src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.ERR.cs +++ b/src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.ERR.cs @@ -91,7 +91,9 @@ internal static void CheckValidOpenSslHandle(SafeHandle handle) { if (handle == null || handle.IsInvalid) { - throw CreateOpenSslCryptographicException(); + Exception e = CreateOpenSslCryptographicException(); + handle?.Dispose(); + throw e; } } diff --git a/src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.EcDsa.ImportExport.cs b/src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.EcDsa.ImportExport.cs index 0810e71a14d3c..b8b545f5f17c8 100644 --- a/src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.EcDsa.ImportExport.cs +++ b/src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.EcDsa.ImportExport.cs @@ -84,8 +84,9 @@ internal static SafeEcKeyHandle EcKeyCreateByExplicitCurve(ECCurve curve) if (key == null || key.IsInvalid) { + Exception e = Interop.Crypto.CreateOpenSslCryptographicException(); key?.Dispose(); - throw Interop.Crypto.CreateOpenSslCryptographicException(); + throw e; } // EcKeyCreateByExplicitParameters may have polluted the error queue, but key was good in the end. diff --git a/src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.X509.cs b/src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.X509.cs index 1362a2a9a7525..56c954f1e9a30 100644 --- a/src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.X509.cs +++ b/src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.X509.cs @@ -151,7 +151,9 @@ internal static SafeX509StoreHandle X509ChainNew(SafeX509StackHandle systemTrust if (store.IsInvalid) { - throw CreateOpenSslCryptographicException(); + Exception e = CreateOpenSslCryptographicException(); + store.Dispose(); + throw e; } return store; diff --git a/src/libraries/Common/src/Interop/Windows/Advapi32/Interop.OpenThreadToken.cs b/src/libraries/Common/src/Interop/Windows/Advapi32/Interop.OpenThreadToken.cs index 2fd37e88408d9..9db97f34f1367 100644 --- a/src/libraries/Common/src/Interop/Windows/Advapi32/Interop.OpenThreadToken.cs +++ b/src/libraries/Common/src/Interop/Windows/Advapi32/Interop.OpenThreadToken.cs @@ -30,6 +30,7 @@ internal static bool OpenThreadToken(TokenAccessLevels desiredAccess, WinSecurit if (openAs == WinSecurityContext.Both) { openAsSelf = false; + tokenHandle.Dispose(); if (OpenThreadToken(Kernel32.GetCurrentThread(), desiredAccess, openAsSelf, out tokenHandle)) return true; } diff --git a/src/libraries/Common/src/Interop/Windows/BCrypt/BCryptAlgorithmCache.cs b/src/libraries/Common/src/Interop/Windows/BCrypt/BCryptAlgorithmCache.cs index 822090b10b18c..5de32d75706dc 100644 --- a/src/libraries/Common/src/Interop/Windows/BCrypt/BCryptAlgorithmCache.cs +++ b/src/libraries/Common/src/Interop/Windows/BCrypt/BCryptAlgorithmCache.cs @@ -3,7 +3,7 @@ using System; using System.Diagnostics; -using System.Collections.Generic; +using System.Collections.Concurrent; using System.Security.Cryptography; using Microsoft.Win32.SafeHandles; @@ -14,79 +14,54 @@ internal static partial class BCrypt { internal static class BCryptAlgorithmCache { + private static readonly ConcurrentDictionary<(string HashAlgorithmId, BCryptOpenAlgorithmProviderFlags Flags), (SafeBCryptAlgorithmHandle Handle, int HashSizeInBytes)> s_handles = new(); + /// - /// Return a SafeBCryptAlgorithmHandle of the desired algorithm and flags. This is a shared handle so do not dispose it! + /// Returns a SafeBCryptAlgorithmHandle of the desired algorithm and flags. This is a shared handle so do not dispose it! /// - public static SafeBCryptAlgorithmHandle GetCachedBCryptAlgorithmHandle(string hashAlgorithmId, BCryptOpenAlgorithmProviderFlags flags, out int hashSizeInBytes) + public static unsafe SafeBCryptAlgorithmHandle GetCachedBCryptAlgorithmHandle(string hashAlgorithmId, BCryptOpenAlgorithmProviderFlags flags, out int hashSizeInBytes) { - // There aren't that many hash algorithms around so rather than use a LowLevelDictionary and guard it with a lock, - // we'll use a simple list. To avoid locking, we'll recreate the entire list each time an entry is added and replace it atomically. - // - // This does mean that on occasion, racing threads may create two handles of the same type, but this is ok. + var key = (hashAlgorithmId, flags); - // Latch the _cache value into a local so we aren't disrupted by concurrent changes to it. - Entry[] cache = _cache; - foreach (Entry entry in cache) + while (true) { - if (entry.HashAlgorithmId == hashAlgorithmId && entry.Flags == flags) + if (s_handles.TryGetValue(key, out (SafeBCryptAlgorithmHandle Handle, int HashSizeInBytes) result)) { - hashSizeInBytes = entry.HashSizeInBytes; - return entry.Handle; + hashSizeInBytes = result.HashSizeInBytes; + return result.Handle; } - } - - SafeBCryptAlgorithmHandle safeBCryptAlgorithmHandle; - NTSTATUS ntStatus = Interop.BCrypt.BCryptOpenAlgorithmProvider(out safeBCryptAlgorithmHandle, hashAlgorithmId, null, flags); - if (ntStatus != NTSTATUS.STATUS_SUCCESS) - throw Interop.BCrypt.CreateCryptographicException(ntStatus); - - Array.Resize(ref cache, cache.Length + 1); - Entry newEntry = new Entry(hashAlgorithmId, flags, safeBCryptAlgorithmHandle); - cache[^1] = new Entry(hashAlgorithmId, flags, safeBCryptAlgorithmHandle); - - // Atomically overwrite the cache with our new cache. It's possible some other thread raced to add a new entry with us - if so, one of the new entries - // will be lost and the next request will have to allocate it again. That's considered acceptable collateral damage. - _cache = cache; - hashSizeInBytes = newEntry.HashSizeInBytes; - return newEntry.Handle; - } - - private static volatile Entry[] _cache = Array.Empty(); - - private struct Entry - { - public unsafe Entry(string hashAlgorithmId, BCryptOpenAlgorithmProviderFlags flags, SafeBCryptAlgorithmHandle handle) - : this() - { - HashAlgorithmId = hashAlgorithmId; - Flags = flags; - Handle = handle; + NTSTATUS ntStatus = Interop.BCrypt.BCryptOpenAlgorithmProvider(out SafeBCryptAlgorithmHandle handle, key.hashAlgorithmId, null, key.flags); + if (ntStatus != NTSTATUS.STATUS_SUCCESS) + { + Exception e = Interop.BCrypt.CreateCryptographicException(ntStatus); + handle.Dispose(); + throw e; + } int hashSize; - NTSTATUS ntStatus = Interop.BCrypt.BCryptGetProperty( + ntStatus = Interop.BCrypt.BCryptGetProperty( handle, Interop.BCrypt.BCryptPropertyStrings.BCRYPT_HASH_LENGTH, &hashSize, sizeof(int), out int cbHashSize, 0); - if (ntStatus != NTSTATUS.STATUS_SUCCESS) { - throw Interop.BCrypt.CreateCryptographicException(ntStatus); + Exception e = Interop.BCrypt.CreateCryptographicException(ntStatus); + handle.Dispose(); + throw e; } Debug.Assert(cbHashSize == sizeof(int)); Debug.Assert(hashSize > 0); - HashSizeInBytes = hashSize; + if (!s_handles.TryAdd(key, (handle, hashSize))) + { + handle.Dispose(); + } } - - public string HashAlgorithmId { get; private set; } - public BCryptOpenAlgorithmProviderFlags Flags { get; private set; } - public SafeBCryptAlgorithmHandle Handle { get; private set; } - public int HashSizeInBytes { get; private set; } } } } diff --git a/src/libraries/Common/src/Interop/Windows/Crypt32/Interop.CryptEncodeDecodeWrappers.cs b/src/libraries/Common/src/Interop/Windows/Crypt32/Interop.CryptEncodeDecodeWrappers.cs index d26bd3404b7b1..8f26d00a64c2d 100644 --- a/src/libraries/Common/src/Interop/Windows/Crypt32/Interop.CryptEncodeDecodeWrappers.cs +++ b/src/libraries/Common/src/Interop/Windows/Crypt32/Interop.CryptEncodeDecodeWrappers.cs @@ -29,11 +29,17 @@ internal static unsafe SafeHandle CryptDecodeObjectToMemory(CryptDecodeObjectStr int cbRequired = 0; if (!CryptDecodeObject(MsgEncodingType.All, (IntPtr)lpszStructType, pbEncoded, cbEncoded, 0, null, ref cbRequired)) + { throw Marshal.GetLastWin32Error().ToCryptographicException(); + } SafeHandle sh = SafeHeapAllocHandle.Alloc(cbRequired); if (!CryptDecodeObject(MsgEncodingType.All, (IntPtr)lpszStructType, pbEncoded, cbEncoded, 0, (void*)sh.DangerousGetHandle(), ref cbRequired)) - throw Marshal.GetLastWin32Error().ToCryptographicException(); + { + Exception e = Marshal.GetLastWin32Error().ToCryptographicException(); + sh.Dispose(); + throw e; + } return sh; } @@ -42,11 +48,15 @@ internal static unsafe byte[] CryptEncodeObjectToByteArray(CryptDecodeObjectStru { int cb = 0; if (!CryptEncodeObject(MsgEncodingType.All, lpszStructType, decoded, null, ref cb)) + { throw Marshal.GetLastWin32Error().ToCryptographicException(); + } byte[] encoded = new byte[cb]; if (!CryptEncodeObject(MsgEncodingType.All, lpszStructType, decoded, encoded, ref cb)) + { throw Marshal.GetLastWin32Error().ToCryptographicException(); + } return encoded.Resize(cb); } diff --git a/src/libraries/Common/src/Interop/Windows/Kernel32/Interop.DuplicateHandle_SafeAccessTokenHandle.cs b/src/libraries/Common/src/Interop/Windows/Kernel32/Interop.DuplicateHandle_SafeAccessTokenHandle.cs index b23d9a394b0b7..fb80482270b74 100644 --- a/src/libraries/Common/src/Interop/Windows/Kernel32/Interop.DuplicateHandle_SafeAccessTokenHandle.cs +++ b/src/libraries/Common/src/Interop/Windows/Kernel32/Interop.DuplicateHandle_SafeAccessTokenHandle.cs @@ -15,7 +15,7 @@ internal static partial bool DuplicateHandle( IntPtr hSourceProcessHandle, IntPtr hSourceHandle, IntPtr hTargetProcessHandle, - ref SafeAccessTokenHandle lpTargetHandle, + out SafeAccessTokenHandle lpTargetHandle, uint dwDesiredAccess, [MarshalAs(UnmanagedType.Bool)] bool bInheritHandle, uint dwOptions); diff --git a/src/libraries/Common/src/Interop/Windows/NCrypt/Interop.NCryptDeriveSecretAgreement.cs b/src/libraries/Common/src/Interop/Windows/NCrypt/Interop.NCryptDeriveSecretAgreement.cs index 6886fe241c0f1..67607f41148aa 100644 --- a/src/libraries/Common/src/Interop/Windows/NCrypt/Interop.NCryptDeriveSecretAgreement.cs +++ b/src/libraries/Common/src/Interop/Windows/NCrypt/Interop.NCryptDeriveSecretAgreement.cs @@ -44,6 +44,7 @@ internal static SafeNCryptSecretHandle DeriveSecretAgreement( if (error != ErrorCode.ERROR_SUCCESS) { + secretAgreement.Dispose(); throw error.ToCryptographicException(); } diff --git a/src/libraries/Common/src/System/Net/ContextAwareResult.Windows.cs b/src/libraries/Common/src/System/Net/ContextAwareResult.Windows.cs index e4124f6204d98..94e0703999e90 100644 --- a/src/libraries/Common/src/System/Net/ContextAwareResult.Windows.cs +++ b/src/libraries/Common/src/System/Net/ContextAwareResult.Windows.cs @@ -10,7 +10,6 @@ internal sealed partial class ContextAwareResult { private WindowsIdentity? _windowsIdentity; - // Security: We need an assert for a call into WindowsIdentity.GetCurrent. private void SafeCaptureIdentity() { Debug.Assert(OperatingSystem.IsWindows()); diff --git a/src/libraries/Common/src/System/Security/Cryptography/DSAOpenSsl.cs b/src/libraries/Common/src/System/Security/Cryptography/DSAOpenSsl.cs index 4ecb5554b2284..19bf822cbdfb4 100644 --- a/src/libraries/Common/src/System/Security/Cryptography/DSAOpenSsl.cs +++ b/src/libraries/Common/src/System/Security/Cryptography/DSAOpenSsl.cs @@ -126,7 +126,9 @@ public override void ImportParameters(DSAParameters parameters) parameters.Y, parameters.Y.Length, parameters.X, parameters.X != null ? parameters.X.Length : 0)) { - throw Interop.Crypto.CreateOpenSslCryptographicException(); + Exception e = Interop.Crypto.CreateOpenSslCryptographicException(); + key.Dispose(); + throw e; } SetKey(key); @@ -183,7 +185,9 @@ private SafeDsaHandle GenerateKey() if (!Interop.Crypto.DsaGenerateKey(out key, KeySize)) { - throw Interop.Crypto.CreateOpenSslCryptographicException(); + Exception e = Interop.Crypto.CreateOpenSslCryptographicException(); + key.Dispose(); + throw e; } return key; diff --git a/src/libraries/Common/src/System/Security/Cryptography/ECAndroid.ImportExport.cs b/src/libraries/Common/src/System/Security/Cryptography/ECAndroid.ImportExport.cs index bff866b4edf10..6015d06387dfb 100644 --- a/src/libraries/Common/src/System/Security/Cryptography/ECAndroid.ImportExport.cs +++ b/src/libraries/Common/src/System/Security/Cryptography/ECAndroid.ImportExport.cs @@ -34,6 +34,7 @@ public int ImportParameters(ECParameters parameters) if (key == null || key.IsInvalid) { + key?.Dispose(); throw new CryptographicException(); } @@ -175,7 +176,10 @@ public static SafeEcKeyHandle GenerateKeyByKeySize(int keySize) SafeEcKeyHandle? key = Interop.AndroidCrypto.EcKeyCreateByOid(oid); if (key == null || key.IsInvalid) + { + key?.Dispose(); throw new PlatformNotSupportedException(SR.Format(SR.Cryptography_CurveNotSupported, oid)); + } return key; } diff --git a/src/libraries/Common/src/System/Security/Cryptography/ECAndroid.cs b/src/libraries/Common/src/System/Security/Cryptography/ECAndroid.cs index a90f729c0b31d..671b6b63643fc 100644 --- a/src/libraries/Common/src/System/Security/Cryptography/ECAndroid.cs +++ b/src/libraries/Common/src/System/Security/Cryptography/ECAndroid.cs @@ -69,6 +69,7 @@ internal int GenerateKey(ECCurve curve) if (key == null || key.IsInvalid) { + key?.Dispose(); throw new PlatformNotSupportedException(SR.Format(SR.Cryptography_CurveNotSupported, oid)); } @@ -80,6 +81,7 @@ internal int GenerateKey(ECCurve curve) if (key == null || key.IsInvalid) { + key?.Dispose(); throw new CryptographicException(); } diff --git a/src/libraries/Common/src/System/Security/Cryptography/ECCng.ImportExport.cs b/src/libraries/Common/src/System/Security/Cryptography/ECCng.ImportExport.cs index 5e394540dad20..8e8726fb3cd2c 100644 --- a/src/libraries/Common/src/System/Security/Cryptography/ECCng.ImportExport.cs +++ b/src/libraries/Common/src/System/Security/Cryptography/ECCng.ImportExport.cs @@ -503,6 +503,7 @@ ref MemoryMarshal.GetReference(keyBlob), if (errorCode != ErrorCode.ERROR_SUCCESS) { Exception e = errorCode.ToCryptographicException(); + keyHandle.Dispose(); if (errorCode == ErrorCode.NTE_INVALID_PARAMETER) { throw new PlatformNotSupportedException(SR.Format(SR.Cryptography_CurveNotSupported, curveName), e); diff --git a/src/libraries/Common/src/System/Security/Cryptography/ECDiffieHellmanCng.cs b/src/libraries/Common/src/System/Security/Cryptography/ECDiffieHellmanCng.cs index f61f2326e8beb..b79dd9474a91d 100644 --- a/src/libraries/Common/src/System/Security/Cryptography/ECDiffieHellmanCng.cs +++ b/src/libraries/Common/src/System/Security/Cryptography/ECDiffieHellmanCng.cs @@ -20,8 +20,16 @@ public ECDiffieHellmanCng(int keySize) [SupportedOSPlatform("windows")] public ECDiffieHellmanCng(ECCurve curve) { - // GenerateKey will already do all of the validation we need. - GenerateKey(curve); + try + { + // GenerateKey will already do all of the validation we need. + GenerateKey(curve); + } + catch + { + Dispose(); + throw; + } } public override int KeySize diff --git a/src/libraries/Common/src/System/Security/Cryptography/ECDiffieHellmanOpenSslPublicKey.cs b/src/libraries/Common/src/System/Security/Cryptography/ECDiffieHellmanOpenSslPublicKey.cs index 311dd4605e7a7..ae6064bef2887 100644 --- a/src/libraries/Common/src/System/Security/Cryptography/ECDiffieHellmanOpenSslPublicKey.cs +++ b/src/libraries/Common/src/System/Security/Cryptography/ECDiffieHellmanOpenSslPublicKey.cs @@ -22,8 +22,9 @@ internal ECDiffieHellmanOpenSslPublicKey(SafeEvpPKeyHandle pkeyHandle) if (key.IsInvalid) { + Exception e = Interop.Crypto.CreateOpenSslCryptographicException(); key.Dispose(); - throw Interop.Crypto.CreateOpenSslCryptographicException(); + throw e; } _key = new ECOpenSsl(key); diff --git a/src/libraries/Common/src/System/Security/Cryptography/ECDsaCng.cs b/src/libraries/Common/src/System/Security/Cryptography/ECDsaCng.cs index 17e6a19687f99..0edc06e488069 100644 --- a/src/libraries/Common/src/System/Security/Cryptography/ECDsaCng.cs +++ b/src/libraries/Common/src/System/Security/Cryptography/ECDsaCng.cs @@ -16,8 +16,16 @@ public sealed partial class ECDsaCng : ECDsa, IRuntimeAlgorithm [SupportedOSPlatform("windows")] public ECDsaCng(ECCurve curve) { - // Specified curves generate the key immediately - GenerateKey(curve); + try + { + // Specified curves generate the key immediately + GenerateKey(curve); + } + catch + { + Dispose(); + throw; + } } /// diff --git a/src/libraries/Common/src/System/Security/Cryptography/ECOpenSsl.ImportExport.cs b/src/libraries/Common/src/System/Security/Cryptography/ECOpenSsl.ImportExport.cs index 93ac077893dd1..9f2c31fa7aebf 100644 --- a/src/libraries/Common/src/System/Security/Cryptography/ECOpenSsl.ImportExport.cs +++ b/src/libraries/Common/src/System/Security/Cryptography/ECOpenSsl.ImportExport.cs @@ -38,7 +38,9 @@ public int ImportParameters(ECParameters parameters) if (key == null || key.IsInvalid) { - throw Interop.Crypto.CreateOpenSslCryptographicException(); + Exception e = Interop.Crypto.CreateOpenSslCryptographicException(); + key?.Dispose(); + throw e; } // The Import* methods above may have polluted the error queue even if in the end they succeeded. @@ -183,10 +185,16 @@ public static SafeEcKeyHandle GenerateKeyByKeySize(int keySize) SafeEcKeyHandle? key = Interop.Crypto.EcKeyCreateByOid(oid); if (key == null || key.IsInvalid) + { + key?.Dispose(); throw new PlatformNotSupportedException(SR.Format(SR.Cryptography_CurveNotSupported, oid)); + } if (!Interop.Crypto.EcKeyGenerateKey(key)) + { + key.Dispose(); throw Interop.Crypto.CreateOpenSslCryptographicException(); + } return key; } diff --git a/src/libraries/Common/src/System/Security/Cryptography/ECOpenSsl.cs b/src/libraries/Common/src/System/Security/Cryptography/ECOpenSsl.cs index 5157a2e1246c5..29c5d8c944a24 100644 --- a/src/libraries/Common/src/System/Security/Cryptography/ECOpenSsl.cs +++ b/src/libraries/Common/src/System/Security/Cryptography/ECOpenSsl.cs @@ -93,6 +93,7 @@ internal int GenerateKey(ECCurve curve) if (key == null || key.IsInvalid) { + key?.Dispose(); throw new PlatformNotSupportedException(SR.Format(SR.Cryptography_CurveNotSupported, oid)); } diff --git a/src/libraries/Common/src/System/Security/Cryptography/RSAAndroid.cs b/src/libraries/Common/src/System/Security/Cryptography/RSAAndroid.cs index 5de515013ae9a..ffa351411bc22 100644 --- a/src/libraries/Common/src/System/Security/Cryptography/RSAAndroid.cs +++ b/src/libraries/Common/src/System/Security/Cryptography/RSAAndroid.cs @@ -355,6 +355,7 @@ public override void ImportParameters(RSAParameters parameters) if (key is null || key.IsInvalid) { + key?.Dispose(); throw new CryptographicException(); } @@ -434,6 +435,7 @@ public override unsafe void ImportRSAPublicKey(ReadOnlySpan source, out in SafeRsaHandle key = Interop.AndroidCrypto.DecodeRsaSubjectPublicKeyInfo(writer.Encode()); if (key is null || key.IsInvalid) { + key?.Dispose(); throw new CryptographicException(); } @@ -567,6 +569,7 @@ private SafeRsaHandle GenerateKey() if (key is null || key.IsInvalid) { + key?.Dispose(); throw new CryptographicException(); } diff --git a/src/libraries/Common/tests/System/IO/Compression/CompressionStreamUnitTestBase.cs b/src/libraries/Common/tests/System/IO/Compression/CompressionStreamUnitTestBase.cs index d581fac9dc1e1..e3222ea9fd69a 100644 --- a/src/libraries/Common/tests/System/IO/Compression/CompressionStreamUnitTestBase.cs +++ b/src/libraries/Common/tests/System/IO/Compression/CompressionStreamUnitTestBase.cs @@ -139,7 +139,7 @@ public virtual async Task Dispose_WithUnfinishedReadAsync() using (var readStream = await ManualSyncMemoryStream.GetStreamFromFileAsync(compressedPath, false)) { - var decompressor = CreateStream(readStream, CompressionMode.Decompress, true); + using var decompressor = CreateStream(readStream, CompressionMode.Decompress, true); Task task = decompressor.ReadAsync(uncompressedBytes, 0, uncompressedBytes.Length); decompressor.Dispose(); readStream.manualResetEvent.Set(); @@ -154,7 +154,7 @@ public async Task Read(string testFile) { var uncompressedStream = await LocalMemoryStream.readAppFileAsync(testFile); var compressedStream = await LocalMemoryStream.readAppFileAsync(CompressedTestFile(testFile)); - var decompressor = CreateStream(compressedStream, CompressionMode.Decompress); + using var decompressor = CreateStream(compressedStream, CompressionMode.Decompress); var decompressorOutput = new MemoryStream(); int _bufferSize = 1024; @@ -198,7 +198,7 @@ public async Task Read_EndOfStreamPosition() compressedStream.Write(bytes, 0, _bufferSize); compressedStream.Write(bytes, 0, _bufferSize); compressedStream.Position = 0; - var decompressor = CreateStream(compressedStream, CompressionMode.Decompress); + using var decompressor = CreateStream(compressedStream, CompressionMode.Decompress); while (decompressor.Read(bytes, 0, _bufferSize) > 0); Assert.Equal(((compressedEndPosition / BufferSize) + 1) * BufferSize, compressedStream.Position); @@ -211,7 +211,7 @@ public async Task Read_BaseStreamSlowly() string testFile = UncompressedTestFile(); var uncompressedStream = await LocalMemoryStream.readAppFileAsync(testFile); var compressedStream = new BadWrappedStream(BadWrappedStream.Mode.ReadSlowly, File.ReadAllBytes(CompressedTestFile(testFile))); - var decompressor = CreateStream(compressedStream, CompressionMode.Decompress); + using var decompressor = CreateStream(compressedStream, CompressionMode.Decompress); var decompressorOutput = new MemoryStream(); int _bufferSize = 1024; @@ -247,7 +247,7 @@ public async Task Read_BaseStreamSlowly() public void CanDisposeBaseStream(CompressionMode mode) { var ms = new MemoryStream(); - var compressor = CreateStream(ms, mode); + using var compressor = CreateStream(ms, mode); ms.Dispose(); // This would throw if this was invalid } @@ -307,7 +307,8 @@ IEnumerable> CtorFunctions() int _bufferSize = 1024; var bytes = new byte[_bufferSize]; var baseStream = new MemoryStream(bytes, writable: true); - Stream compressor = create(baseStream); + + using Stream compressor = create(baseStream); //Write some data and Close the stream string strData = "Test Data"; @@ -321,7 +322,7 @@ IEnumerable> CtorFunctions() //Read the data byte[] data2 = new byte[_bufferSize]; baseStream = new MemoryStream(bytes, writable: false); - var decompressor = CreateStream(baseStream, CompressionMode.Decompress); + using var decompressor = CreateStream(baseStream, CompressionMode.Decompress); int size = decompressor.Read(data2, 0, _bufferSize - 5); //Verify the data roundtripped @@ -436,7 +437,7 @@ public async Task BaseStream_Modify(CompressionMode mode) public void BaseStream_NullAfterDisposeWithFalseLeaveOpen(CompressionMode mode) { var ms = new MemoryStream(); - var compressor = CreateStream(ms, mode); + using var compressor = CreateStream(ms, mode); compressor.Dispose(); Assert.Null(BaseStream(compressor)); @@ -451,7 +452,7 @@ public void BaseStream_NullAfterDisposeWithFalseLeaveOpen(CompressionMode mode) public async Task BaseStream_ValidAfterDisposeWithTrueLeaveOpen(CompressionMode mode) { var ms = await LocalMemoryStream.readAppFileAsync(CompressedTestFile(UncompressedTestFile())); - var decompressor = CreateStream(ms, mode, leaveOpen: true); + using var decompressor = CreateStream(ms, mode, leaveOpen: true); var baseStream = BaseStream(decompressor); Assert.Same(ms, baseStream); decompressor.Dispose(); diff --git a/src/libraries/Common/tests/System/IO/ReparsePointUtilities.cs b/src/libraries/Common/tests/System/IO/ReparsePointUtilities.cs index 7a1757a8249ac..4153761111ae9 100644 --- a/src/libraries/Common/tests/System/IO/ReparsePointUtilities.cs +++ b/src/libraries/Common/tests/System/IO/ReparsePointUtilities.cs @@ -86,7 +86,7 @@ public static bool CreateSymbolicLink(string linkPath, string targetPath, bool i bool isWindows = OperatingSystem.IsWindows(); #endif - Process symLinkProcess = new Process(); + using Process symLinkProcess = new Process(); if (isWindows) { symLinkProcess.StartInfo.FileName = "cmd"; @@ -170,7 +170,7 @@ private static ProcessStartInfo CreateProcessStartInfo(string fileName, params s private static bool RunProcess(ProcessStartInfo startInfo) { - var process = Process.Start(startInfo); + using Process process = Process.Start(startInfo); process.WaitForExit(); return process.ExitCode == 0; } diff --git a/src/libraries/Common/tests/System/Net/Http/DefaultCredentialsTest.cs b/src/libraries/Common/tests/System/Net/Http/DefaultCredentialsTest.cs index 76ef1eb99647d..e51991b0cb21e 100644 --- a/src/libraries/Common/tests/System/Net/Http/DefaultCredentialsTest.cs +++ b/src/libraries/Common/tests/System/Net/Http/DefaultCredentialsTest.cs @@ -86,7 +86,8 @@ public async Task UseDefaultCredentials_SetTrue_ConnectAsCurrentIdentity(string Assert.Equal(HttpStatusCode.OK, response.StatusCode); string responseBody = await response.Content.ReadAsStringAsync(); - WindowsIdentity currentIdentity = WindowsIdentity.GetCurrent(); + + using WindowsIdentity currentIdentity = WindowsIdentity.GetCurrent(); _output.WriteLine("currentIdentity={0}", currentIdentity.Name); VerifyAuthentication(responseBody, true, currentIdentity.Name); } @@ -110,7 +111,8 @@ public async Task Credentials_SetToWrappedDefaultCredential_ConnectAsCurrentIden Assert.Equal(HttpStatusCode.OK, response.StatusCode); string responseBody = await response.Content.ReadAsStringAsync(); - WindowsIdentity currentIdentity = WindowsIdentity.GetCurrent(); + + using WindowsIdentity currentIdentity = WindowsIdentity.GetCurrent(); _output.WriteLine("currentIdentity={0}", currentIdentity.Name); VerifyAuthentication(responseBody, true, currentIdentity.Name); } diff --git a/src/libraries/Common/tests/System/Net/Sockets/SocketTestServerAsync.cs b/src/libraries/Common/tests/System/Net/Sockets/SocketTestServerAsync.cs index e42d4b55cd810..60bde7692b766 100644 --- a/src/libraries/Common/tests/System/Net/Sockets/SocketTestServerAsync.cs +++ b/src/libraries/Common/tests/System/Net/Sockets/SocketTestServerAsync.cs @@ -26,7 +26,7 @@ public class SocketTestServerAsync : SocketTestServer private SocketAsyncEventArgsPool _readWritePool; // Pool of reusable SocketAsyncEventArgs objects for write, read and accept socket operations. private int _totalBytesRead; // Counter of the total # bytes received by the server. private int _numConnectedSockets; // The total number of clients connected to the server. - private Semaphore _maxNumberAcceptedClientsSemaphore; + private SemaphoreSlim _maxNumberAcceptedClientsSemaphore; private int _acceptRetryCount = 10; private ProtocolType _protocolType; @@ -49,7 +49,7 @@ public SocketTestServerAsync(int numConnections, int receiveBufferSize, EndPoint receiveBufferSize); _readWritePool = new SocketAsyncEventArgsPool(numConnections); - _maxNumberAcceptedClientsSemaphore = new Semaphore(numConnections, numConnections); + _maxNumberAcceptedClientsSemaphore = new SemaphoreSlim(numConnections, numConnections); _protocolType = protocolType; Init(); Start(localEndPoint); @@ -136,7 +136,7 @@ private void StartAccept(SocketAsyncEventArgs acceptEventArg) } _log.WriteLine(this.GetHashCode() + " StartAccept(_numConnectedSockets={0})", _numConnectedSockets); - if (!_maxNumberAcceptedClientsSemaphore.WaitOne(TestSettings.PassingTestTimeout)) + if (!_maxNumberAcceptedClientsSemaphore.Wait(TestSettings.PassingTestTimeout)) { throw new TimeoutException("Timeout waiting for client connection."); } diff --git a/src/libraries/Common/tests/System/Security/Cryptography/AlgorithmImplementations/AES/AesCipherTests.cs b/src/libraries/Common/tests/System/Security/Cryptography/AlgorithmImplementations/AES/AesCipherTests.cs index 8bf77c7e73b3d..95a8e74076317 100644 --- a/src/libraries/Common/tests/System/Security/Cryptography/AlgorithmImplementations/AES/AesCipherTests.cs +++ b/src/libraries/Common/tests/System/Security/Cryptography/AlgorithmImplementations/AES/AesCipherTests.cs @@ -589,7 +589,8 @@ public static void WrongKeyFailDecrypt() Assert.Throws(() => { using (MemoryStream input = new MemoryStream(encryptedBytes)) - using (CryptoStream cryptoStream = new CryptoStream(input, aes.CreateDecryptor(), CryptoStreamMode.Read)) + using (ICryptoTransform decryptor = aes.CreateDecryptor()) + using (CryptoStream cryptoStream = new CryptoStream(input, decryptor, CryptoStreamMode.Read)) using (MemoryStream output = new MemoryStream()) { cryptoStream.CopyTo(output); @@ -639,7 +640,8 @@ public static void WrongKeyFailDecrypt_2() Assert.Throws(() => { using (MemoryStream input = new MemoryStream(encryptedBytes)) - using (CryptoStream cryptoStream = new CryptoStream(input, aes.CreateDecryptor(), CryptoStreamMode.Read)) + using (ICryptoTransform decryptor = aes.CreateDecryptor()) + using (CryptoStream cryptoStream = new CryptoStream(input, decryptor, CryptoStreamMode.Read)) using (MemoryStream output = new MemoryStream()) { cryptoStream.CopyTo(output); @@ -1026,7 +1028,8 @@ public static void AesZeroPad(CipherMode cipherMode) byte[] encryptedBytes; using (MemoryStream input = new MemoryStream(s_multiBlockBytes)) - using (CryptoStream cryptoStream = new CryptoStream(input, aes.CreateEncryptor(), CryptoStreamMode.Read)) + using (ICryptoTransform encryptor = aes.CreateEncryptor()) + using (CryptoStream cryptoStream = new CryptoStream(input, encryptor, CryptoStreamMode.Read)) using (MemoryStream output = new MemoryStream()) { cryptoStream.CopyTo(output); @@ -1034,7 +1037,8 @@ public static void AesZeroPad(CipherMode cipherMode) } using (MemoryStream input = new MemoryStream(encryptedBytes)) - using (CryptoStream cryptoStream = new CryptoStream(input, aes.CreateDecryptor(), CryptoStreamMode.Read)) + using (ICryptoTransform decryptor = aes.CreateDecryptor()) + using (CryptoStream cryptoStream = new CryptoStream(input, decryptor, CryptoStreamMode.Read)) using (MemoryStream output = new MemoryStream()) { cryptoStream.CopyTo(output); @@ -1092,7 +1096,8 @@ private static void RandomKeyRoundtrip(Aes aes) byte[] encryptedBytes; using (MemoryStream input = new MemoryStream(s_multiBlockBytes)) - using (CryptoStream cryptoStream = new CryptoStream(input, aes.CreateEncryptor(), CryptoStreamMode.Read)) + using (ICryptoTransform encryptor = aes.CreateEncryptor()) + using (CryptoStream cryptoStream = new CryptoStream(input, encryptor, CryptoStreamMode.Read)) using (MemoryStream output = new MemoryStream()) { cryptoStream.CopyTo(output); @@ -1102,7 +1107,8 @@ private static void RandomKeyRoundtrip(Aes aes) Assert.NotEqual(s_multiBlockBytes, encryptedBytes); using (MemoryStream input = new MemoryStream(encryptedBytes)) - using (CryptoStream cryptoStream = new CryptoStream(input, aes.CreateDecryptor(), CryptoStreamMode.Read)) + using (ICryptoTransform decryptor = aes.CreateDecryptor()) + using (CryptoStream cryptoStream = new CryptoStream(input, decryptor, CryptoStreamMode.Read)) using (MemoryStream output = new MemoryStream()) { cryptoStream.CopyTo(output); @@ -1139,7 +1145,8 @@ private static void TestAesDecrypt( } using (MemoryStream input = new MemoryStream(encryptedBytes)) - using (CryptoStream cryptoStream = new CryptoStream(input, aes.CreateDecryptor(), CryptoStreamMode.Read)) + using (ICryptoTransform decryptor = aes.CreateDecryptor()) + using (CryptoStream cryptoStream = new CryptoStream(input, decryptor, CryptoStreamMode.Read)) using (MemoryStream output = new MemoryStream()) { cryptoStream.CopyTo(output); @@ -1223,7 +1230,8 @@ private static void TestAesTransformDirectKey( private static byte[] AesEncryptDirectKey(Aes aes, byte[] key, byte[] iv, byte[] plainBytes) { using (MemoryStream output = new MemoryStream()) - using (CryptoStream cryptoStream = new CryptoStream(output, aes.CreateEncryptor(key, iv), CryptoStreamMode.Write)) + using (ICryptoTransform encryptor = aes.CreateEncryptor(key, iv)) + using (CryptoStream cryptoStream = new CryptoStream(output, encryptor, CryptoStreamMode.Write)) { cryptoStream.Write(plainBytes, 0, plainBytes.Length); cryptoStream.FlushFinalBlock(); @@ -1235,7 +1243,8 @@ private static byte[] AesEncryptDirectKey(Aes aes, byte[] key, byte[] iv, byte[] private static byte[] AesDecryptDirectKey(Aes aes, byte[] key, byte[] iv, byte[] cipherBytes) { using (MemoryStream output = new MemoryStream()) - using (CryptoStream cryptoStream = new CryptoStream(output, aes.CreateDecryptor(key, iv), CryptoStreamMode.Write)) + using (ICryptoTransform decryptor = aes.CreateDecryptor(key, iv)) + using (CryptoStream cryptoStream = new CryptoStream(output, decryptor, CryptoStreamMode.Write)) { cryptoStream.Write(cipherBytes, 0, cipherBytes.Length); cryptoStream.FlushFinalBlock(); diff --git a/src/libraries/Common/tests/System/Security/Cryptography/AlgorithmImplementations/AES/AesContractTests.cs b/src/libraries/Common/tests/System/Security/Cryptography/AlgorithmImplementations/AES/AesContractTests.cs index dc8940d6b38d9..9de09ff120325 100644 --- a/src/libraries/Common/tests/System/Security/Cryptography/AlgorithmImplementations/AES/AesContractTests.cs +++ b/src/libraries/Common/tests/System/Security/Cryptography/AlgorithmImplementations/AES/AesContractTests.cs @@ -261,8 +261,9 @@ public static void VerifyIVGeneration() public static void ValidateEncryptorProperties() { using (Aes aes = AesFactory.Create()) + using (ICryptoTransform encryptor = aes.CreateEncryptor()) { - ValidateTransformProperties(aes, aes.CreateEncryptor()); + ValidateTransformProperties(aes, encryptor); } } @@ -271,8 +272,9 @@ public static void ValidateEncryptorProperties() public static void ValidateDecryptorProperties() { using (Aes aes = AesFactory.Create()) + using (ICryptoTransform decryptor = aes.CreateDecryptor()) { - ValidateTransformProperties(aes, aes.CreateDecryptor()); + ValidateTransformProperties(aes, decryptor); } } diff --git a/src/libraries/Common/tests/System/Security/Cryptography/AlgorithmImplementations/AES/AesCornerTests.cs b/src/libraries/Common/tests/System/Security/Cryptography/AlgorithmImplementations/AES/AesCornerTests.cs index ffbbac78c65bc..11aefef02e20a 100644 --- a/src/libraries/Common/tests/System/Security/Cryptography/AlgorithmImplementations/AES/AesCornerTests.cs +++ b/src/libraries/Common/tests/System/Security/Cryptography/AlgorithmImplementations/AES/AesCornerTests.cs @@ -31,7 +31,7 @@ public static void EncryptorReusability() a.Mode = CipherMode.CBC; a.Padding = PaddingMode.None; - ICryptoTransform encryptor = a.CreateEncryptor(); + using ICryptoTransform encryptor = a.CreateEncryptor(); Assert.True(encryptor.CanReuseTransform); for (int i = 0; i < 4; i++) @@ -67,10 +67,10 @@ public static void TransformStateSeparation() MemoryStream cipher1 = new MemoryStream(cipher); MemoryStream cipher2 = new MemoryStream(cipher); - ICryptoTransform encryptor1 = a.CreateEncryptor(); - ICryptoTransform encryptor2 = a.CreateEncryptor(); - ICryptoTransform decryptor1 = a.CreateDecryptor(); - ICryptoTransform decryptor2 = a.CreateDecryptor(); + using ICryptoTransform encryptor1 = a.CreateEncryptor(); + using ICryptoTransform encryptor2 = a.CreateEncryptor(); + using ICryptoTransform decryptor1 = a.CreateDecryptor(); + using ICryptoTransform decryptor2 = a.CreateDecryptor(); List encryptionCollector1 = new List(); List encryptionCollector2 = new List(); diff --git a/src/libraries/Common/tests/System/Security/Cryptography/AlgorithmImplementations/DES/DESCipherTests.cs b/src/libraries/Common/tests/System/Security/Cryptography/AlgorithmImplementations/DES/DESCipherTests.cs index 707db864be13f..0d24acaf85281 100644 --- a/src/libraries/Common/tests/System/Security/Cryptography/AlgorithmImplementations/DES/DESCipherTests.cs +++ b/src/libraries/Common/tests/System/Security/Cryptography/AlgorithmImplementations/DES/DESCipherTests.cs @@ -578,7 +578,8 @@ private static void TestDESTransformDirectKey( private static byte[] DESEncryptDirectKey(DES des, byte[] key, byte[] iv, byte[] plainBytes) { using (MemoryStream output = new MemoryStream()) - using (CryptoStream cryptoStream = new CryptoStream(output, des.CreateEncryptor(key, iv), CryptoStreamMode.Write)) + using (ICryptoTransform encryptor = des.CreateEncryptor(key, iv)) + using (CryptoStream cryptoStream = new CryptoStream(output, encryptor, CryptoStreamMode.Write)) { cryptoStream.Write(plainBytes, 0, plainBytes.Length); cryptoStream.FlushFinalBlock(); @@ -590,7 +591,8 @@ private static byte[] DESEncryptDirectKey(DES des, byte[] key, byte[] iv, byte[] private static byte[] DESDecryptDirectKey(DES des, byte[] key, byte[] iv, byte[] cipherBytes) { using (MemoryStream output = new MemoryStream()) - using (CryptoStream cryptoStream = new CryptoStream(output, des.CreateDecryptor(key, iv), CryptoStreamMode.Write)) + using (ICryptoTransform decryptor = des.CreateDecryptor(key, iv)) + using (CryptoStream cryptoStream = new CryptoStream(output, decryptor, CryptoStreamMode.Write)) { cryptoStream.Write(cipherBytes, 0, cipherBytes.Length); cryptoStream.FlushFinalBlock(); diff --git a/src/libraries/Common/tests/System/Security/Cryptography/AlgorithmImplementations/ECDiffieHellman/ECDiffieHellmanTests.ImportExport.cs b/src/libraries/Common/tests/System/Security/Cryptography/AlgorithmImplementations/ECDiffieHellman/ECDiffieHellmanTests.ImportExport.cs index 7ee472cf247f2..0c06a3f12f151 100644 --- a/src/libraries/Common/tests/System/Security/Cryptography/AlgorithmImplementations/ECDiffieHellman/ECDiffieHellmanTests.ImportExport.cs +++ b/src/libraries/Common/tests/System/Security/Cryptography/AlgorithmImplementations/ECDiffieHellman/ECDiffieHellmanTests.ImportExport.cs @@ -47,8 +47,11 @@ public static void TestNamedCurvesNegative(CurveDef curveDef) return; // An exception may be thrown during Create() if the Oid is bad, or later during native calls - Assert.Throws( - () => ECDiffieHellmanFactory.Create(curveDef.Curve).ExportParameters(false)); + Assert.Throws(() => + { + using ECDiffieHellman ecdh = ECDiffieHellmanFactory.Create(curveDef.Curve); + ecdh.ExportParameters(false); + }); } [Theory, MemberData(nameof(TestCurvesFull))] diff --git a/src/libraries/Common/tests/System/Security/Cryptography/AlgorithmImplementations/ECDsa/ECDsaImportExport.cs b/src/libraries/Common/tests/System/Security/Cryptography/AlgorithmImplementations/ECDsa/ECDsaImportExport.cs index adb9732c1477a..bff75c2092293 100644 --- a/src/libraries/Common/tests/System/Security/Cryptography/AlgorithmImplementations/ECDsa/ECDsaImportExport.cs +++ b/src/libraries/Common/tests/System/Security/Cryptography/AlgorithmImplementations/ECDsa/ECDsaImportExport.cs @@ -4,6 +4,7 @@ using Xunit; using System.Security.Cryptography.Tests; using Test.Cryptography; +using System.Security.Cryptography.EcDiffieHellman.Tests; namespace System.Security.Cryptography.EcDsa.Tests { @@ -88,7 +89,11 @@ public static void TestNamedCurvesNegative(CurveDef curveDef) return; // An exception may be thrown during Create() if the Oid is bad, or later during native calls - Assert.Throws(() => ECDsaFactory.Create(curveDef.Curve).ExportParameters(false)); + Assert.Throws(() => + { + using ECDiffieHellman ecdh = ECDiffieHellmanFactory.Create(curveDef.Curve); + ecdh.ExportParameters(false); + }); } [ConditionalTheory(nameof(ECExplicitCurvesSupported)), MemberData(nameof(TestCurvesFull))] diff --git a/src/libraries/Common/tests/System/Security/Cryptography/AlgorithmImplementations/ECDsa/ECDsaTests.cs b/src/libraries/Common/tests/System/Security/Cryptography/AlgorithmImplementations/ECDsa/ECDsaTests.cs index d2b0c1a5ea49f..0b333ef0cfb35 100644 --- a/src/libraries/Common/tests/System/Security/Cryptography/AlgorithmImplementations/ECDsa/ECDsaTests.cs +++ b/src/libraries/Common/tests/System/Security/Cryptography/AlgorithmImplementations/ECDsa/ECDsaTests.cs @@ -280,18 +280,12 @@ public void SignVerify_InteroperableSameKeys_RoundTripsUnlessTampered(ECDsa ecds byte[] dataArray2 = new byte[dataArray.Length + 2]; dataArray.CopyTo(dataArray2, 1); - HashAlgorithm halg; - if (hashAlgorithm == HashAlgorithmName.MD5) - halg = MD5.Create(); - else if (hashAlgorithm == HashAlgorithmName.SHA1) - halg = SHA1.Create(); - else if (hashAlgorithm == HashAlgorithmName.SHA256) - halg = SHA256.Create(); - else if (hashAlgorithm == HashAlgorithmName.SHA384) - halg = SHA384.Create(); - else if (hashAlgorithm == HashAlgorithmName.SHA512) - halg = SHA512.Create(); - else + using HashAlgorithm halg = + hashAlgorithm == HashAlgorithmName.MD5 ? MD5.Create() : + hashAlgorithm == HashAlgorithmName.SHA1 ? SHA1.Create() : + hashAlgorithm == HashAlgorithmName.SHA256 ? SHA256.Create() : + hashAlgorithm == HashAlgorithmName.SHA384 ? SHA384.Create() : + hashAlgorithm == HashAlgorithmName.SHA512 ? SHA512.Create() : throw new Exception("Hash algorithm not supported."); List signatures = new List(6); diff --git a/src/libraries/Common/tests/System/Security/Cryptography/AlgorithmImplementations/TripleDES/TripleDESCipherTests.cs b/src/libraries/Common/tests/System/Security/Cryptography/AlgorithmImplementations/TripleDES/TripleDESCipherTests.cs index fc627effbc78d..77ac6d65f6b14 100644 --- a/src/libraries/Common/tests/System/Security/Cryptography/AlgorithmImplementations/TripleDES/TripleDESCipherTests.cs +++ b/src/libraries/Common/tests/System/Security/Cryptography/AlgorithmImplementations/TripleDES/TripleDESCipherTests.cs @@ -460,7 +460,8 @@ public static void VerifyKnownTransform_CFB64_NoPadding_9() private static byte[] TripleDESEncryptDirectKey(TripleDES tdes, byte[] key, byte[] iv, byte[] plainBytes) { using (MemoryStream output = new MemoryStream()) - using (CryptoStream cryptoStream = new CryptoStream(output, tdes.CreateEncryptor(key, iv), CryptoStreamMode.Write)) + using (ICryptoTransform encryptor = tdes.CreateEncryptor(key, iv)) + using (CryptoStream cryptoStream = new CryptoStream(output, encryptor, CryptoStreamMode.Write)) { cryptoStream.Write(plainBytes, 0, plainBytes.Length); cryptoStream.FlushFinalBlock(); @@ -472,7 +473,8 @@ private static byte[] TripleDESEncryptDirectKey(TripleDES tdes, byte[] key, byte private static byte[] TripleDESDecryptDirectKey(TripleDES tdes, byte[] key, byte[] iv, byte[] cipherBytes) { using (MemoryStream output = new MemoryStream()) - using (CryptoStream cryptoStream = new CryptoStream(output, tdes.CreateDecryptor(key, iv), CryptoStreamMode.Write)) + using (ICryptoTransform decryptor = tdes.CreateDecryptor(key, iv)) + using (CryptoStream cryptoStream = new CryptoStream(output, decryptor, CryptoStreamMode.Write)) { cryptoStream.Write(cipherBytes, 0, cipherBytes.Length); cryptoStream.FlushFinalBlock(); diff --git a/src/libraries/Common/tests/TestUtilities/System/Buffers/BoundedMemory.Windows.cs b/src/libraries/Common/tests/TestUtilities/System/Buffers/BoundedMemory.Windows.cs index 3b2d8d7dd747b..093f84353ce16 100644 --- a/src/libraries/Common/tests/TestUtilities/System/Buffers/BoundedMemory.Windows.cs +++ b/src/libraries/Common/tests/TestUtilities/System/Buffers/BoundedMemory.Windows.cs @@ -41,7 +41,9 @@ private static WindowsImplementation AllocateWithoutDataPopulationWindows( if (handle == null || handle.IsInvalid) { - Marshal.ThrowExceptionForHR(Marshal.GetHRForLastWin32Error()); + int lastError = Marshal.GetHRForLastWin32Error(); + handle?.Dispose(); + Marshal.ThrowExceptionForHR(lastError); throw new InvalidOperationException("VirtualAlloc failed unexpectedly."); } diff --git a/src/libraries/Microsoft.Win32.Registry/src/Microsoft/Win32/RegistryKey.Windows.cs b/src/libraries/Microsoft.Win32.Registry/src/Microsoft/Win32/RegistryKey.Windows.cs index 487d453868611..b4273a6092dc4 100644 --- a/src/libraries/Microsoft.Win32.Registry/src/Microsoft/Win32/RegistryKey.Windows.cs +++ b/src/libraries/Microsoft.Win32.Registry/src/Microsoft/Win32/RegistryKey.Windows.cs @@ -350,15 +350,13 @@ private SafeRegistryHandle SystemKeyHandle GetRegistryKeyAccess(IsWritable()) | (int)_regView, out SafeRegistryHandle result); - if (ret == 0 && !result.IsInvalid) - { - return result; - } - else + if (ret != 0 || result.IsInvalid) { + result.Dispose(); Win32Error(ret, null); - throw new IOException(Interop.Kernel32.GetMessage(ret), ret); } + + return result; } } diff --git a/src/libraries/System.Console/src/System/TermInfo.cs b/src/libraries/System.Console/src/System/TermInfo.cs index 8f4d31c60a1f1..d44e99afa1c24 100644 --- a/src/libraries/System.Console/src/System/TermInfo.cs +++ b/src/libraries/System.Console/src/System/TermInfo.cs @@ -227,6 +227,7 @@ private static bool TryOpen(string filePath, [NotNullWhen(true)] out SafeFileHan if (fd.IsInvalid) { // Don't throw in this case, as we'll be polling multiple locations looking for the file. + fd.Dispose(); fd = null; return false; } diff --git a/src/libraries/System.Diagnostics.EventLog/src/System/Diagnostics/EventLogInternal.cs b/src/libraries/System.Diagnostics.EventLog/src/System/Diagnostics/EventLogInternal.cs index fe59456017fe0..8c998fba98162 100644 --- a/src/libraries/System.Diagnostics.EventLog/src/System/Diagnostics/EventLogInternal.cs +++ b/src/libraries/System.Diagnostics.EventLog/src/System/Diagnostics/EventLogInternal.cs @@ -1066,6 +1066,7 @@ private void OpenForRead(string currentMachineName) { e = new Win32Exception(); } + handle.Dispose(); throw new InvalidOperationException(SR.Format(SR.CantOpenLog, logname, currentMachineName, e?.Message ?? "")); } @@ -1089,6 +1090,7 @@ private void OpenForWrite(string currentMachineName) { e = new Win32Exception(); } + handle.Dispose(); throw new InvalidOperationException(SR.Format(SR.CantOpenLogAccess, sourceName), e); } writeHandle = handle; diff --git a/src/libraries/System.Diagnostics.EventLog/src/System/Diagnostics/Reader/NativeWrapper.cs b/src/libraries/System.Diagnostics.EventLog/src/System/Diagnostics/Reader/NativeWrapper.cs index ff5470242c7c7..e58e0017ada57 100644 --- a/src/libraries/System.Diagnostics.EventLog/src/System/Diagnostics/Reader/NativeWrapper.cs +++ b/src/libraries/System.Diagnostics.EventLog/src/System/Diagnostics/Reader/NativeWrapper.cs @@ -58,7 +58,11 @@ public static EventLogHandle EvtQuery( EventLogHandle handle = UnsafeNativeMethods.EvtQuery(session, path, query, flags); int win32Error = Marshal.GetLastWin32Error(); if (handle.IsInvalid) + { + handle.Dispose(); EventLogException.Throw(win32Error); + } + return handle; } @@ -122,7 +126,11 @@ public static EventLogHandle EvtOpenProviderMetadata( int win32Error = Marshal.GetLastWin32Error(); if (handle.IsInvalid) + { + handle.Dispose(); EventLogException.Throw(win32Error); + } + return handle; } @@ -141,7 +149,11 @@ public static EventLogHandle EvtOpenEventMetadataEnum(EventLogHandle ProviderMet EventLogHandle emEnumHandle = UnsafeNativeMethods.EvtOpenEventMetadataEnum(ProviderMetadata, flags); int win32Error = Marshal.GetLastWin32Error(); if (emEnumHandle.IsInvalid) + { + emEnumHandle.Dispose(); EventLogException.Throw(win32Error); + } + return emEnumHandle; } @@ -153,6 +165,7 @@ public static EventLogHandle EvtNextEventMetadata(EventLogHandle eventMetadataEn if (emHandle.IsInvalid) { + emHandle.Dispose(); if (win32Error != Interop.Errors.ERROR_NO_MORE_ITEMS) EventLogException.Throw(win32Error); return null; @@ -166,7 +179,11 @@ public static EventLogHandle EvtOpenChannelEnum(EventLogHandle session, int flag EventLogHandle channelEnum = UnsafeNativeMethods.EvtOpenChannelEnum(session, flags); int win32Error = Marshal.GetLastWin32Error(); if (channelEnum.IsInvalid) + { + channelEnum.Dispose(); EventLogException.Throw(win32Error); + } + return channelEnum; } @@ -175,7 +192,11 @@ public static EventLogHandle EvtOpenProviderEnum(EventLogHandle session, int fla EventLogHandle pubEnum = UnsafeNativeMethods.EvtOpenPublisherEnum(session, flags); int win32Error = Marshal.GetLastWin32Error(); if (pubEnum.IsInvalid) + { + pubEnum.Dispose(); EventLogException.Throw(win32Error); + } + return pubEnum; } @@ -184,7 +205,11 @@ public static EventLogHandle EvtOpenChannelConfig(EventLogHandle session, string EventLogHandle handle = UnsafeNativeMethods.EvtOpenChannelConfig(session, channelPath, flags); int win32Error = Marshal.GetLastWin32Error(); if (handle.IsInvalid) + { + handle.Dispose(); EventLogException.Throw(win32Error); + } + return handle; } @@ -201,7 +226,11 @@ public static EventLogHandle EvtOpenLog(EventLogHandle session, string path, Pat EventLogHandle logHandle = UnsafeNativeMethods.EvtOpenLog(session, path, flags); int win32Error = Marshal.GetLastWin32Error(); if (logHandle.IsInvalid) + { + logHandle.Dispose(); EventLogException.Throw(win32Error); + } + return logHandle; } @@ -253,7 +282,11 @@ public static EventLogHandle EvtCreateRenderContext( EventLogHandle renderContextHandleValues = UnsafeNativeMethods.EvtCreateRenderContext(valuePathsCount, valuePaths, flags); int win32Error = Marshal.GetLastWin32Error(); if (renderContextHandleValues.IsInvalid) + { + renderContextHandleValues.Dispose(); EventLogException.Throw(win32Error); + } + return renderContextHandleValues; } @@ -302,7 +335,11 @@ public static EventLogHandle EvtCreateBookmark(string bookmarkXml) EventLogHandle handle = UnsafeNativeMethods.EvtCreateBookmark(bookmarkXml); int win32Error = Marshal.GetLastWin32Error(); if (handle.IsInvalid) + { + handle.Dispose(); EventLogException.Throw(win32Error); + } + return handle; } diff --git a/src/libraries/System.Diagnostics.PerformanceCounter/src/System/Diagnostics/PerformanceDataRegistryKey.cs b/src/libraries/System.Diagnostics.PerformanceCounter/src/System/Diagnostics/PerformanceDataRegistryKey.cs index c1ba8ddabc3f2..fd47977c2196b 100644 --- a/src/libraries/System.Diagnostics.PerformanceCounter/src/System/Diagnostics/PerformanceDataRegistryKey.cs +++ b/src/libraries/System.Diagnostics.PerformanceCounter/src/System/Diagnostics/PerformanceDataRegistryKey.cs @@ -26,6 +26,13 @@ public static PerformanceDataRegistryKey OpenRemoteBaseKey(string machineName) // connect to the specified remote registry int ret = Interop.Advapi32.RegConnectRegistry(machineName, new SafeRegistryHandle(new IntPtr(PerformanceData), ownsHandle: false), out SafeRegistryHandle foreignHKey); + if (ret == 0 && !foreignHKey.IsInvalid) + { + return new PerformanceDataRegistryKey(foreignHKey); + } + + foreignHKey.Dispose(); + if (ret == Interop.Errors.ERROR_DLL_INIT_FAILED) { // return value indicates an error occurred @@ -37,13 +44,8 @@ public static PerformanceDataRegistryKey OpenRemoteBaseKey(string machineName) Win32Error(ret, null); } - if (foreignHKey.IsInvalid) - { - // return value indicates an error occurred - throw new ArgumentException(SR.Format(SR.Arg_RegKeyNoRemoteConnect, machineName)); - } - - return new PerformanceDataRegistryKey(foreignHKey); + // return value indicates an error occurred + throw new ArgumentException(SR.Format(SR.Arg_RegKeyNoRemoteConnect, machineName)); } public static PerformanceDataRegistryKey OpenLocal() diff --git a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Win32.cs b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Win32.cs index 4113c31b90de2..5632f1e5d402d 100644 --- a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Win32.cs +++ b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Win32.cs @@ -430,17 +430,14 @@ private bool Equals(Process process) foreach (Process p in GetProcesses()) { SafeProcessHandle h = SafeGetHandle(p); - if (!h.IsInvalid) + if (!h.IsInvalid && predicate(this, p)) { - if (predicate(this, p)) - { - results.Add((p, h)); - } - else - { - p.Dispose(); - h.Dispose(); - } + results.Add((p, h)); + } + else + { + p.Dispose(); + h.Dispose(); } } diff --git a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Windows.cs b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Windows.cs index 214947a72c9b8..a7045f01d9541 100644 --- a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Windows.cs +++ b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Windows.cs @@ -596,6 +596,14 @@ ref processInfo // pointer to PROCESS_INFORMATION throw CreateExceptionForErrorStartingProcess(nativeErrorMessage, errorCode, startInfo.FileName, workingDirectory); } } + catch + { + parentInputPipeHandle?.Dispose(); + parentOutputPipeHandle?.Dispose(); + parentErrorPipeHandle?.Dispose(); + procSH.Dispose(); + throw; + } finally { childInputPipeHandle?.Dispose(); @@ -624,7 +632,10 @@ ref processInfo // pointer to PROCESS_INFORMATION commandLine.Dispose(); if (procSH.IsInvalid) + { + procSH.Dispose(); return false; + } SetProcessHandle(procSH); SetProcessId((int)processInfo.dwProcessId); diff --git a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessManager.Windows.cs b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessManager.Windows.cs index a35913265d9cc..f1b2142e933b5 100644 --- a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessManager.Windows.cs +++ b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessManager.Windows.cs @@ -244,6 +244,8 @@ public static SafeProcessHandle OpenProcess(int processId, int access, bool thro return processHandle; } + processHandle.Dispose(); + if (processId == 0) { throw new Win32Exception(Interop.Errors.ERROR_ACCESS_DENIED); @@ -271,6 +273,7 @@ public static SafeThreadHandle OpenThread(int threadId, int access) int result = Marshal.GetLastWin32Error(); if (threadHandle.IsInvalid) { + threadHandle.Dispose(); if (result == Interop.Errors.ERROR_INVALID_PARAMETER) throw new InvalidOperationException(SR.Format(SR.ThreadExited, threadId.ToString())); throw new Win32Exception(result); diff --git a/src/libraries/System.DirectoryServices.AccountManagement/src/System/DirectoryServices/AccountManagement/AD/SidList.cs b/src/libraries/System.DirectoryServices.AccountManagement/src/System/DirectoryServices/AccountManagement/AD/SidList.cs index 4df7b5397cc39..2aa13c19389e0 100644 --- a/src/libraries/System.DirectoryServices.AccountManagement/src/System/DirectoryServices/AccountManagement/AD/SidList.cs +++ b/src/libraries/System.DirectoryServices.AccountManagement/src/System/DirectoryServices/AccountManagement/AD/SidList.cs @@ -94,6 +94,8 @@ private void TranslateSids(string target, IntPtr[] pSids) out policyHandle); if (err != 0) { + policyHandle.Dispose(); + GlobalDebug.WriteLineIf(GlobalDebug.Warn, "AuthZSet", "SidList: couldn't get policy handle, err={0}", err); throw new PrincipalOperationException(SR.Format( @@ -119,6 +121,9 @@ private void TranslateSids(string target, IntPtr[] pSids) err != Interop.StatusOptions.STATUS_SOME_NOT_MAPPED && err != Interop.StatusOptions.STATUS_NONE_MAPPED) { + domainsHandle.Dispose(); + namesHandle.Dispose(); + GlobalDebug.WriteLineIf(GlobalDebug.Warn, "AuthZSet", "SidList: LsaLookupSids failed, err={0}", err); throw new PrincipalOperationException(SR.Format( diff --git a/src/libraries/System.DirectoryServices.AccountManagement/src/System/DirectoryServices/AccountManagement/AuthZSet.cs b/src/libraries/System.DirectoryServices.AccountManagement/src/System/DirectoryServices/AccountManagement/AuthZSet.cs index b69798ae0b6d7..442d3ef975682 100644 --- a/src/libraries/System.DirectoryServices.AccountManagement/src/System/DirectoryServices/AccountManagement/AuthZSet.cs +++ b/src/libraries/System.DirectoryServices.AccountManagement/src/System/DirectoryServices/AccountManagement/AuthZSet.cs @@ -192,14 +192,9 @@ out pClientContext { GlobalDebug.WriteLineIf(GlobalDebug.Error, "AuthZSet", "Caught exception {0} with message {1}", e.GetType(), e.Message); - if (_psBuffer != null && !_psBuffer.IsInvalid) - _psBuffer.Close(); - - if (_psUserSid != null && !_psUserSid.IsInvalid) - _psUserSid.Close(); - - if (_psMachineSid != null && !_psMachineSid.IsInvalid) - _psMachineSid.Close(); + _psBuffer?.Dispose(); + _psUserSid?.Dispose(); + _psMachineSid?.Dispose(); // We're on a platform that doesn't have the AuthZ library if (e is DllNotFoundException) diff --git a/src/libraries/System.DirectoryServices.AccountManagement/src/System/DirectoryServices/AccountManagement/Utils.cs b/src/libraries/System.DirectoryServices.AccountManagement/src/System/DirectoryServices/AccountManagement/Utils.cs index 12cfa84157004..286ec3cd3029f 100644 --- a/src/libraries/System.DirectoryServices.AccountManagement/src/System/DirectoryServices/AccountManagement/Utils.cs +++ b/src/libraries/System.DirectoryServices.AccountManagement/src/System/DirectoryServices/AccountManagement/Utils.cs @@ -357,6 +357,7 @@ out tokenHandle if ((error = Marshal.GetLastWin32Error()) == 1008) // ERROR_NO_TOKEN { Debug.Assert(tokenHandle.IsInvalid); + tokenHandle.Dispose(); // Current thread doesn't have a token, try the process if (!Interop.Advapi32.OpenProcessToken( diff --git a/src/libraries/System.DirectoryServices/src/System/DirectoryServices/ActiveDirectory/DirectoryContext.cs b/src/libraries/System.DirectoryServices/src/System/DirectoryServices/ActiveDirectory/DirectoryContext.cs index 021afb3e35a15..f03a4cb88e4b1 100644 --- a/src/libraries/System.DirectoryServices/src/System/DirectoryServices/ActiveDirectory/DirectoryContext.cs +++ b/src/libraries/System.DirectoryServices/src/System/DirectoryServices/ActiveDirectory/DirectoryContext.cs @@ -605,7 +605,7 @@ internal static string GetLoggedOnDomain() // If this is a directory user, extract domain info from username if (!Utils.IsSamUser()) { - WindowsIdentity identity = WindowsIdentity.GetCurrent(); + using WindowsIdentity identity = WindowsIdentity.GetCurrent(); int index = identity.Name.IndexOf('\\'); Debug.Assert(index != -1); diff --git a/src/libraries/System.DirectoryServices/src/System/DirectoryServices/ActiveDirectory/Utils.cs b/src/libraries/System.DirectoryServices/src/System/DirectoryServices/ActiveDirectory/Utils.cs index 4769dc9dd1b79..14a3988378c6e 100644 --- a/src/libraries/System.DirectoryServices/src/System/DirectoryServices/ActiveDirectory/Utils.cs +++ b/src/libraries/System.DirectoryServices/src/System/DirectoryServices/ActiveDirectory/Utils.cs @@ -2082,6 +2082,7 @@ out tokenHandle if ((error = Marshal.GetLastWin32Error()) == 1008) // ERROR_NO_TOKEN { Debug.Assert(tokenHandle.IsInvalid); + tokenHandle.Dispose(); // Current thread doesn't have a token, try the process if (!global::Interop.Advapi32.OpenProcessToken( diff --git a/src/libraries/System.IO.Compression/src/System/IO/Compression/DeflateZLib/Deflater.cs b/src/libraries/System.IO.Compression/src/System/IO/Compression/DeflateZLib/Deflater.cs index f2d02cde7296b..8ceb38f13eb8c 100644 --- a/src/libraries/System.IO.Compression/src/System/IO/Compression/DeflateZLib/Deflater.cs +++ b/src/libraries/System.IO.Compression/src/System/IO/Compression/DeflateZLib/Deflater.cs @@ -64,17 +64,21 @@ internal Deflater(CompressionLevel compressionLevel, int windowBits) ZLibNative.CompressionStrategy strategy = ZLibNative.CompressionStrategy.DefaultStrategy; + ZLibNative.ZLibStreamHandle? zlibStream = null; ZErrorCode errC; try { - errC = ZLibNative.CreateZLibStreamForDeflate(out _zlibStream, zlibCompressionLevel, + errC = ZLibNative.CreateZLibStreamForDeflate(out zlibStream, zlibCompressionLevel, windowBits, memLevel, strategy); } catch (Exception cause) { + zlibStream?.Dispose(); throw new ZLibException(SR.ZLibErrorDLLLoadError, cause); } + _zlibStream = zlibStream; + switch (errC) { case ZErrorCode.Ok: diff --git a/src/libraries/System.IO.Compression/src/System/IO/Compression/DeflateZLib/Inflater.cs b/src/libraries/System.IO.Compression/src/System/IO/Compression/DeflateZLib/Inflater.cs index 3028ce561ea98..ecc997f5dc8cc 100644 --- a/src/libraries/System.IO.Compression/src/System/IO/Compression/DeflateZLib/Inflater.cs +++ b/src/libraries/System.IO.Compression/src/System/IO/Compression/DeflateZLib/Inflater.cs @@ -239,16 +239,20 @@ public void Dispose() [MemberNotNull(nameof(_zlibStream))] private void InflateInit(int windowBits) { + ZLibNative.ZLibStreamHandle? zlibStream = null; ZLibNative.ErrorCode error; try { - error = ZLibNative.CreateZLibStreamForInflate(out _zlibStream, windowBits); + error = ZLibNative.CreateZLibStreamForInflate(out zlibStream, windowBits); } catch (Exception exception) // could not load the ZLib dll { + zlibStream?.Dispose(); throw new ZLibException(SR.ZLibErrorDLLLoadError, exception); } + _zlibStream = zlibStream; + switch (error) { case ZLibNative.ErrorCode.Ok: // Successful initialization diff --git a/src/libraries/System.IO.FileSystem.Watcher/src/System/IO/FileSystemWatcher.Linux.cs b/src/libraries/System.IO.FileSystem.Watcher/src/System/IO/FileSystemWatcher.Linux.cs index 718bdd8608b2d..6439e2496ad61 100644 --- a/src/libraries/System.IO.FileSystem.Watcher/src/System/IO/FileSystemWatcher.Linux.cs +++ b/src/libraries/System.IO.FileSystem.Watcher/src/System/IO/FileSystemWatcher.Linux.cs @@ -36,6 +36,7 @@ private void StartRaisingEvents() if (handle.IsInvalid) { Interop.ErrorInfo error = Interop.Sys.GetLastErrorInfo(); + handle.Dispose(); switch (error.Error) { case Interop.Error.EMFILE: diff --git a/src/libraries/System.IO.FileSystem.Watcher/src/System/IO/FileSystemWatcher.OSX.cs b/src/libraries/System.IO.FileSystem.Watcher/src/System/IO/FileSystemWatcher.OSX.cs index 7938fe9b92b89..ba8cc8f4ee1ec 100644 --- a/src/libraries/System.IO.FileSystem.Watcher/src/System/IO/FileSystemWatcher.OSX.cs +++ b/src/libraries/System.IO.FileSystem.Watcher/src/System/IO/FileSystemWatcher.OSX.cs @@ -324,7 +324,9 @@ internal unsafe void Start(CancellationToken cancellationToken) EventStreamFlags); if (eventStream.IsInvalid) { - throw Interop.GetExceptionForIoErrno(Interop.Sys.GetLastErrorInfo(), _fullDirectory, true); + Exception e = Interop.GetExceptionForIoErrno(Interop.Sys.GetLastErrorInfo(), _fullDirectory, true); + eventStream.Dispose(); + throw e; } cleanupGCHandle = false; diff --git a/src/libraries/System.IO.FileSystem/tests/Directory/Delete.Windows.cs b/src/libraries/System.IO.FileSystem/tests/Directory/Delete.Windows.cs index e42969aeaacb8..afe712f9fbab9 100644 --- a/src/libraries/System.IO.FileSystem/tests/Directory/Delete.Windows.cs +++ b/src/libraries/System.IO.FileSystem/tests/Directory/Delete.Windows.cs @@ -15,10 +15,12 @@ public partial class Directory_Delete_str_bool : Directory_Delete_str [PlatformSpecific(TestPlatforms.Windows)] public void RecursiveDelete_NoListDirectoryPermission() // https://github.com/dotnet/runtime/issues/56922 { + using WindowsIdentity currentIdentity = WindowsIdentity.GetCurrent(); + string parentPath = GetTestFilePath(); var parent = Directory.CreateDirectory(parentPath); var ac = parent.GetAccessControl(); - var rule = new FileSystemAccessRule(WindowsIdentity.GetCurrent().User, FileSystemRights.ListDirectory, AccessControlType.Deny); + var rule = new FileSystemAccessRule(currentIdentity.User, FileSystemRights.ListDirectory, AccessControlType.Deny); ac.SetAccessRule(rule); parent.SetAccessControl(ac); diff --git a/src/libraries/System.IO.FileSystem/tests/FSAssert.cs b/src/libraries/System.IO.FileSystem/tests/FSAssert.cs index a3f33f5e432dc..af803b5bc5ea5 100644 --- a/src/libraries/System.IO.FileSystem/tests/FSAssert.cs +++ b/src/libraries/System.IO.FileSystem/tests/FSAssert.cs @@ -37,7 +37,8 @@ public static void CompletesSynchronously(Task task) else { // first wait for the task to complete without throwing - ((IAsyncResult)task).AsyncWaitHandle.WaitOne(); + using WaitHandle wh = ((IAsyncResult)task).AsyncWaitHandle; + wh.WaitOne(); // now assert, we ignore the result of the task intentionally, // As it previously did not complete synchronously. diff --git a/src/libraries/System.IO.FileSystem/tests/File/OpenHandle.cs b/src/libraries/System.IO.FileSystem/tests/File/OpenHandle.cs index 120ed9aec222e..c89097197d2d8 100644 --- a/src/libraries/System.IO.FileSystem/tests/File/OpenHandle.cs +++ b/src/libraries/System.IO.FileSystem/tests/File/OpenHandle.cs @@ -21,7 +21,18 @@ protected override FileStream CreateFileStream(string path, FileMode mode, FileA => new FileStream(File.OpenHandle(path, mode, access), access); protected override FileStream CreateFileStream(string path, FileMode mode, FileAccess access, FileShare share, int bufferSize, FileOptions options) - => new FileStream(File.OpenHandle(path, mode, access, share, options), access, bufferSize, (options & FileOptions.Asynchronous) != 0); + { + SafeFileHandle handle = File.OpenHandle(path, mode, access, share, options); + try + { + return new FileStream(handle, access, bufferSize, (options & FileOptions.Asynchronous) != 0); + } + catch + { + handle.Dispose(); + throw; + } + } protected override FileStream CreateFileStream(string path, FileMode mode, FileAccess access, FileShare share, int bufferSize, FileOptions options, long preallocationSize) => new FileStream(File.OpenHandle(path, mode, access, share, options, preallocationSize), access, bufferSize, (options & FileOptions.Asynchronous) != 0); diff --git a/src/libraries/System.IO.MemoryMappedFiles/src/System/IO/MemoryMappedFiles/MemoryMappedFile.Windows.cs b/src/libraries/System.IO.MemoryMappedFiles/src/System/IO/MemoryMappedFiles/MemoryMappedFile.Windows.cs index 37e780f8f6977..1691079a83a1c 100644 --- a/src/libraries/System.IO.MemoryMappedFiles/src/System/IO/MemoryMappedFiles/MemoryMappedFile.Windows.cs +++ b/src/libraries/System.IO.MemoryMappedFiles/src/System/IO/MemoryMappedFiles/MemoryMappedFile.Windows.cs @@ -169,6 +169,7 @@ private static SafeMemoryMappedFileHandle CreateOrOpenCore( // finished retrying but couldn't create or open if (handle == null || handle.IsInvalid) { + handle?.Dispose(); throw new InvalidOperationException(SR.InvalidOperation_CantCreateFileMapping); } diff --git a/src/libraries/System.IO.Pipes/src/System/IO/Pipes/AnonymousPipeClientStream.cs b/src/libraries/System.IO.Pipes/src/System/IO/Pipes/AnonymousPipeClientStream.cs index b504db433bcb0..f21cb313eae93 100644 --- a/src/libraries/System.IO.Pipes/src/System/IO/Pipes/AnonymousPipeClientStream.cs +++ b/src/libraries/System.IO.Pipes/src/System/IO/Pipes/AnonymousPipeClientStream.cs @@ -37,6 +37,7 @@ public AnonymousPipeClientStream(PipeDirection direction, string pipeHandleAsStr SafePipeHandle safePipeHandle = new SafePipeHandle((IntPtr)result, true); if (safePipeHandle.IsInvalid) { + safePipeHandle.Dispose(); throw new ArgumentException(SR.Argument_InvalidHandle, nameof(pipeHandleAsString)); } diff --git a/src/libraries/System.IO.Pipes/src/System/IO/Pipes/AnonymousPipeServerStream.Unix.cs b/src/libraries/System.IO.Pipes/src/System/IO/Pipes/AnonymousPipeServerStream.Unix.cs index d144a12a0ed1a..4d8a604c1960d 100644 --- a/src/libraries/System.IO.Pipes/src/System/IO/Pipes/AnonymousPipeServerStream.Unix.cs +++ b/src/libraries/System.IO.Pipes/src/System/IO/Pipes/AnonymousPipeServerStream.Unix.cs @@ -17,14 +17,23 @@ private void Create(PipeDirection direction, HandleInheritability inheritability Debug.Assert(direction != PipeDirection.InOut, "Anonymous pipe direction shouldn't be InOut"); // Ignore bufferSize. It's optional, and the fcntl F_SETPIPE_SZ for changing it is Linux specific. - SafePipeHandle serverHandle, clientHandle; - if (direction == PipeDirection.In) + SafePipeHandle? serverHandle = null, clientHandle = null; + try { - CreateAnonymousPipe(reader: out serverHandle, writer: out clientHandle); + if (direction == PipeDirection.In) + { + CreateAnonymousPipe(reader: out serverHandle, writer: out clientHandle); + } + else + { + CreateAnonymousPipe(reader: out clientHandle, writer: out serverHandle); + } } - else + catch { - CreateAnonymousPipe(reader: out clientHandle, writer: out serverHandle); + serverHandle?.Dispose(); + clientHandle?.Dispose(); + throw; } // We always create pipes with both file descriptors being O_CLOEXEC. diff --git a/src/libraries/System.IO.Pipes/src/System/IO/Pipes/AnonymousPipeServerStream.Windows.cs b/src/libraries/System.IO.Pipes/src/System/IO/Pipes/AnonymousPipeServerStream.Windows.cs index 8ae44a14cbcdd..59859f0dde833 100644 --- a/src/libraries/System.IO.Pipes/src/System/IO/Pipes/AnonymousPipeServerStream.Windows.cs +++ b/src/libraries/System.IO.Pipes/src/System/IO/Pipes/AnonymousPipeServerStream.Windows.cs @@ -39,7 +39,7 @@ private void Create(PipeDirection direction, HandleInheritability inheritability Debug.Assert(bufferSize >= 0, "bufferSize is negative"); bool bSuccess; - SafePipeHandle serverHandle; + SafePipeHandle serverHandle, clientHandle; SafePipeHandle newServerHandle; // Create the two pipe handles that make up the anonymous pipe. @@ -50,11 +50,11 @@ private void Create(PipeDirection direction, HandleInheritability inheritability if (direction == PipeDirection.In) { - bSuccess = Interop.Kernel32.CreatePipe(out serverHandle, out _clientHandle, ref secAttrs, bufferSize); + bSuccess = Interop.Kernel32.CreatePipe(out serverHandle, out clientHandle, ref secAttrs, bufferSize); } else { - bSuccess = Interop.Kernel32.CreatePipe(out _clientHandle, out serverHandle, ref secAttrs, bufferSize); + bSuccess = Interop.Kernel32.CreatePipe(out clientHandle, out serverHandle, ref secAttrs, bufferSize); } } finally @@ -67,6 +67,8 @@ private void Create(PipeDirection direction, HandleInheritability inheritability if (!bSuccess) { + serverHandle.Dispose(); + clientHandle.Dispose(); throw Win32Marshal.GetExceptionForLastWin32Error(); } @@ -79,12 +81,19 @@ private void Create(PipeDirection direction, HandleInheritability inheritability if (!bSuccess) { - throw Win32Marshal.GetExceptionForLastWin32Error(); + Exception e = Win32Marshal.GetExceptionForLastWin32Error(); + + serverHandle.Dispose(); + newServerHandle.Dispose(); + clientHandle.Dispose(); + + throw e; } // Close the inheritable server handle. serverHandle.Dispose(); + _clientHandle = clientHandle; InitializeHandle(newServerHandle, false, false); State = PipeState.Connected; diff --git a/src/libraries/System.IO.Pipes/src/System/IO/Pipes/NamedPipeClientStream.Windows.cs b/src/libraries/System.IO.Pipes/src/System/IO/Pipes/NamedPipeClientStream.Windows.cs index 2de8366900293..0cefc507a7b86 100644 --- a/src/libraries/System.IO.Pipes/src/System/IO/Pipes/NamedPipeClientStream.Windows.cs +++ b/src/libraries/System.IO.Pipes/src/System/IO/Pipes/NamedPipeClientStream.Windows.cs @@ -50,6 +50,8 @@ private bool TryConnect(int timeout, CancellationToken cancellationToken) { int errorCode = Marshal.GetLastPInvokeError(); + handle.Dispose(); + // CreateFileW: "If the CreateNamedPipe function was not successfully called on the server prior to this operation, // a pipe will not exist and CreateFile will fail with ERROR_FILE_NOT_FOUND" // WaitNamedPipeW: "If no instances of the specified named pipe exist, @@ -85,6 +87,8 @@ private bool TryConnect(int timeout, CancellationToken cancellationToken) { errorCode = Marshal.GetLastPInvokeError(); + handle.Dispose(); + // WaitNamedPipe: "A subsequent CreateFile call to the pipe can fail, // because the instance was closed by the server or opened by another client." if (errorCode == Interop.Errors.ERROR_PIPE_BUSY || // opened by another client diff --git a/src/libraries/System.IO.Pipes/src/System/IO/Pipes/NamedPipeServerStream.Windows.cs b/src/libraries/System.IO.Pipes/src/System/IO/Pipes/NamedPipeServerStream.Windows.cs index 625d2e322f236..7ec4f97e1f30e 100644 --- a/src/libraries/System.IO.Pipes/src/System/IO/Pipes/NamedPipeServerStream.Windows.cs +++ b/src/libraries/System.IO.Pipes/src/System/IO/Pipes/NamedPipeServerStream.Windows.cs @@ -141,6 +141,7 @@ private void Create(string pipeName, PipeDirection direction, int maxNumberOfSer if (handle.IsInvalid) { + handle.Dispose(); throw Win32Marshal.GetExceptionForLastWin32Error(); } diff --git a/src/libraries/System.IO.Pipes/tests/AnonymousPipeTests/AnonymousPipeTest.CreateClient.cs b/src/libraries/System.IO.Pipes/tests/AnonymousPipeTests/AnonymousPipeTest.CreateClient.cs index a7ac7215517ec..0d1abcc10acc1 100644 --- a/src/libraries/System.IO.Pipes/tests/AnonymousPipeTests/AnonymousPipeTest.CreateClient.cs +++ b/src/libraries/System.IO.Pipes/tests/AnonymousPipeTests/AnonymousPipeTest.CreateClient.cs @@ -39,7 +39,7 @@ public static void InvalidStringParameters_Throws_ArgumentException(string handl [Fact] public static void InvalidPipeHandle_Throws_ArgumentException() { - SafePipeHandle pipeHandle = new SafePipeHandle(new IntPtr(-1), true); + using SafePipeHandle pipeHandle = new SafePipeHandle(new IntPtr(-1), true); AssertExtensions.Throws("safePipeHandle", () => new AnonymousPipeClientStream(PipeDirection.In, pipeHandle)); } @@ -49,7 +49,7 @@ public static void InOutPipeDirection_Throws_NotSupportedException() // Anonymous pipes can't be made with PipeDirection.InOut Assert.Throws(() => new AnonymousPipeClientStream(PipeDirection.InOut, "123")); - SafePipeHandle pipeHandle = new SafePipeHandle(new IntPtr(-1), true); + using SafePipeHandle pipeHandle = new SafePipeHandle(new IntPtr(-1), true); Assert.Throws(() => new AnonymousPipeClientStream(PipeDirection.InOut, pipeHandle)); } } diff --git a/src/libraries/System.IO.Pipes/tests/AnonymousPipeTests/AnonymousPipeTest.CreateServer.cs b/src/libraries/System.IO.Pipes/tests/AnonymousPipeTests/AnonymousPipeTest.CreateServer.cs index 6dbca6b861b59..976fe5fc571fe 100644 --- a/src/libraries/System.IO.Pipes/tests/AnonymousPipeTests/AnonymousPipeTest.CreateServer.cs +++ b/src/libraries/System.IO.Pipes/tests/AnonymousPipeTests/AnonymousPipeTest.CreateServer.cs @@ -59,12 +59,13 @@ public static void InvalidPipeDirection_Throws_ArgumentOutOfRangeException() public static void InvalidPipeHandle_Throws() { using (AnonymousPipeServerStream dummyserver = new AnonymousPipeServerStream(PipeDirection.Out)) + using (dummyserver.ClientSafePipeHandle) { AssertExtensions.Throws("serverSafePipeHandle", () => new AnonymousPipeServerStream(PipeDirection.Out, null, dummyserver.ClientSafePipeHandle)); AssertExtensions.Throws("clientSafePipeHandle", () => new AnonymousPipeServerStream(PipeDirection.Out, dummyserver.SafePipeHandle, null)); - SafePipeHandle pipeHandle = new SafePipeHandle(new IntPtr(-1), true); + using SafePipeHandle pipeHandle = new SafePipeHandle(new IntPtr(-1), true); AssertExtensions.Throws("serverSafePipeHandle", () => new AnonymousPipeServerStream(PipeDirection.Out, pipeHandle, dummyserver.ClientSafePipeHandle)); AssertExtensions.Throws("clientSafePipeHandle", () => new AnonymousPipeServerStream(PipeDirection.Out, dummyserver.SafePipeHandle, pipeHandle)); diff --git a/src/libraries/System.IO.Pipes/tests/NamedPipeTests/NamedPipeTest.CreateClient.cs b/src/libraries/System.IO.Pipes/tests/NamedPipeTests/NamedPipeTest.CreateClient.cs index 9ebef55d8909e..362ddd80be881 100644 --- a/src/libraries/System.IO.Pipes/tests/NamedPipeTests/NamedPipeTest.CreateClient.cs +++ b/src/libraries/System.IO.Pipes/tests/NamedPipeTests/NamedPipeTest.CreateClient.cs @@ -123,7 +123,7 @@ public static void NullHandle_Throws_ArgumentNullException(PipeDirection directi [InlineData(PipeDirection.Out)] public static void InvalidHandle_Throws_ArgumentException(PipeDirection direction) { - SafePipeHandle pipeHandle = new SafePipeHandle(new IntPtr(-1), true); + using SafePipeHandle pipeHandle = new SafePipeHandle(new IntPtr(-1), true); AssertExtensions.Throws("safePipeHandle", () => new NamedPipeClientStream(direction, false, true, pipeHandle)); } diff --git a/src/libraries/System.IO.Pipes/tests/NamedPipeTests/NamedPipeTest.CreateServer.cs b/src/libraries/System.IO.Pipes/tests/NamedPipeTests/NamedPipeTest.CreateServer.cs index 30c10259dbcf3..412010a0c9953 100644 --- a/src/libraries/System.IO.Pipes/tests/NamedPipeTests/NamedPipeTest.CreateServer.cs +++ b/src/libraries/System.IO.Pipes/tests/NamedPipeTests/NamedPipeTest.CreateServer.cs @@ -160,7 +160,7 @@ public static void NullPipeHandle_Throws_ArgumentNullException(PipeDirection dir [InlineData(PipeDirection.Out)] public static void InvalidPipeHandle_Throws_ArgumentException(PipeDirection direction) { - SafePipeHandle pipeHandle = new SafePipeHandle(new IntPtr(-1), true); + using SafePipeHandle pipeHandle = new SafePipeHandle(new IntPtr(-1), true); AssertExtensions.Throws("safePipeHandle", () => new NamedPipeServerStream(direction, false, true, pipeHandle)); } diff --git a/src/libraries/System.IO.Ports/src/System/IO/Ports/SafeSerialDeviceHandle.Unix.cs b/src/libraries/System.IO.Ports/src/System/IO/Ports/SafeSerialDeviceHandle.Unix.cs index 4d2202b99abb8..025643262b570 100644 --- a/src/libraries/System.IO.Ports/src/System/IO/Ports/SafeSerialDeviceHandle.Unix.cs +++ b/src/libraries/System.IO.Ports/src/System/IO/Ports/SafeSerialDeviceHandle.Unix.cs @@ -23,6 +23,8 @@ internal static SafeSerialDeviceHandle Open(string portName) if (handle.IsInvalid) { + handle.Dispose(); + // exception type is matching Windows throw new UnauthorizedAccessException( SR.Format(SR.UnauthorizedAccess_IODenied_Port, portName), diff --git a/src/libraries/System.IO.Ports/src/System/IO/Ports/SerialStream.Windows.cs b/src/libraries/System.IO.Ports/src/System/IO/Ports/SerialStream.Windows.cs index 27cf00cfcd070..9643235e94cb4 100644 --- a/src/libraries/System.IO.Ports/src/System/IO/Ports/SerialStream.Windows.cs +++ b/src/libraries/System.IO.Ports/src/System/IO/Ports/SerialStream.Windows.cs @@ -578,7 +578,9 @@ internal SerialStream(string portName, int baudRate, Parity parity, int dataBits if (tempHandle.IsInvalid) { - throw Win32Marshal.GetExceptionForLastWin32Error(portName); + Exception e = Win32Marshal.GetExceptionForLastWin32Error(portName); + tempHandle.Dispose(); + throw e; } try diff --git a/src/libraries/System.Net.Http.WinHttpHandler/src/System/Net/Http/WinHttpHandler.cs b/src/libraries/System.Net.Http.WinHttpHandler/src/System/Net/Http/WinHttpHandler.cs index 91ba3aaec1309..2d72c26fbac53 100644 --- a/src/libraries/System.Net.Http.WinHttpHandler/src/System/Net/Http/WinHttpHandler.cs +++ b/src/libraries/System.Net.Http.WinHttpHandler/src/System/Net/Http/WinHttpHandler.cs @@ -820,6 +820,7 @@ private void EnsureSessionHandleExists(WinHttpRequestState state) { int lastError = Marshal.GetLastWin32Error(); if (NetEventSource.Log.IsEnabled()) NetEventSource.Error(this, $"error={lastError}"); + if (lastError != Interop.WinHttp.ERROR_INVALID_PARAMETER) { ThrowOnInvalidHandle(sessionHandle, nameof(Interop.WinHttp.WinHttpOpen)); @@ -1659,6 +1660,9 @@ private void ThrowOnInvalidHandle(SafeWinHttpHandle handle, string nameOfCalledF { int lastError = Marshal.GetLastWin32Error(); if (NetEventSource.Log.IsEnabled()) NetEventSource.Error(this, $"error={lastError}"); + + handle.Dispose(); + throw WinHttpException.CreateExceptionUsingError(lastError, nameOfCalledFunction); } } diff --git a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/HttpWindowsProxy.cs b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/HttpWindowsProxy.cs index bd7d03388c723..3a2c4ee6ff5de 100644 --- a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/HttpWindowsProxy.cs +++ b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/HttpWindowsProxy.cs @@ -55,6 +55,7 @@ public static bool TryCreate([NotNullWhen(true)] out IWebProxy? proxy) { // Proxy failures are currently ignored by managed handler. if (NetEventSource.Log.IsEnabled()) NetEventSource.Error(proxyHelper, $"{nameof(Interop.WinHttp.WinHttpOpen)} returned invalid handle"); + sessionHandle.Dispose(); return false; } } diff --git a/src/libraries/System.Net.Http/tests/FunctionalTests/ImpersonatedAuthTests.cs b/src/libraries/System.Net.Http/tests/FunctionalTests/ImpersonatedAuthTests.cs index 32f44d5e00f70..8287046984924 100644 --- a/src/libraries/System.Net.Http/tests/FunctionalTests/ImpersonatedAuthTests.cs +++ b/src/libraries/System.Net.Http/tests/FunctionalTests/ImpersonatedAuthTests.cs @@ -50,13 +50,19 @@ await LoopbackServer.CreateClientAndServerAsync( string initialUser = response.Headers.GetValues(NtAuthTests.UserHeaderName).First(); - _output.WriteLine($"Starting test as {WindowsIdentity.GetCurrent().Name}"); + using (WindowsIdentity currentIdentity = WindowsIdentity.GetCurrent()) + { + _output.WriteLine($"Starting test as {currentIdentity.Name}"); + } // get token and run another request as different user. WindowsIdentity.RunImpersonated(_fixture.TestAccount.AccountTokenHandle, () => { - _output.WriteLine($"Running test as {WindowsIdentity.GetCurrent().Name}"); - Assert.Equal(_fixture.TestAccount.AccountName, WindowsIdentity.GetCurrent().Name); + using (WindowsIdentity currentIdentity = WindowsIdentity.GetCurrent()) + { + _output.WriteLine($"Running test as {currentIdentity.Name}"); + Assert.Equal(_fixture.TestAccount.AccountName, currentIdentity.Name); + } requestMessage = new HttpRequestMessage(HttpMethod.Get, uri); requestMessage.Version = new Version(1, 1); diff --git a/src/libraries/System.Net.HttpListener/src/System/Net/Windows/HttpListener.Windows.cs b/src/libraries/System.Net.HttpListener/src/System/Net/Windows/HttpListener.Windows.cs index 11a0f6345f91e..5a78ae0cff28e 100644 --- a/src/libraries/System.Net.HttpListener/src/System/Net/Windows/HttpListener.Windows.cs +++ b/src/libraries/System.Net.HttpListener/src/System/Net/Windows/HttpListener.Windows.cs @@ -1725,6 +1725,7 @@ private static unsafe int GetTokenSizeFromBlob(IntPtr blob) token = Interop.HttpApi.SafeLocalFreeChannelBinding.LocalAlloc(tokenSize); if (token.IsInvalid) { + token.Dispose(); throw new OutOfMemoryException(); } Marshal.Copy(blob, tokenOffset, token.DangerousGetHandle(), tokenSize); diff --git a/src/libraries/System.Net.HttpListener/src/System/Net/Windows/WebSockets/SafeWebSocketHandle.cs b/src/libraries/System.Net.HttpListener/src/System/Net/Windows/WebSockets/SafeWebSocketHandle.cs index 90ad547f5a7b1..110c80185ed7f 100644 --- a/src/libraries/System.Net.HttpListener/src/System/Net/Windows/WebSockets/SafeWebSocketHandle.cs +++ b/src/libraries/System.Net.HttpListener/src/System/Net/Windows/WebSockets/SafeWebSocketHandle.cs @@ -16,12 +16,10 @@ public SafeWebSocketHandle() protected override bool ReleaseHandle() { - if (IsInvalid) + if (!IsInvalid) { - return true; + WebSocketProtocolComponent.WebSocketDeleteHandle(handle); } - - WebSocketProtocolComponent.WebSocketDeleteHandle(handle); return true; } } diff --git a/src/libraries/System.Net.HttpListener/src/System/Net/Windows/WebSockets/ServerWebSocket.cs b/src/libraries/System.Net.HttpListener/src/System/Net/Windows/WebSockets/ServerWebSocket.cs index de881a4e20c08..8913f5e4be249 100644 --- a/src/libraries/System.Net.HttpListener/src/System/Net/Windows/WebSockets/ServerWebSocket.cs +++ b/src/libraries/System.Net.HttpListener/src/System/Net/Windows/WebSockets/ServerWebSocket.cs @@ -45,13 +45,15 @@ public ServerWebSocket(Stream innerStream, WebSocketBuffer.CreateServerBuffer(internalBuffer, receiveBufferSize)) { _properties = InternalBuffer.CreateProperties(false); - _sessionHandle = CreateWebSocketHandle(); + SafeHandle sessionHandle = CreateWebSocketHandle(); - if (_sessionHandle == null || _sessionHandle.IsInvalid) + if (sessionHandle == null || sessionHandle.IsInvalid) { + sessionHandle?.Dispose(); HttpWebSocket.ThrowPlatformNotSupportedException_WSPC(); } + _sessionHandle = sessionHandle; StartKeepAliveTimer(); } @@ -67,14 +69,9 @@ internal override SafeHandle SessionHandle private SafeHandle CreateWebSocketHandle() { Debug.Assert(_properties != null, "'_properties' MUST NOT be NULL."); - SafeWebSocketHandle sessionHandle; - WebSocketProtocolComponent.WebSocketCreateServerHandle( + return WebSocketProtocolComponent.WebSocketCreateServerHandle( _properties, - _properties.Length, - out sessionHandle); - Debug.Assert(sessionHandle != null, "'sessionHandle MUST NOT be NULL."); - - return sessionHandle; + _properties.Length); } } } diff --git a/src/libraries/System.Net.HttpListener/src/System/Net/Windows/WebSockets/WebSocketProtocolComponent.cs b/src/libraries/System.Net.HttpListener/src/System/Net/Windows/WebSockets/WebSocketProtocolComponent.cs index 8381a7831e3e5..87e5266982bf5 100644 --- a/src/libraries/System.Net.HttpListener/src/System/Net/Windows/WebSockets/WebSocketProtocolComponent.cs +++ b/src/libraries/System.Net.HttpListener/src/System/Net/Windows/WebSockets/WebSocketProtocolComponent.cs @@ -199,9 +199,7 @@ internal static string GetSupportedVersion() } } - internal static void WebSocketCreateServerHandle(Interop.WebSocket.Property[] properties, - int propertyCount, - out SafeWebSocketHandle webSocketHandle) + internal static SafeWebSocketHandle WebSocketCreateServerHandle(Interop.WebSocket.Property[] properties, int propertyCount) { Debug.Assert(propertyCount >= 0, "'propertyCount' MUST NOT be negative."); Debug.Assert((properties == null && propertyCount == 0) || @@ -213,40 +211,45 @@ internal static void WebSocketCreateServerHandle(Interop.WebSocket.Property[] pr HttpWebSocket.ThrowPlatformNotSupportedException_WSPC(); } - int errorCode = Interop.WebSocket.WebSocketCreateServerHandle(properties!, (uint)propertyCount, out webSocketHandle); - ThrowOnError(errorCode); - - if (webSocketHandle == null || - webSocketHandle.IsInvalid) + SafeWebSocketHandle? webSocketHandle = null; + try { - HttpWebSocket.ThrowPlatformNotSupportedException_WSPC(); - } - - // Currently the WSPC doesn't allow to initiate a data session - // without also being involved in the http handshake - // There is no information whatsoever, which is needed by the - // WSPC for parsing WebSocket frames from the HTTP handshake - // In the managed implementation the HTTP header handling - // will be done using the managed HTTP stack and we will - // just fake an HTTP handshake for the WSPC calling - // WebSocketBeginServerHandshake and WebSocketEndServerHandshake - // with statically defined dummy headers. - errorCode = Interop.WebSocket.WebSocketBeginServerHandshake(webSocketHandle!, - IntPtr.Zero, - IntPtr.Zero, - 0, - s_ServerFakeRequestHeaders!, - (uint)s_ServerFakeRequestHeaders!.Length, - out _, - out _); - - ThrowOnError(errorCode); + int errorCode = Interop.WebSocket.WebSocketCreateServerHandle(properties!, (uint)propertyCount, out webSocketHandle); + ThrowOnError(errorCode); + if (webSocketHandle.IsInvalid) + { + HttpWebSocket.ThrowPlatformNotSupportedException_WSPC(); + } - errorCode = Interop.WebSocket.WebSocketEndServerHandshake(webSocketHandle!); + // Currently the WSPC doesn't allow to initiate a data session + // without also being involved in the http handshake + // There is no information whatsoever, which is needed by the + // WSPC for parsing WebSocket frames from the HTTP handshake + // In the managed implementation the HTTP header handling + // will be done using the managed HTTP stack and we will + // just fake an HTTP handshake for the WSPC calling + // WebSocketBeginServerHandshake and WebSocketEndServerHandshake + // with statically defined dummy headers. + errorCode = Interop.WebSocket.WebSocketBeginServerHandshake(webSocketHandle, + IntPtr.Zero, + IntPtr.Zero, + 0, + s_ServerFakeRequestHeaders!, + (uint)s_ServerFakeRequestHeaders!.Length, + out _, + out _); + ThrowOnError(errorCode); - ThrowOnError(errorCode); + errorCode = Interop.WebSocket.WebSocketEndServerHandshake(webSocketHandle); + ThrowOnError(errorCode); + } + catch + { + webSocketHandle?.Dispose(); + throw; + } - Debug.Assert(webSocketHandle != null, "'webSocketHandle' MUST NOT be NULL at this point."); + return webSocketHandle; } internal static void WebSocketAbortHandle(SafeHandle webSocketHandle) diff --git a/src/libraries/System.Net.Ping/src/System/Net/NetworkInformation/Ping.Windows.cs b/src/libraries/System.Net.Ping/src/System/Net/NetworkInformation/Ping.Windows.cs index f34c696c96fcf..56f1883d83fdb 100644 --- a/src/libraries/System.Net.Ping/src/System/Net/NetworkInformation/Ping.Windows.cs +++ b/src/libraries/System.Net.Ping/src/System/Net/NetworkInformation/Ping.Windows.cs @@ -145,6 +145,7 @@ private void InitialiseIcmpHandle() _handlePingV4 = Interop.IpHlpApi.IcmpCreateFile(); if (_handlePingV4.IsInvalid) { + _handlePingV4.Dispose(); _handlePingV4 = null; throw new Win32Exception(); // Gets last error. } @@ -154,6 +155,7 @@ private void InitialiseIcmpHandle() _handlePingV6 = Interop.IpHlpApi.Icmp6CreateFile(); if (_handlePingV6.IsInvalid) { + _handlePingV6.Dispose(); _handlePingV6 = null; throw new Win32Exception(); // Gets last error. } diff --git a/src/libraries/System.Net.Security/src/System/Net/CertificateValidationPal.Windows.cs b/src/libraries/System.Net.Security/src/System/Net/CertificateValidationPal.Windows.cs index 2e5c5e5daba2f..28712fec33575 100644 --- a/src/libraries/System.Net.Security/src/System/Net/CertificateValidationPal.Windows.cs +++ b/src/libraries/System.Net.Security/src/System/Net/CertificateValidationPal.Windows.cs @@ -62,13 +62,16 @@ internal static SslPolicyErrors VerifyCertificateProperties( } finally { - if (remoteContext != null && !remoteContext.IsInvalid) + if (remoteContext != null) { - if (retrieveChainCertificates) + if (!remoteContext.IsInvalid) { - chain ??= new X509Chain(); + if (retrieveChainCertificates) + { + chain ??= new X509Chain(); - UnmanagedCertificateContext.GetRemoteCertificatesFromStoreContext(remoteContext, chain.ChainPolicy.ExtraStore); + UnmanagedCertificateContext.GetRemoteCertificatesFromStoreContext(remoteContext, chain.ChainPolicy.ExtraStore); + } } remoteContext.Dispose(); @@ -128,7 +131,8 @@ internal static X509Store OpenStore(StoreLocation storeLocation) // For app-compat We want to ensure the store is opened under the **process** account. try { - WindowsIdentity.RunImpersonated(SafeAccessTokenHandle.InvalidHandle, () => + using SafeAccessTokenHandle invalidHandle = SafeAccessTokenHandle.InvalidHandle; + WindowsIdentity.RunImpersonated(invalidHandle, () => { store.Open(OpenFlags.ReadOnly | OpenFlags.OpenExistingOnly); }); diff --git a/src/libraries/System.Net.Security/src/System/Net/Security/SslStreamPal.Windows.cs b/src/libraries/System.Net.Security/src/System/Net/Security/SslStreamPal.Windows.cs index db9e14f64804b..1321cc0754ed0 100644 --- a/src/libraries/System.Net.Security/src/System/Net/Security/SslStreamPal.Windows.cs +++ b/src/libraries/System.Net.Security/src/System/Net/Security/SslStreamPal.Windows.cs @@ -571,7 +571,8 @@ private static unsafe SafeFreeCredentials AcquireCredentialsHandle(Interop.SspiC // // For app-compat we want to ensure the credential are accessed under >>process<< account. // - return WindowsIdentity.RunImpersonated(SafeAccessTokenHandle.InvalidHandle, () => + using SafeAccessTokenHandle invalidHandle = SafeAccessTokenHandle.InvalidHandle; + return WindowsIdentity.RunImpersonated(invalidHandle, () => { return SSPIWrapper.AcquireCredentialsHandle(GlobalSSPI.SSPISecureChannel, SecurityPackage, credUsage, secureCredential); }); @@ -591,7 +592,8 @@ private static unsafe SafeFreeCredentials AcquireCredentialsHandle(Interop.SspiC // // For app-compat we want to ensure the credential are accessed under >>process<< account. // - return WindowsIdentity.RunImpersonated(SafeAccessTokenHandle.InvalidHandle, () => + using SafeAccessTokenHandle invalidHandle = SafeAccessTokenHandle.InvalidHandle; + return WindowsIdentity.RunImpersonated(invalidHandle, () => { return SSPIWrapper.AcquireCredentialsHandle(GlobalSSPI.SSPISecureChannel, SecurityPackage, credUsage, secureCredential); }); diff --git a/src/libraries/System.Net.Security/tests/FunctionalTests/IdentityValidator.Windows.cs b/src/libraries/System.Net.Security/tests/FunctionalTests/IdentityValidator.Windows.cs index 1c1510e3c275a..7ca81877be69c 100644 --- a/src/libraries/System.Net.Security/tests/FunctionalTests/IdentityValidator.Windows.cs +++ b/src/libraries/System.Net.Security/tests/FunctionalTests/IdentityValidator.Windows.cs @@ -11,7 +11,8 @@ public class IdentityValidator { public static void AssertIsCurrentIdentity(IIdentity identity) { - Assert.Equal(WindowsIdentity.GetCurrent().Name, identity.Name); + using WindowsIdentity currentIdentity = WindowsIdentity.GetCurrent(); + Assert.Equal(currentIdentity.Name, identity.Name); } public static void AssertHasName(IIdentity identity, string expectedName) diff --git a/src/libraries/System.Net.Sockets/src/System/Net/Sockets/Socket.cs b/src/libraries/System.Net.Sockets/src/System/Net/Sockets/Socket.cs index 836d4485a4c24..61473709129ef 100644 --- a/src/libraries/System.Net.Sockets/src/System/Net/Sockets/Socket.cs +++ b/src/libraries/System.Net.Sockets/src/System/Net/Sockets/Socket.cs @@ -1053,6 +1053,7 @@ public Socket Accept() SocketsTelemetry.Log.AfterAccept(errorCode); + acceptedSocketHandle.Dispose(); UpdateStatusAfterSocketErrorAndThrowException(errorCode); } diff --git a/src/libraries/System.Net.Sockets/src/System/Net/Sockets/SocketPal.Unix.cs b/src/libraries/System.Net.Sockets/src/System/Net/Sockets/SocketPal.Unix.cs index 7e51174d7569e..f80ed04d4d459 100644 --- a/src/libraries/System.Net.Sockets/src/System/Net/Sockets/SocketPal.Unix.cs +++ b/src/libraries/System.Net.Sockets/src/System/Net/Sockets/SocketPal.Unix.cs @@ -88,6 +88,10 @@ public static unsafe SocketError CreateSocket(AddressFamily addressFamily, Socke socket = new SafeSocketHandle(fd, ownsHandle: true); if (NetEventSource.Log.IsEnabled()) NetEventSource.Info(null, socket); + if (socket.IsInvalid) + { + socket.Dispose(); + } return errorCode; } diff --git a/src/libraries/System.Net.Sockets/tests/FunctionalTests/StartupTests.Windows.cs b/src/libraries/System.Net.Sockets/tests/FunctionalTests/StartupTests.Windows.cs index d48d76799676e..8f6503ca9c0fe 100644 --- a/src/libraries/System.Net.Sockets/tests/FunctionalTests/StartupTests.Windows.cs +++ b/src/libraries/System.Net.Sockets/tests/FunctionalTests/StartupTests.Windows.cs @@ -2,6 +2,7 @@ // The .NET Foundation licenses this file to you under the MIT license. using System.IO.Pipes; +using System.Runtime.InteropServices; using Microsoft.DotNet.RemoteExecutor; using Xunit; @@ -88,7 +89,8 @@ public static void Ctor_AddressFamily_SocketType_ProtocolType() public static void Ctor_SafeHandle() => RemoteExecutor.Invoke(() => { using var pipe = new AnonymousPipeServerStream(); - SocketException se = Assert.Throws(() => new Socket(new SafeSocketHandle(pipe.ClientSafePipeHandle.DangerousGetHandle(), ownsHandle: false))); + using SafeHandle clientSafeHandle = pipe.ClientSafePipeHandle; + SocketException se = Assert.Throws(() => new Socket(new SafeSocketHandle(clientSafeHandle.DangerousGetHandle(), ownsHandle: false))); Assert.Equal(SocketError.NotSocket, se.SocketErrorCode); }).Dispose(); } diff --git a/src/libraries/System.Net.Sockets/tests/FunctionalTests/UdpClientTest.cs b/src/libraries/System.Net.Sockets/tests/FunctionalTests/UdpClientTest.cs index 66f69e1ce70da..3ea2bf0f8cb1f 100644 --- a/src/libraries/System.Net.Sockets/tests/FunctionalTests/UdpClientTest.cs +++ b/src/libraries/System.Net.Sockets/tests/FunctionalTests/UdpClientTest.cs @@ -16,7 +16,7 @@ public class UdpClientTest private const int DiscardPort = 9; - private ManualResetEvent _waitHandle = new ManualResetEvent(false); + private ManualResetEventSlim _waitHandle = new ManualResetEventSlim(false); [Theory] [InlineData(AddressFamily.InterNetwork)] @@ -370,7 +370,7 @@ public void BeginSend_AsyncOperationCompletes_Success() _waitHandle.Reset(); udpClient.BeginSend(sendBytes, sendBytes.Length, remoteServer, new AsyncCallback(AsyncCompleted), udpClient); - Assert.True(_waitHandle.WaitOne(TestSettings.PassingTestTimeout), "Timed out while waiting for connection"); + Assert.True(_waitHandle.Wait(TestSettings.PassingTestTimeout), "Timed out while waiting for connection"); } } diff --git a/src/libraries/System.Net.WebSockets/src/System/Net/WebSockets/Compression/WebSocketDeflater.cs b/src/libraries/System.Net.WebSockets/src/System/Net/WebSockets/Compression/WebSocketDeflater.cs index 7c8db8eedcc87..8f3e2054ba6f4 100644 --- a/src/libraries/System.Net.WebSockets/src/System/Net/WebSockets/Compression/WebSocketDeflater.cs +++ b/src/libraries/System.Net.WebSockets/src/System/Net/WebSockets/Compression/WebSocketDeflater.cs @@ -210,7 +210,7 @@ private static ErrorCode Deflate(ZLibStreamHandle stream, FlushCode flushCode) private ZLibStreamHandle CreateDeflater() { - ZLibStreamHandle stream; + ZLibStreamHandle? stream = null; ErrorCode errorCode; try { @@ -222,6 +222,7 @@ private ZLibStreamHandle CreateDeflater() } catch (Exception cause) { + stream?.Dispose(); throw new WebSocketException(SR.ZLibErrorDLLLoadError, cause); } diff --git a/src/libraries/System.Net.WebSockets/src/System/Net/WebSockets/Compression/WebSocketInflater.cs b/src/libraries/System.Net.WebSockets/src/System/Net/WebSockets/Compression/WebSocketInflater.cs index 5f580e9c9fdb3..6b87b3180ebc3 100644 --- a/src/libraries/System.Net.WebSockets/src/System/Net/WebSockets/Compression/WebSocketInflater.cs +++ b/src/libraries/System.Net.WebSockets/src/System/Net/WebSockets/Compression/WebSocketInflater.cs @@ -259,7 +259,7 @@ private static unsafe int Inflate(ZLibStreamHandle stream, Span destinatio private ZLibStreamHandle CreateInflater() { - ZLibStreamHandle stream; + ZLibStreamHandle? stream = null; ErrorCode errorCode; try @@ -268,6 +268,7 @@ private ZLibStreamHandle CreateInflater() } catch (Exception exception) { + stream?.Dispose(); throw new WebSocketException(SR.ZLibErrorDLLLoadError, exception); } diff --git a/src/libraries/System.Private.CoreLib/src/Internal/Win32/RegistryKey.cs b/src/libraries/System.Private.CoreLib/src/Internal/Win32/RegistryKey.cs index 9bc76f0513f69..3406060f90e5e 100644 --- a/src/libraries/System.Private.CoreLib/src/Internal/Win32/RegistryKey.cs +++ b/src/libraries/System.Private.CoreLib/src/Internal/Win32/RegistryKey.cs @@ -95,6 +95,8 @@ internal static RegistryKey OpenBaseKey(IntPtr hKey) return new RegistryKey(result); } + result.Dispose(); + // Return null if we didn't find the key. if (ret == Interop.Errors.ERROR_ACCESS_DENIED || ret == Interop.Errors.ERROR_BAD_IMPERSONATION_LEVEL) { diff --git a/src/libraries/System.Private.CoreLib/src/Microsoft/Win32/SafeHandles/SafeFileHandle.Windows.cs b/src/libraries/System.Private.CoreLib/src/Microsoft/Win32/SafeHandles/SafeFileHandle.Windows.cs index 66218ec2c61a8..534331acaa878 100644 --- a/src/libraries/System.Private.CoreLib/src/Microsoft/Win32/SafeHandles/SafeFileHandle.Windows.cs +++ b/src/libraries/System.Private.CoreLib/src/Microsoft/Win32/SafeHandles/SafeFileHandle.Windows.cs @@ -109,6 +109,7 @@ private static unsafe SafeFileHandle CreateFile(string fullPath, FileMode mode, errorCode = Interop.Errors.ERROR_ACCESS_DENIED; } + fileHandle.Dispose(); throw Win32Marshal.GetExceptionForWin32Error(errorCode, fullPath); } diff --git a/src/libraries/System.Private.CoreLib/src/System/IO/FileSystem.Windows.cs b/src/libraries/System.Private.CoreLib/src/System/IO/FileSystem.Windows.cs index 73362512175f3..7a53afb2cde0a 100644 --- a/src/libraries/System.Private.CoreLib/src/System/IO/FileSystem.Windows.cs +++ b/src/libraries/System.Private.CoreLib/src/System/IO/FileSystem.Windows.cs @@ -221,6 +221,7 @@ private static SafeFileHandle OpenHandleToWriteAttributes(string fullPath, bool if (!asDirectory && errorCode == Interop.Errors.ERROR_PATH_NOT_FOUND && fullPath.Equals(Directory.GetDirectoryRoot(fullPath))) errorCode = Interop.Errors.ERROR_ACCESS_DENIED; + handle.Dispose(); throw Win32Marshal.GetExceptionForWin32Error(errorCode, fullPath); } diff --git a/src/libraries/System.Private.CoreLib/src/System/Runtime/InteropServices/SafeHandle.cs b/src/libraries/System.Private.CoreLib/src/System/Runtime/InteropServices/SafeHandle.cs index aa4534afcb11a..4de94b04f7937 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Runtime/InteropServices/SafeHandle.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Runtime/InteropServices/SafeHandle.cs @@ -2,6 +2,7 @@ // The .NET Foundation licenses this file to you under the MIT license. using System.Diagnostics; +using System.Runtime.CompilerServices; using System.Runtime.ConstrainedExecution; using System.Threading; @@ -13,6 +14,10 @@ namespace System.Runtime.InteropServices /// Represents a wrapper class for operating system handles. public abstract partial class SafeHandle : CriticalFinalizerObject, IDisposable { +#if DEBUG + private static readonly bool s_logFinalization = Environment.GetEnvironmentVariable("DEBUG_SAFEHANDLE_FINALIZATION") == "1"; +#endif + // IMPORTANT: // - Do not add or rearrange fields as the EE depends on this layout, // as well as on the values of the StateBits flags. @@ -20,6 +25,9 @@ public abstract partial class SafeHandle : CriticalFinalizerObject, IDisposable // code, so this managed code must not assume it is the only code // manipulating _state. +#if DEBUG + private readonly string? _ctorStackTrace; +#endif /// Specifies the handle to be wrapped. protected IntPtr handle; /// Combined ref count and closed/disposed flags (so we can atomically modify them). @@ -60,11 +68,19 @@ protected SafeHandle(IntPtr invalidHandleValue, bool ownsHandle) { GC.SuppressFinalize(this); } +#if DEBUG + else if (s_logFinalization) + { + int lastError = Marshal.GetLastPInvokeError(); + _ctorStackTrace = Environment.StackTrace; + Marshal.SetLastPInvokeError(lastError); + } +#endif _fullyInitialized = true; } -#if !NATIVEAOT // NativeAOT doesn't correctly support CriticalFinalizerObject +#if !NATIVEAOT // NativeAOT doesn't correctly support CriticalFinalizerObject; separate implementation provided ~SafeHandle() { if (_fullyInitialized) @@ -94,6 +110,12 @@ public void Dispose() protected virtual void Dispose(bool disposing) { +#if DEBUG + if (!disposing && _ctorStackTrace is not null) + { + Internal.Console.WriteLine($"{Environment.NewLine}*** {GetType()} (0x{handle.ToInt64():x}) finalized! Ctor stack:{Environment.NewLine}{_ctorStackTrace}{Environment.NewLine}"); + } +#endif Debug.Assert(_fullyInitialized); InternalRelease(disposeOrFinalizeOperation: true); } diff --git a/src/libraries/System.Private.CoreLib/src/System/Threading/EventWaitHandle.Windows.cs b/src/libraries/System.Private.CoreLib/src/System/Threading/EventWaitHandle.Windows.cs index bf4427fa36c3e..95750074698b0 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Threading/EventWaitHandle.Windows.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Threading/EventWaitHandle.Windows.cs @@ -53,6 +53,8 @@ private static OpenExistingResult OpenExistingWorker(string name, out EventWaitH { int errorCode = Marshal.GetLastPInvokeError(); + myHandle.Dispose(); + if (errorCode == Interop.Errors.ERROR_FILE_NOT_FOUND || errorCode == Interop.Errors.ERROR_INVALID_NAME) return OpenExistingResult.NameNotFound; if (errorCode == Interop.Errors.ERROR_PATH_NOT_FOUND) diff --git a/src/libraries/System.Private.CoreLib/src/System/Threading/Mutex.Windows.cs b/src/libraries/System.Private.CoreLib/src/System/Threading/Mutex.Windows.cs index f1a22346801d4..ab5663297cdc5 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Threading/Mutex.Windows.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Threading/Mutex.Windows.cs @@ -53,6 +53,9 @@ private static OpenExistingResult OpenExistingWorker(string name, out Mutex? res if (myHandle.IsInvalid) { int errorCode = Marshal.GetLastPInvokeError(); + + myHandle.Dispose(); + #if TARGET_UNIX || TARGET_BROWSER if (errorCode == Interop.Errors.ERROR_FILENAME_EXCED_RANGE) { diff --git a/src/libraries/System.Private.CoreLib/src/System/Threading/Semaphore.Windows.cs b/src/libraries/System.Private.CoreLib/src/System/Threading/Semaphore.Windows.cs index 013023fe212a8..a4cb8323ef5b2 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Threading/Semaphore.Windows.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Threading/Semaphore.Windows.cs @@ -32,6 +32,8 @@ private void CreateSemaphoreCore(int initialCount, int maximumCount, string? nam int errorCode = Marshal.GetLastPInvokeError(); if (myHandle.IsInvalid) { + myHandle.Dispose(); + if (!string.IsNullOrEmpty(name) && errorCode == Interop.Errors.ERROR_INVALID_HANDLE) throw new WaitHandleCannotBeOpenedException( SR.Format(SR.Threading_WaitHandleCannotBeOpenedException_InvalidHandle, name)); @@ -55,6 +57,8 @@ private static OpenExistingResult OpenExistingWorker(string name, out Semaphore? result = null; int errorCode = Marshal.GetLastPInvokeError(); + myHandle.Dispose(); + if (errorCode == Interop.Errors.ERROR_FILE_NOT_FOUND || errorCode == Interop.Errors.ERROR_INVALID_NAME) return OpenExistingResult.NameNotFound; if (errorCode == Interop.Errors.ERROR_PATH_NOT_FOUND) diff --git a/src/libraries/System.Reflection.Metadata/src/System/Reflection/PortableExecutable/PEReader.EmbeddedPortablePdb.cs b/src/libraries/System.Reflection.Metadata/src/System/Reflection/PortableExecutable/PEReader.EmbeddedPortablePdb.cs index 08ba31fdda3c0..0c9d4450e2c52 100644 --- a/src/libraries/System.Reflection.Metadata/src/System/Reflection/PortableExecutable/PEReader.EmbeddedPortablePdb.cs +++ b/src/libraries/System.Reflection.Metadata/src/System/Reflection/PortableExecutable/PEReader.EmbeddedPortablePdb.cs @@ -92,7 +92,7 @@ internal static unsafe NativeHeapMemoryBlock DecodeEmbeddedPortablePdbDebugDirec try { var compressed = new ReadOnlyUnmanagedMemoryStream(headerReader.CurrentPointer, headerReader.RemainingBytes); - var deflate = new DeflateStream(compressed, CompressionMode.Decompress, leaveOpen: true); + using var deflate = new DeflateStream(compressed, CompressionMode.Decompress, leaveOpen: true); if (decompressedSize > 0) { diff --git a/src/libraries/System.Security.AccessControl/src/System/Security/Principal/Win32.cs b/src/libraries/System.Security.AccessControl/src/System/Security/Principal/Win32.cs index ebef5192bdd3a..292ffa3c1e63b 100644 --- a/src/libraries/System.Security.AccessControl/src/System/Security/Principal/Win32.cs +++ b/src/libraries/System.Security.AccessControl/src/System/Security/Principal/Win32.cs @@ -28,6 +28,7 @@ internal static int OpenThreadToken(TokenAccessLevels dwDesiredAccess, WinSecuri { openAsSelf = false; hr = 0; + phThreadToken.Dispose(); if (!Interop.Advapi32.OpenThreadToken((IntPtr)(-2), dwDesiredAccess, openAsSelf, out phThreadToken)) hr = Marshal.GetHRForLastWin32Error(); } @@ -36,7 +37,12 @@ internal static int OpenThreadToken(TokenAccessLevels dwDesiredAccess, WinSecuri hr = Marshal.GetHRForLastWin32Error(); } } - if (hr != 0) phThreadToken = null; + if (hr != 0) + { + phThreadToken.Dispose(); + phThreadToken = null; + } + return hr; } diff --git a/src/libraries/System.Security.Cryptography.Pkcs/src/Internal/Cryptography/Pal/Windows/DecryptorPalWindows.Decode.cs b/src/libraries/System.Security.Cryptography.Pkcs/src/Internal/Cryptography/Pal/Windows/DecryptorPalWindows.Decode.cs index d2df673bdd673..f999f555eea65 100644 --- a/src/libraries/System.Security.Cryptography.Pkcs/src/Internal/Cryptography/Pal/Windows/DecryptorPalWindows.Decode.cs +++ b/src/libraries/System.Security.Cryptography.Pkcs/src/Internal/Cryptography/Pal/Windows/DecryptorPalWindows.Decode.cs @@ -25,44 +25,53 @@ internal static DecryptorPalWindows Decode( out CryptographicAttributeObjectCollection unprotectedAttributes ) { - SafeCryptMsgHandle hCryptMsg = Interop.Crypt32.CryptMsgOpenToDecode(MsgEncodingType.All, 0, 0, IntPtr.Zero, IntPtr.Zero, IntPtr.Zero); - if (hCryptMsg == null || hCryptMsg.IsInvalid) - throw Marshal.GetLastWin32Error().ToCryptographicException(); - - if (!Interop.Crypt32.CryptMsgUpdate( - hCryptMsg, - ref MemoryMarshal.GetReference(encodedMessage), - encodedMessage.Length, - fFinal: true)) + SafeCryptMsgHandle? hCryptMsg = null; + try { - throw Marshal.GetLastWin32Error().ToCryptographicException(); - } + hCryptMsg = Interop.Crypt32.CryptMsgOpenToDecode(MsgEncodingType.All, 0, 0, IntPtr.Zero, IntPtr.Zero, IntPtr.Zero); + if (hCryptMsg == null || hCryptMsg.IsInvalid) + throw Marshal.GetLastWin32Error().ToCryptographicException(); - CryptMsgType cryptMsgType = hCryptMsg.GetMessageType(); - if (cryptMsgType != CryptMsgType.CMSG_ENVELOPED) - throw ErrorCode.CRYPT_E_INVALID_MSG_TYPE.ToCryptographicException(); + if (!Interop.Crypt32.CryptMsgUpdate( + hCryptMsg, + ref MemoryMarshal.GetReference(encodedMessage), + encodedMessage.Length, + fFinal: true)) + { + throw Marshal.GetLastWin32Error().ToCryptographicException(); + } - version = hCryptMsg.GetVersion(); + CryptMsgType cryptMsgType = hCryptMsg.GetMessageType(); + if (cryptMsgType != CryptMsgType.CMSG_ENVELOPED) + throw ErrorCode.CRYPT_E_INVALID_MSG_TYPE.ToCryptographicException(); - contentInfo = hCryptMsg.GetContentInfo(); + version = hCryptMsg.GetVersion(); - AlgorithmIdentifierAsn contentEncryptionAlgorithmAsn; - using (SafeHandle sh = hCryptMsg.GetMsgParamAsMemory(CryptMsgParamType.CMSG_ENVELOPE_ALGORITHM_PARAM)) - { - unsafe + contentInfo = hCryptMsg.GetContentInfo(); + + AlgorithmIdentifierAsn contentEncryptionAlgorithmAsn; + using (SafeHandle sh = hCryptMsg.GetMsgParamAsMemory(CryptMsgParamType.CMSG_ENVELOPE_ALGORITHM_PARAM)) { - CRYPT_ALGORITHM_IDENTIFIER* pCryptAlgorithmIdentifier = (CRYPT_ALGORITHM_IDENTIFIER*)(sh.DangerousGetHandle()); - contentEncryptionAlgorithm = (*pCryptAlgorithmIdentifier).ToAlgorithmIdentifier(); - contentEncryptionAlgorithmAsn.Algorithm = contentEncryptionAlgorithm.Oid.Value!; - contentEncryptionAlgorithmAsn.Parameters = (*pCryptAlgorithmIdentifier).Parameters.ToByteArray(); + unsafe + { + CRYPT_ALGORITHM_IDENTIFIER* pCryptAlgorithmIdentifier = (CRYPT_ALGORITHM_IDENTIFIER*)(sh.DangerousGetHandle()); + contentEncryptionAlgorithm = (*pCryptAlgorithmIdentifier).ToAlgorithmIdentifier(); + contentEncryptionAlgorithmAsn.Algorithm = contentEncryptionAlgorithm.Oid.Value!; + contentEncryptionAlgorithmAsn.Parameters = (*pCryptAlgorithmIdentifier).Parameters.ToByteArray(); + } } - } - originatorCerts = hCryptMsg.GetOriginatorCerts(); - unprotectedAttributes = hCryptMsg.GetUnprotectedAttributes(); + originatorCerts = hCryptMsg.GetOriginatorCerts(); + unprotectedAttributes = hCryptMsg.GetUnprotectedAttributes(); - RecipientInfoCollection recipientInfos = CreateRecipientInfos(hCryptMsg); - return new DecryptorPalWindows(hCryptMsg, recipientInfos, contentEncryptionAlgorithmAsn); + RecipientInfoCollection recipientInfos = CreateRecipientInfos(hCryptMsg); + return new DecryptorPalWindows(hCryptMsg, recipientInfos, contentEncryptionAlgorithmAsn); + } + catch + { + hCryptMsg?.Dispose(); + throw; + } } } } diff --git a/src/libraries/System.Security.Cryptography.Pkcs/src/Internal/Cryptography/Pal/Windows/HelpersWindows.cs b/src/libraries/System.Security.Cryptography.Pkcs/src/Internal/Cryptography/Pal/Windows/HelpersWindows.cs index 27796ee44bce8..f675d068c65c7 100644 --- a/src/libraries/System.Security.Cryptography.Pkcs/src/Internal/Cryptography/Pal/Windows/HelpersWindows.cs +++ b/src/libraries/System.Security.Cryptography.Pkcs/src/Internal/Cryptography/Pal/Windows/HelpersWindows.cs @@ -53,11 +53,16 @@ public static SafeHandle GetMsgParamAsMemory(this SafeCryptMsgHandle hCryptMsg, { int cbData = 0; if (!Interop.Crypt32.CryptMsgGetParam(hCryptMsg, paramType, index, IntPtr.Zero, ref cbData)) + { throw Marshal.GetLastWin32Error().ToCryptographicException(); + } SafeHandle pvData = SafeHeapAllocHandle.Alloc(cbData); if (!Interop.Crypt32.CryptMsgGetParam(hCryptMsg, paramType, index, pvData.DangerousGetHandle(), ref cbData)) + { + pvData.Dispose(); throw Marshal.GetLastWin32Error().ToCryptographicException(); + } return pvData; } diff --git a/src/libraries/System.Security.Cryptography.Pkcs/src/Internal/Cryptography/Pal/Windows/PkcsPalWindows.Encrypt.cs b/src/libraries/System.Security.Cryptography.Pkcs/src/Internal/Cryptography/Pal/Windows/PkcsPalWindows.Encrypt.cs index 177cccd040819..12329d93e319c 100644 --- a/src/libraries/System.Security.Cryptography.Pkcs/src/Internal/Cryptography/Pal/Windows/PkcsPalWindows.Encrypt.cs +++ b/src/libraries/System.Security.Cryptography.Pkcs/src/Internal/Cryptography/Pal/Windows/PkcsPalWindows.Encrypt.cs @@ -122,7 +122,11 @@ public static SafeCryptMsgHandle CreateCryptMsgHandleToEncode(CmsRecipientCollec CMSG_ENVELOPED_ENCODE_INFO* pEnvelopedEncodeInfo = CreateCmsEnvelopedEncodeInfo(recipients, contentEncryptionAlgorithm, originatorCerts, unprotectedAttributes, hb); SafeCryptMsgHandle hCryptMsg = Interop.Crypt32.CryptMsgOpenToEncode(MsgEncodingType.All, 0, CryptMsgType.CMSG_ENVELOPED, pEnvelopedEncodeInfo, innerContentType.Value!, IntPtr.Zero); if (hCryptMsg == null || hCryptMsg.IsInvalid) - throw Marshal.GetLastWin32Error().ToCryptographicException(); + { + Exception e = Marshal.GetLastWin32Error().ToCryptographicException(); + hCryptMsg?.Dispose(); + throw e; + } return hCryptMsg; } diff --git a/src/libraries/System.Security.Cryptography.Xml/src/System/Security/Cryptography/Xml/EncryptedXml.cs b/src/libraries/System.Security.Cryptography.Xml/src/System/Security/Cryptography/Xml/EncryptedXml.cs index 3eb0d8d4a168b..5e1bb78434451 100644 --- a/src/libraries/System.Security.Cryptography.Xml/src/System/Security/Cryptography/Xml/EncryptedXml.cs +++ b/src/libraries/System.Security.Cryptography.Xml/src/System/Security/Cryptography/Xml/EncryptedXml.cs @@ -699,7 +699,7 @@ public byte[] EncryptData(byte[] plaintext, SymmetricAlgorithm symmetricAlgorith symmetricAlgorithm.Mode = _mode; symmetricAlgorithm.Padding = _padding; - ICryptoTransform enc = symmetricAlgorithm.CreateEncryptor(); + using ICryptoTransform enc = symmetricAlgorithm.CreateEncryptor(); cipher = enc.TransformFinalBlock(plaintext, 0, plaintext.Length); } finally @@ -777,7 +777,7 @@ public byte[] DecryptData(EncryptedData encryptedData, SymmetricAlgorithm symmet symmetricAlgorithm.Mode = _mode; symmetricAlgorithm.Padding = _padding; - ICryptoTransform dec = symmetricAlgorithm.CreateDecryptor(); + using ICryptoTransform dec = symmetricAlgorithm.CreateDecryptor(); output = dec.TransformFinalBlock(cipherValue, lengthIV, cipherValue.Length - lengthIV); } finally diff --git a/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/AsymmetricAlgorithm.cs b/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/AsymmetricAlgorithm.cs index 4917750edb7d5..6d671cd841482 100644 --- a/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/AsymmetricAlgorithm.cs +++ b/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/AsymmetricAlgorithm.cs @@ -20,7 +20,7 @@ public static AsymmetricAlgorithm Create() => [Obsolete(Obsoletions.CryptoStringFactoryMessage, DiagnosticId = Obsoletions.CryptoStringFactoryDiagId, UrlFormat = Obsoletions.SharedUrlFormat)] [RequiresUnreferencedCode(CryptoConfigForwarder.CreateFromNameUnreferencedCodeMessage)] public static AsymmetricAlgorithm? Create(string algName) => - (AsymmetricAlgorithm?)CryptoConfigForwarder.CreateFromName(algName); + CryptoConfigForwarder.CreateFromName(algName); public virtual int KeySize { diff --git a/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/CapiHelper.Windows.cs b/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/CapiHelper.Windows.cs index 5f626aa985803..a91e22fce4de1 100644 --- a/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/CapiHelper.Windows.cs +++ b/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/CapiHelper.Windows.cs @@ -274,6 +274,7 @@ internal static SafeProvHandle CreateProvHandle(CspParameters parameters, bool r if (hr != S_OK) { safeProvHandle.Dispose(); + // If UseExistingKey flag is used and the key container does not exist // throw an exception without attempting to create the container. if (IsFlagBitSet((uint)parameters.Flags, (uint)CspProviderFlags.UseExistingKey) || @@ -294,6 +295,7 @@ internal static SafeProvHandle CreateProvHandle(CspParameters parameters, bool r if (!Interop.Advapi32.CryptSetProvParam(safeProvHandle, CryptProvParam.PP_CLIENT_HWND, ref parentWindowHandle, 0)) { + safeProvHandle.Dispose(); throw GetErrorCode().ToCryptographicException(); } } @@ -309,6 +311,7 @@ internal static SafeProvHandle CreateProvHandle(CspParameters parameters, bool r CryptProvParam.PP_KEYEXCHANGE_PIN; if (!Interop.Advapi32.CryptSetProvParam(safeProvHandle, param, password, 0)) { + safeProvHandle.Dispose(); throw GetErrorCode().ToCryptographicException(); } } diff --git a/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/CngHelpers.cs b/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/CngHelpers.cs index 24d5f7637c2f5..59db73d98f073 100644 --- a/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/CngHelpers.cs +++ b/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/CngHelpers.cs @@ -29,6 +29,7 @@ internal static SafeNCryptProviderHandle OpenStorageProvider(this CngProvider pr if (errorCode != ErrorCode.ERROR_SUCCESS) { + providerHandle.Dispose(); throw errorCode.ToCryptographicException(); } diff --git a/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/CngKey.Create.cs b/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/CngKey.Create.cs index e184e13aaaa9d..2b2ad17d1668d 100644 --- a/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/CngKey.Create.cs +++ b/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/CngKey.Create.cs @@ -39,32 +39,41 @@ public static CngKey Create(CngAlgorithm algorithm, string? keyName, CngKeyCreat creationParameters ??= new CngKeyCreationParameters(); SafeNCryptProviderHandle providerHandle = creationParameters.Provider!.OpenStorageProvider(); - SafeNCryptKeyHandle keyHandle; - ErrorCode errorCode = Interop.NCrypt.NCryptCreatePersistedKey(providerHandle, out keyHandle, algorithm.Algorithm, keyName, 0, creationParameters.KeyCreationOptions); - if (errorCode != ErrorCode.ERROR_SUCCESS) + SafeNCryptKeyHandle? keyHandle = null; + try { - // For ecc, the exception may be caught and re-thrown as PlatformNotSupportedException - throw errorCode.ToCryptographicException(); - } + ErrorCode errorCode = Interop.NCrypt.NCryptCreatePersistedKey(providerHandle, out keyHandle, algorithm.Algorithm, keyName, 0, creationParameters.KeyCreationOptions); + if (errorCode != ErrorCode.ERROR_SUCCESS) + { + // For ecc, the exception may be caught and re-thrown as PlatformNotSupportedException + throw errorCode.ToCryptographicException(); + } - InitializeKeyProperties(keyHandle, creationParameters); + InitializeKeyProperties(keyHandle, creationParameters); - errorCode = Interop.NCrypt.NCryptFinalizeKey(keyHandle, 0); - if (errorCode != ErrorCode.ERROR_SUCCESS) - { - // For ecc, the exception may be caught and re-thrown as PlatformNotSupportedException - throw errorCode.ToCryptographicException(); - } + errorCode = Interop.NCrypt.NCryptFinalizeKey(keyHandle, 0); + if (errorCode != ErrorCode.ERROR_SUCCESS) + { + // For ecc, the exception may be caught and re-thrown as PlatformNotSupportedException + throw errorCode.ToCryptographicException(); + } + + CngKey key = new CngKey(providerHandle, keyHandle); - CngKey key = new CngKey(providerHandle, keyHandle); + // No name translates to an ephemeral key + if (keyName == null) + { + key.IsEphemeral = true; + } - // No name translates to an ephemeral key - if (keyName == null) + return key; + } + catch { - key.IsEphemeral = true; + keyHandle?.Dispose(); + providerHandle.Dispose(); + throw; } - - return key; } /// diff --git a/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/CngKey.Import.cs b/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/CngKey.Import.cs index 25a2329d91879..b7355f7b3c07d 100644 --- a/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/CngKey.Import.cs +++ b/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/CngKey.Import.cs @@ -92,6 +92,7 @@ ref MemoryMarshal.GetReference(keyBlob), if (errorCode != ErrorCode.ERROR_SUCCESS) { keyHandle.Dispose(); + providerHandle.Dispose(); throw errorCode.ToCryptographicException(); } } @@ -122,37 +123,48 @@ internal static CngKey Import( ArgumentNullException.ThrowIfNull(provider); SafeNCryptProviderHandle providerHandle = provider.OpenStorageProvider(); - SafeNCryptKeyHandle? keyHandle; - ErrorCode errorCode; - - if (curveName == null) + SafeNCryptKeyHandle? keyHandle = null; + try { - errorCode = Interop.NCrypt.NCryptImportKey( - providerHandle, - IntPtr.Zero, - format.Format, - IntPtr.Zero, - out keyHandle, - ref MemoryMarshal.GetReference(keyBlob), - keyBlob.Length, - 0); + ErrorCode errorCode; - if (errorCode != ErrorCode.ERROR_SUCCESS) + if (curveName == null) { - throw errorCode.ToCryptographicException(); + errorCode = Interop.NCrypt.NCryptImportKey( + providerHandle, + IntPtr.Zero, + format.Format, + IntPtr.Zero, + out keyHandle, + ref MemoryMarshal.GetReference(keyBlob), + keyBlob.Length, + 0); + + if (errorCode != ErrorCode.ERROR_SUCCESS) + { + providerHandle.Dispose(); + keyHandle.Dispose(); + throw errorCode.ToCryptographicException(); + } + } + else + { + keyHandle = ECCng.ImportKeyBlob(format.Format, keyBlob, curveName, providerHandle); } - } - else - { - keyHandle = ECCng.ImportKeyBlob(format.Format, keyBlob, curveName, providerHandle); - } - CngKey key = new CngKey(providerHandle, keyHandle); + CngKey key = new CngKey(providerHandle, keyHandle); - // We can't tell directly if an OpaqueTransport blob imported as an ephemeral key or not - key.IsEphemeral = format != CngKeyBlobFormat.OpaqueTransportBlob; + // We can't tell directly if an OpaqueTransport blob imported as an ephemeral key or not + key.IsEphemeral = format != CngKeyBlobFormat.OpaqueTransportBlob; - return key; + return key; + } + catch + { + keyHandle?.Dispose(); + providerHandle.Dispose(); + throw; + } } } } diff --git a/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/CngKey.Open.cs b/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/CngKey.Open.cs index c0b8b8ea91cea..aea118b3c862f 100644 --- a/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/CngKey.Open.cs +++ b/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/CngKey.Open.cs @@ -41,7 +41,11 @@ public static CngKey Open(string keyName, CngProvider provider, CngKeyOpenOption SafeNCryptKeyHandle keyHandle; ErrorCode errorCode = Interop.NCrypt.NCryptOpenKey(providerHandle, out keyHandle, keyName, 0, openOptions); if (errorCode != ErrorCode.ERROR_SUCCESS) + { + keyHandle.Dispose(); + providerHandle.Dispose(); throw errorCode.ToCryptographicException(); + } return new CngKey(providerHandle, keyHandle); } diff --git a/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/CryptoConfigForwarder.cs b/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/CryptoConfigForwarder.cs index 803cfc85aac75..f7da98e1e7905 100644 --- a/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/CryptoConfigForwarder.cs +++ b/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/CryptoConfigForwarder.cs @@ -41,7 +41,19 @@ internal static class CryptoConfigForwarder } [RequiresUnreferencedCode(CreateFromNameUnreferencedCodeMessage)] - internal static object? CreateFromName(string name) => s_createFromName(name); + internal static T? CreateFromName(string name) where T : class + { + object? o = s_createFromName(name); + try + { + return (T?)o; + } + catch + { + (o as IDisposable)?.Dispose(); + throw; + } + } internal static HashAlgorithm CreateDefaultHashAlgorithm() => throw new PlatformNotSupportedException(SR.Cryptography_DefaultAlgorithm_NotSupported); diff --git a/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/DSACng.ImportExport.cs b/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/DSACng.ImportExport.cs index a3b45627f6d56..7ac9a0f118dfa 100644 --- a/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/DSACng.ImportExport.cs +++ b/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/DSACng.ImportExport.cs @@ -23,14 +23,29 @@ private void ImportKeyBlob(byte[] dsaBlob, bool includePrivate) CngKeyBlobFormat.GenericPublicBlob; CngKey newKey = CngKey.Import(dsaBlob, blobFormat); - newKey.ExportPolicy |= CngExportPolicies.AllowPlaintextExport; - - Key = newKey; + try + { + newKey.ExportPolicy |= CngExportPolicies.AllowPlaintextExport; + Key = newKey; + } + catch + { + newKey.Dispose(); + throw; + } } private void AcceptImport(CngPkcs8.Pkcs8Response response) { - Key = response.Key; + try + { + Key = response.Key; + } + catch + { + response.FreeKey(); + throw; + } } public override bool TryExportPkcs8PrivateKey(Span destination, out int bytesWritten) diff --git a/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/ECDiffieHellmanCng.cs b/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/ECDiffieHellmanCng.cs index 5f75ab78519f2..b39124e81e085 100644 --- a/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/ECDiffieHellmanCng.cs +++ b/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/ECDiffieHellmanCng.cs @@ -144,12 +144,30 @@ private void DisposeKey() private void ImportFullKeyBlob(byte[] ecfullKeyBlob, bool includePrivateParameters) { - Key = ECCng.ImportFullKeyBlob(ecfullKeyBlob, includePrivateParameters); + CngKey newKey = ECCng.ImportFullKeyBlob(ecfullKeyBlob, includePrivateParameters); + try + { + Key = newKey; + } + catch + { + newKey.Dispose(); + throw; + } } private void ImportKeyBlob(byte[] ecfullKeyBlob, string curveName, bool includePrivateParameters) { - Key = ECCng.ImportKeyBlob(ecfullKeyBlob, curveName, includePrivateParameters); + CngKey newKey = ECCng.ImportKeyBlob(ecfullKeyBlob, curveName, includePrivateParameters); + try + { + Key = newKey; + } + catch + { + newKey.Dispose(); + throw; + } } private byte[] ExportKeyBlob(bool includePrivateParameters) @@ -164,7 +182,15 @@ private byte[] ExportFullKeyBlob(bool includePrivateParameters) private void AcceptImport(CngPkcs8.Pkcs8Response response) { - Key = response.Key; + try + { + Key = response.Key; + } + catch + { + response.FreeKey(); + throw; + } } public override bool TryExportPkcs8PrivateKey(Span destination, out int bytesWritten) diff --git a/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/ECDsa.Create.SecurityTransforms.cs b/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/ECDsa.Create.SecurityTransforms.cs index 5dcbbde048c24..0aa9a1b71b8e0 100644 --- a/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/ECDsa.Create.SecurityTransforms.cs +++ b/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/ECDsa.Create.SecurityTransforms.cs @@ -22,7 +22,16 @@ public partial class ECDsa : ECAlgorithm public static partial ECDsa Create(ECCurve curve) { ECDsa ecdsa = Create(); - ecdsa.GenerateKey(curve); + try + { + ecdsa.GenerateKey(curve); + } + catch + { + ecdsa.Dispose(); + throw; + } + return ecdsa; } @@ -35,7 +44,16 @@ public static partial ECDsa Create(ECCurve curve) public static partial ECDsa Create(ECParameters parameters) { ECDsa ecdsa = Create(); - ecdsa.ImportParameters(parameters); + try + { + ecdsa.ImportParameters(parameters); + } + catch + { + ecdsa.Dispose(); + throw; + } + return ecdsa; } } diff --git a/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/ECDsaCng.cs b/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/ECDsaCng.cs index ad50b1b33896c..76d8ce0520ae3 100644 --- a/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/ECDsaCng.cs +++ b/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/ECDsaCng.cs @@ -80,12 +80,30 @@ private static bool IsEccAlgorithmGroup(CngAlgorithmGroup? algorithmGroup) private void ImportFullKeyBlob(byte[] ecfullKeyBlob, bool includePrivateParameters) { - Key = ECCng.ImportFullKeyBlob(ecfullKeyBlob, includePrivateParameters); + CngKey key = ECCng.ImportFullKeyBlob(ecfullKeyBlob, includePrivateParameters); + try + { + Key = key; + } + catch + { + key.Dispose(); + throw; + } } private void ImportKeyBlob(byte[] ecfullKeyBlob, string curveName, bool includePrivateParameters) { - Key = ECCng.ImportKeyBlob(ecfullKeyBlob, curveName, includePrivateParameters); + CngKey key = ECCng.ImportKeyBlob(ecfullKeyBlob, curveName, includePrivateParameters); + try + { + Key = key; + } + catch + { + key.Dispose(); + throw; + } } private byte[] ExportKeyBlob(bool includePrivateParameters) @@ -100,7 +118,15 @@ private byte[] ExportFullKeyBlob(bool includePrivateParameters) private void AcceptImport(CngPkcs8.Pkcs8Response response) { - Key = response.Key; + try + { + Key = response.Key; + } + catch + { + response.FreeKey(); + throw; + } } public override bool TryExportPkcs8PrivateKey(Span destination, out int bytesWritten) diff --git a/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/HMAC.cs b/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/HMAC.cs index 468d3ac860344..d542ba3a5cf32 100644 --- a/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/HMAC.cs +++ b/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/HMAC.cs @@ -25,7 +25,7 @@ protected HMAC() { } [Obsolete(Obsoletions.CryptoStringFactoryMessage, DiagnosticId = Obsoletions.CryptoStringFactoryDiagId, UrlFormat = Obsoletions.SharedUrlFormat)] [RequiresUnreferencedCode(CryptoConfigForwarder.CreateFromNameUnreferencedCodeMessage)] public static new HMAC? Create(string algorithmName) => - (HMAC?)CryptoConfigForwarder.CreateFromName(algorithmName); + CryptoConfigForwarder.CreateFromName(algorithmName); public string HashName { diff --git a/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/HashAlgorithm.cs b/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/HashAlgorithm.cs index 9a032e96ead70..add544fafff49 100644 --- a/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/HashAlgorithm.cs +++ b/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/HashAlgorithm.cs @@ -25,7 +25,7 @@ public static HashAlgorithm Create() => [Obsolete(Obsoletions.CryptoStringFactoryMessage, DiagnosticId = Obsoletions.CryptoStringFactoryDiagId, UrlFormat = Obsoletions.SharedUrlFormat)] [RequiresUnreferencedCode(CryptoConfigForwarder.CreateFromNameUnreferencedCodeMessage)] public static HashAlgorithm? Create(string hashName) => - (HashAlgorithm?)CryptoConfigForwarder.CreateFromName(hashName); + CryptoConfigForwarder.CreateFromName(hashName); public virtual int HashSize => HashSizeValue; diff --git a/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/KeyedHashAlgorithm.cs b/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/KeyedHashAlgorithm.cs index dcfd0a4a683e7..7217e96d6b55e 100644 --- a/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/KeyedHashAlgorithm.cs +++ b/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/KeyedHashAlgorithm.cs @@ -17,7 +17,7 @@ protected KeyedHashAlgorithm() { } [Obsolete(Obsoletions.CryptoStringFactoryMessage, DiagnosticId = Obsoletions.CryptoStringFactoryDiagId, UrlFormat = Obsoletions.SharedUrlFormat)] [RequiresUnreferencedCode(CryptoConfigForwarder.CreateFromNameUnreferencedCodeMessage)] public static new KeyedHashAlgorithm? Create(string algName) => - (KeyedHashAlgorithm?)CryptoConfigForwarder.CreateFromName(algName); + CryptoConfigForwarder.CreateFromName(algName); public virtual byte[] Key { diff --git a/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/RSACng.ImportExport.cs b/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/RSACng.ImportExport.cs index 79a6fbbc238c4..b656c41fa82b1 100644 --- a/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/RSACng.ImportExport.cs +++ b/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/RSACng.ImportExport.cs @@ -20,14 +20,29 @@ private void ImportKeyBlob(byte[] rsaBlob, bool includePrivate) CngKeyBlobFormat blobFormat = includePrivate ? s_rsaPrivateBlob : s_rsaPublicBlob; CngKey newKey = CngKey.Import(rsaBlob, blobFormat); - newKey.ExportPolicy |= CngExportPolicies.AllowPlaintextExport; - - Key = newKey; + try + { + newKey.ExportPolicy |= CngExportPolicies.AllowPlaintextExport; + Key = newKey; + } + catch + { + newKey.Dispose(); + throw; + } } private void AcceptImport(CngPkcs8.Pkcs8Response response) { - Key = response.Key; + try + { + Key = response.Key; + } + catch + { + response.FreeKey(); + throw; + } } private byte[] ExportKeyBlob(bool includePrivateParameters) diff --git a/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/SafeEvpPKeyHandle.OpenSsl.cs b/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/SafeEvpPKeyHandle.OpenSsl.cs index 47b6ba4812e54..99c0f42106db8 100644 --- a/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/SafeEvpPKeyHandle.OpenSsl.cs +++ b/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/SafeEvpPKeyHandle.OpenSsl.cs @@ -62,7 +62,9 @@ public SafeEvpPKeyHandle DuplicateHandle() if (success != 1) { Debug.Fail("Called UpRefEvpPkey on a key which was already marked for destruction"); - throw Interop.Crypto.CreateOpenSslCryptographicException(); + Exception e = Interop.Crypto.CreateOpenSslCryptographicException(); + safeHandle.Dispose(); + throw e; } // Since we didn't actually create a new handle, copy the handle diff --git a/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/SymmetricAlgorithm.cs b/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/SymmetricAlgorithm.cs index f364e4f4db988..f9f200ff47b92 100644 --- a/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/SymmetricAlgorithm.cs +++ b/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/SymmetricAlgorithm.cs @@ -22,7 +22,7 @@ public static SymmetricAlgorithm Create() => [Obsolete(Obsoletions.CryptoStringFactoryMessage, DiagnosticId = Obsoletions.CryptoStringFactoryDiagId, UrlFormat = Obsoletions.SharedUrlFormat)] [RequiresUnreferencedCode(CryptoConfigForwarder.CreateFromNameUnreferencedCodeMessage)] public static SymmetricAlgorithm? Create(string algName) => - (SymmetricAlgorithm?)CryptoConfigForwarder.CreateFromName(algName); + CryptoConfigForwarder.CreateFromName(algName); public virtual int FeedbackSize { diff --git a/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/X509Certificates/CertificatePal.Windows.Import.cs b/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/X509Certificates/CertificatePal.Windows.Import.cs index e79e50136887e..794fa64b22a25 100644 --- a/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/X509Certificates/CertificatePal.Windows.Import.cs +++ b/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/X509Certificates/CertificatePal.Windows.Import.cs @@ -133,7 +133,12 @@ private static unsafe SafeCertContextHandle GetSignerInPKCS7Store(SafeCertStoreH SafeCertContextHandle? pCertContext = null; if (!Interop.crypt32.CertFindCertificateInStore(hCertStore, Interop.Crypt32.CertFindType.CERT_FIND_SUBJECT_CERT, &certInfo, ref pCertContext)) - throw Marshal.GetHRForLastWin32Error().ToCryptographicException(); + { + Exception e = Marshal.GetHRForLastWin32Error().ToCryptographicException(); + pCertContext.Dispose(); + throw e; + } + return pCertContext; } } @@ -152,7 +157,9 @@ private static SafeCertContextHandle FilterPFXStore( hStore = Interop.Crypt32.PFXImportCertStore(ref certBlob, password, pfxCertStoreFlags); if (hStore.IsInvalid) { - throw Marshal.GetHRForLastWin32Error().ToCryptographicException(); + Exception e = Marshal.GetHRForLastWin32Error().ToCryptographicException(); + hStore.Dispose(); + throw e; } } } diff --git a/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/X509Certificates/CertificatePal.Windows.PrivateKey.cs b/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/X509Certificates/CertificatePal.Windows.PrivateKey.cs index a93817fac81cd..71d9882dc8f6c 100644 --- a/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/X509Certificates/CertificatePal.Windows.PrivateKey.cs +++ b/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/X509Certificates/CertificatePal.Windows.PrivateKey.cs @@ -385,8 +385,9 @@ public ICertificatePal CopyWithPrivateKey(RSA rsa) Interop.Crypt32.CertSetPropertyFlags.None, &keyProvInfo)) { + Exception e = Marshal.GetLastWin32Error().ToCryptographicException(); pal.Dispose(); - throw Marshal.GetLastWin32Error().ToCryptographicException(); + throw e; } } @@ -573,8 +574,9 @@ private static bool TryGuessDsaKeySpec(CspParameters cspParameters, out int keyS Interop.Crypt32.CertSetPropertyFlags.None, &keyProvInfo)) { + Exception e = Marshal.GetLastWin32Error().ToCryptographicException(); pal.Dispose(); - throw Marshal.GetLastWin32Error().ToCryptographicException(); + throw e; } } @@ -596,8 +598,9 @@ private ICertificatePal CopyWithEphemeralKey(CngKey cngKey) Interop.Crypt32.CertSetPropertyFlags.CERT_SET_PROPERTY_INHIBIT_PERSIST_FLAG, handle)) { + Exception e = Marshal.GetLastWin32Error().ToCryptographicException(); pal.Dispose(); - throw Marshal.GetLastWin32Error().ToCryptographicException(); + throw e; } // The value was transferred to the certificate. diff --git a/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/X509Certificates/CertificatePal.Windows.cs b/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/X509Certificates/CertificatePal.Windows.cs index f083e78cdf513..e3bb7dd984066 100644 --- a/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/X509Certificates/CertificatePal.Windows.cs +++ b/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/X509Certificates/CertificatePal.Windows.cs @@ -24,7 +24,11 @@ internal static partial ICertificatePal FromHandle(IntPtr handle) SafeCertContextHandle safeCertContextHandle = Interop.Crypt32.CertDuplicateCertificateContext(handle); if (safeCertContextHandle.IsInvalid) - throw ErrorCode.HRESULT_INVALID_HANDLE.ToCryptographicException(); + { + Exception e = ErrorCode.HRESULT_INVALID_HANDLE.ToCryptographicException(); + safeCertContextHandle.Dispose(); + throw e; + } int cbData = 0; bool deleteKeyContainer = Interop.Crypt32.CertGetCertificateContextProperty(safeCertContextHandle, Interop.Crypt32.CertContextPropId.CERT_CLR_DELETE_KEY_PROP_ID, out Interop.Crypt32.DATA_BLOB _, ref cbData); diff --git a/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/X509Certificates/ChainPal.Windows.cs b/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/X509Certificates/ChainPal.Windows.cs index 4682b66e0ef0d..511f65d4e3980 100644 --- a/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/X509Certificates/ChainPal.Windows.cs +++ b/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/X509Certificates/ChainPal.Windows.cs @@ -25,7 +25,10 @@ internal static partial IChainPal FromHandle(IntPtr chainContext) SafeX509ChainHandle certChainHandle = Interop.Crypt32.CertDuplicateCertificateChain(chainContext); if (certChainHandle == null || certChainHandle.IsInvalid) + { + certChainHandle?.Dispose(); throw new CryptographicException(SR.Cryptography_InvalidContextHandle, nameof(chainContext)); + } var pal = new ChainPal(certChainHandle); return pal; diff --git a/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/X509Certificates/FindPal.Windows.cs b/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/X509Certificates/FindPal.Windows.cs index 2b672f55fd91b..0308d291b86e4 100644 --- a/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/X509Certificates/FindPal.Windows.cs +++ b/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/X509Certificates/FindPal.Windows.cs @@ -395,7 +395,9 @@ private unsafe void FindCore(Interop.Crypt32.CertFindType dwFindType, vo if (findResults.IsInvalid) { - throw Marshal.GetHRForLastWin32Error().ToCryptographicException(); + Exception e = Marshal.GetHRForLastWin32Error().ToCryptographicException(); + findResults.Dispose(); + throw e; } SafeCertContextHandle? pCertContext = null; diff --git a/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/X509Certificates/OpenSslCertificateAssetDownloader.cs b/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/X509Certificates/OpenSslCertificateAssetDownloader.cs index 87ccab4924bca..2abdf8a791c86 100644 --- a/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/X509Certificates/OpenSslCertificateAssetDownloader.cs +++ b/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/X509Certificates/OpenSslCertificateAssetDownloader.cs @@ -56,6 +56,8 @@ internal static class OpenSslCertificateAssetDownloader return handle; } + handle.Dispose(); + using (SafeBioHandle bio = Interop.Crypto.CreateMemoryBio()) { Interop.Crypto.CheckValidOpenSslHandle(bio); @@ -72,6 +74,8 @@ internal static class OpenSslCertificateAssetDownloader { return handle; } + + handle.Dispose(); } if (OpenSslX509ChainEventSource.Log.IsEnabled()) diff --git a/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/X509Certificates/StorePal.Windows.Import.cs b/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/X509Certificates/StorePal.Windows.Import.cs index 264e1bf11fb6d..175f5874e6e39 100644 --- a/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/X509Certificates/StorePal.Windows.Import.cs +++ b/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/X509Certificates/StorePal.Windows.Import.cs @@ -55,7 +55,9 @@ private static StorePal FromBlobOrFile(ReadOnlySpan rawData, string? fileN IntPtr.Zero )) { - throw Marshal.GetLastWin32Error().ToCryptographicException(); + Exception e = Marshal.GetLastWin32Error().ToCryptographicException(); + certStore.Dispose(); + throw e; } if (contentType == Interop.Crypt32.ContentType.CERT_QUERY_CONTENT_PFX) @@ -71,7 +73,11 @@ private static StorePal FromBlobOrFile(ReadOnlySpan rawData, string? fileN Interop.Crypt32.DATA_BLOB blob2 = new Interop.Crypt32.DATA_BLOB(new IntPtr(pRawData2), (uint)rawData!.Length); certStore = Interop.Crypt32.PFXImportCertStore(ref blob2, password, certStoreFlags); if (certStore == null || certStore.IsInvalid) - throw Marshal.GetLastWin32Error().ToCryptographicException(); + { + Exception e = Marshal.GetLastWin32Error().ToCryptographicException(); + certStore?.Dispose(); + throw e; + } } if (!persistKeySet) @@ -86,7 +92,11 @@ private static StorePal FromBlobOrFile(ReadOnlySpan rawData, string? fileN { Interop.Crypt32.DATA_BLOB nullBlob = new Interop.Crypt32.DATA_BLOB(IntPtr.Zero, 0); if (!Interop.Crypt32.CertSetCertificateContextProperty(pCertContext, Interop.Crypt32.CertContextPropId.CERT_CLR_DELETE_KEY_PROP_ID, Interop.Crypt32.CertSetPropertyFlags.CERT_SET_PROPERTY_INHIBIT_PERSIST_FLAG, &nullBlob)) - throw Marshal.GetLastWin32Error().ToCryptographicException(); + { + Exception e = Marshal.GetLastWin32Error().ToCryptographicException(); + certStore.Dispose(); + throw e; + } } } } @@ -107,10 +117,15 @@ internal static partial IExportPal FromCertificate(ICertificatePalCore cert) IntPtr.Zero, Interop.Crypt32.CertStoreFlags.CERT_STORE_ENUM_ARCHIVED_FLAG | Interop.Crypt32.CertStoreFlags.CERT_STORE_CREATE_NEW_FLAG | Interop.Crypt32.CertStoreFlags.CERT_STORE_DEFER_CLOSE_UNTIL_LAST_FREE_FLAG, null); - if (certStore.IsInvalid) - throw Marshal.GetHRForLastWin32Error().ToCryptographicException(); - if (!Interop.Crypt32.CertAddCertificateLinkToStore(certStore, certificatePal.CertContext, Interop.Crypt32.CertStoreAddDisposition.CERT_STORE_ADD_ALWAYS, IntPtr.Zero)) - throw Marshal.GetHRForLastWin32Error().ToCryptographicException(); + + if (certStore.IsInvalid || + !Interop.Crypt32.CertAddCertificateLinkToStore(certStore, certificatePal.CertContext, Interop.Crypt32.CertStoreAddDisposition.CERT_STORE_ADD_ALWAYS, IntPtr.Zero)) + { + Exception e = Marshal.GetHRForLastWin32Error().ToCryptographicException(); + certStore?.Dispose(); + throw e; + } + return new StorePal(certStore); } @@ -130,7 +145,11 @@ internal static partial IExportPal LinkFromCertificateCollection(X509Certificate Interop.Crypt32.CertStoreFlags.CERT_STORE_ENUM_ARCHIVED_FLAG | Interop.Crypt32.CertStoreFlags.CERT_STORE_CREATE_NEW_FLAG, null); if (certStore.IsInvalid) - throw Marshal.GetHRForLastWin32Error().ToCryptographicException(); + { + Exception e = Marshal.GetHRForLastWin32Error().ToCryptographicException(); + certStore?.Dispose(); + throw e; + } // // We use CertAddCertificateLinkToStore to keep a link to the original store, so any property changes get @@ -141,7 +160,11 @@ internal static partial IExportPal LinkFromCertificateCollection(X509Certificate { SafeCertContextHandle certContext = ((CertificatePal)certificates[i].Pal!).CertContext; if (!Interop.Crypt32.CertAddCertificateLinkToStore(certStore, certContext, Interop.Crypt32.CertStoreAddDisposition.CERT_STORE_ADD_ALWAYS, IntPtr.Zero)) - throw Marshal.GetLastWin32Error().ToCryptographicException(); + { + Exception e = Marshal.GetLastWin32Error().ToCryptographicException(); + certStore.Dispose(); + throw e; + } } return new StorePal(certStore); @@ -153,7 +176,11 @@ internal static partial IStorePal FromSystemStore(string storeName, StoreLocatio SafeCertStoreHandle certStore = Interop.crypt32.CertOpenStore(CertStoreProvider.CERT_STORE_PROV_SYSTEM_W, Interop.Crypt32.CertEncodingType.All, IntPtr.Zero, certStoreFlags, storeName); if (certStore.IsInvalid) - throw Marshal.GetLastWin32Error().ToCryptographicException(); + { + Exception e = Marshal.GetLastWin32Error().ToCryptographicException(); + certStore.Dispose(); + throw e; + } // // We want the store to auto-resync when requesting a snapshot so that diff --git a/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/X509Certificates/StorePal.Windows.cs b/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/X509Certificates/StorePal.Windows.cs index f8e871e76fee0..7e1a7c02548bc 100644 --- a/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/X509Certificates/StorePal.Windows.cs +++ b/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/X509Certificates/StorePal.Windows.cs @@ -21,7 +21,10 @@ internal static partial IStorePal FromHandle(IntPtr storeHandle) SafeCertStoreHandle certStoreHandle = Interop.Crypt32.CertDuplicateStore(storeHandle); if (certStoreHandle == null || certStoreHandle.IsInvalid) + { + certStoreHandle?.Dispose(); throw new CryptographicException(SR.Cryptography_InvalidStoreHandle, nameof(storeHandle)); + } var pal = new StorePal(certStoreHandle); return pal; diff --git a/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/X509Certificates/WindowsInterop.crypt32.cs b/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/X509Certificates/WindowsInterop.crypt32.cs index b53cd207b91d7..16e52257e95a4 100644 --- a/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/X509Certificates/WindowsInterop.crypt32.cs +++ b/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/X509Certificates/WindowsInterop.crypt32.cs @@ -133,8 +133,9 @@ internal static SafeChainEngineHandle CertCreateCertificateChainEngine(ref Inter { if (!Interop.Crypt32.CertCreateCertificateChainEngine(ref config, out SafeChainEngineHandle chainEngineHandle)) { - int errorCode = Marshal.GetLastWin32Error(); - throw errorCode.ToCryptographicException(); + Exception e = Marshal.GetLastWin32Error().ToCryptographicException(); + chainEngineHandle.Dispose(); + throw e; } return chainEngineHandle; diff --git a/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/X509Certificates/X509Pal.Apple.ECKey.cs b/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/X509Certificates/X509Pal.Apple.ECKey.cs index df21ef3ceaa58..8d1696721b1a4 100644 --- a/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/X509Certificates/X509Pal.Apple.ECKey.cs +++ b/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/X509Certificates/X509Pal.Apple.ECKey.cs @@ -35,8 +35,10 @@ private static SafeSecKeyRefHandle DecodeECPublicKey(ICertificatePal? certificat // algorithm in the test suite (as of macOS Mojave Developer Preview 4). if (key.IsInvalid) { + key.Dispose(); throw Interop.AppleCrypto.CreateExceptionForOSStatus(errSecInvalidKeyRef); } + // EccGetKeySizeInBits can fail for two reasons. First, the Apple implementation has changed // and we receive values from API that were not previously handled. In that case the // implementation will need to be adjusted to handle these values. Second, we deliberately diff --git a/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/X509Certificates/X509Pal.Windows.PublicKey.cs b/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/X509Certificates/X509Pal.Windows.PublicKey.cs index 20ac22206124b..886554666613b 100644 --- a/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/X509Certificates/X509Pal.Windows.PublicKey.cs +++ b/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/X509Certificates/X509Pal.Windows.PublicKey.cs @@ -131,7 +131,12 @@ private static SafeBCryptKeyHandle ImportPublicKeyInfo(SafeCertContextHandle cer { bool success = Interop.Crypt32.CryptImportPublicKeyInfoEx2(Interop.Crypt32.CertEncodingType.X509_ASN_ENCODING, &(certContext.CertContext->pCertInfo->SubjectPublicKeyInfo), importFlags, null, out bCryptKeyHandle); if (!success) - throw Marshal.GetHRForLastWin32Error().ToCryptographicException(); + { + Exception e = Marshal.GetHRForLastWin32Error().ToCryptographicException(); + bCryptKeyHandle.Dispose(); + throw e; + } + return bCryptKeyHandle; } } diff --git a/src/libraries/System.Security.Cryptography/tests/CryptoConfigTests.cs b/src/libraries/System.Security.Cryptography/tests/CryptoConfigTests.cs index e0fb63c6815cd..921b9bab3c4ca 100644 --- a/src/libraries/System.Security.Cryptography/tests/CryptoConfigTests.cs +++ b/src/libraries/System.Security.Cryptography/tests/CryptoConfigTests.cs @@ -317,12 +317,13 @@ public static void StaticCreateMethods() VerifyStaticCreateResult(SHA384.Create(typeof(SHA384Managed).FullName), typeof(SHA384Managed)); VerifyStaticCreateResult(SHA512.Create(typeof(SHA512Managed).FullName), typeof(SHA512Managed)); #pragma warning restore SYSLIB0022 // Rijndael types are obsolete - } - private static void VerifyStaticCreateResult(object obj, Type expectedType) - { - Assert.NotNull(obj); - Assert.IsType(expectedType, obj); + static void VerifyStaticCreateResult(object obj, Type expectedType) + { + Assert.NotNull(obj); + Assert.IsType(expectedType, obj); + (obj as IDisposable)?.Dispose(); + } } [Fact] diff --git a/src/libraries/System.Security.Cryptography/tests/CryptoStream.cs b/src/libraries/System.Security.Cryptography/tests/CryptoStream.cs index 6e877fbf5a027..fb5fca6dd1307 100644 --- a/src/libraries/System.Security.Cryptography/tests/CryptoStream.cs +++ b/src/libraries/System.Security.Cryptography/tests/CryptoStream.cs @@ -302,14 +302,16 @@ public static void PaddedAes_PartialRead_Success() aes.Key = aes.IV = new byte[] { 0x0, 0x1, 0x2, 0x3, 0x4, 0x5, 0x6, 0x7, 0x8, 0x9, 0xA, 0xB, 0xC, 0xD, 0xE, 0xF, }; var memoryStream = new MemoryStream(); - using (var cryptoStream = new CryptoStream(memoryStream, aes.CreateEncryptor(), CryptoStreamMode.Write, leaveOpen: true)) + using (ICryptoTransform encryptor = aes.CreateEncryptor()) + using (var cryptoStream = new CryptoStream(memoryStream, encryptor, CryptoStreamMode.Write, leaveOpen: true)) { cryptoStream.Write("Sample string that's bigger than cryptoAlg.BlockSize"u8); cryptoStream.FlushFinalBlock(); } memoryStream.Position = 0; - using (var cryptoStream = new CryptoStream(memoryStream, aes.CreateDecryptor(), CryptoStreamMode.Read)) + using (ICryptoTransform decryptor = aes.CreateDecryptor()) + using (var cryptoStream = new CryptoStream(memoryStream, decryptor, CryptoStreamMode.Read)) { cryptoStream.ReadByte(); // Partially read the CryptoStream before disposing it. } diff --git a/src/libraries/System.Security.Cryptography/tests/DSATests.cs b/src/libraries/System.Security.Cryptography/tests/DSATests.cs index aea5a05b84ac6..b995a5e092089 100644 --- a/src/libraries/System.Security.Cryptography/tests/DSATests.cs +++ b/src/libraries/System.Security.Cryptography/tests/DSATests.cs @@ -159,7 +159,12 @@ private sealed class OverrideAbstractDSA : DSA private readonly DSA _dsa; public OverrideAbstractDSA(DSA dsa) => _dsa = dsa; - protected override void Dispose(bool disposing) => _dsa.Dispose(); + + protected override void Dispose(bool disposing) + { + _dsa.Dispose(); + base.Dispose(disposing); + } public override byte[] CreateSignature(byte[] rgbHash) => _dsa.CreateSignature(rgbHash); public override DSAParameters ExportParameters(bool includePrivateParameters) => _dsa.ExportParameters(includePrivateParameters); diff --git a/src/libraries/System.Security.Cryptography/tests/ECDsaTests.cs b/src/libraries/System.Security.Cryptography/tests/ECDsaTests.cs index ae1a937f8a38d..5a871f35c2ef0 100644 --- a/src/libraries/System.Security.Cryptography/tests/ECDsaTests.cs +++ b/src/libraries/System.Security.Cryptography/tests/ECDsaTests.cs @@ -146,6 +146,12 @@ private sealed class OverrideAbstractECDsa : ECDsa public OverrideAbstractECDsa(ECDsa ecdsa) => _ecdsa = ecdsa; + protected override void Dispose(bool disposing) + { + _ecdsa.Dispose(); + base.Dispose(disposing); + } + public override int KeySize { get => _ecdsa.KeySize; diff --git a/src/libraries/System.Security.Cryptography/tests/HmacTests.cs b/src/libraries/System.Security.Cryptography/tests/HmacTests.cs index 3590ab90c5044..436f407c8b81b 100644 --- a/src/libraries/System.Security.Cryptography/tests/HmacTests.cs +++ b/src/libraries/System.Security.Cryptography/tests/HmacTests.cs @@ -236,7 +236,11 @@ protected void VerifyHmacRfc2104_2() byte[] overSizedKey = new byte[BlockSize + 1]; hmac.Key = overSizedKey; byte[] actualKey = hmac.Key; - byte[] expectedKey = CreateHashAlgorithm().ComputeHash(overSizedKey); + byte[] expectedKey; + using (HashAlgorithm hash = CreateHashAlgorithm()) + { + expectedKey = hash.ComputeHash(overSizedKey); + } Assert.Equal(expectedKey, actualKey); // Also ensure that the hashing operation uses the adjusted key. diff --git a/src/libraries/System.Security.Cryptography/tests/Rfc2898Tests.cs b/src/libraries/System.Security.Cryptography/tests/Rfc2898Tests.cs index 8c3785bca7a3f..aa2a0aa281a13 100644 --- a/src/libraries/System.Security.Cryptography/tests/Rfc2898Tests.cs +++ b/src/libraries/System.Security.Cryptography/tests/Rfc2898Tests.cs @@ -188,7 +188,7 @@ public static void GetBytes_ZeroLength() [Fact] public static void GetBytes_NegativeLength() { - Rfc2898DeriveBytes deriveBytes = new Rfc2898DeriveBytes(TestPassword, s_testSalt, DefaultIterationCount, HashAlgorithmName.SHA1); + using Rfc2898DeriveBytes deriveBytes = new Rfc2898DeriveBytes(TestPassword, s_testSalt, DefaultIterationCount, HashAlgorithmName.SHA1); Assert.Throws(() => deriveBytes.GetBytes(-1)); Assert.Throws(() => deriveBytes.GetBytes(int.MinValue)); Assert.Throws(() => deriveBytes.GetBytes(int.MinValue / 2)); diff --git a/src/libraries/System.Security.Cryptography/tests/SignatureDescriptionTests.cs b/src/libraries/System.Security.Cryptography/tests/SignatureDescriptionTests.cs index 2d222ea5f92a6..6568c373a4690 100644 --- a/src/libraries/System.Security.Cryptography/tests/SignatureDescriptionTests.cs +++ b/src/libraries/System.Security.Cryptography/tests/SignatureDescriptionTests.cs @@ -133,45 +133,24 @@ public void Deformatter() public void Digest() { bool rightClass = false; - HashAlgorithm hash = null; SignatureDescription sig = new SignatureDescription(); // null hash - AssertExtensions.Throws("name", () => hash = sig.CreateDigest()); - - sig.DigestAlgorithm = "SHA1"; - hash = sig.CreateDigest(); - Assert.NotNull(hash); - rightClass = (hash.ToString().IndexOf(sig.DigestAlgorithm) > 0); - Assert.True(rightClass, "CreateDigest(SHA1)"); - - sig.DigestAlgorithm = "MD5"; - hash = sig.CreateDigest(); - Assert.NotNull(hash); - rightClass = (hash.ToString().IndexOf(sig.DigestAlgorithm) > 0); - Assert.True(rightClass, "CreateDigest(MD5)"); - - sig.DigestAlgorithm = "SHA256"; - hash = sig.CreateDigest(); - Assert.NotNull(hash); - rightClass = (hash.ToString().IndexOf(sig.DigestAlgorithm) > 0); - Assert.True(rightClass, "CreateDigest(SHA256)"); - - sig.DigestAlgorithm = "SHA384"; - hash = sig.CreateDigest(); - Assert.NotNull(hash); - rightClass = (hash.ToString().IndexOf(sig.DigestAlgorithm) > 0); - Assert.True(rightClass, "CreateDigest(SHA384)"); - - sig.DigestAlgorithm = "SHA512"; - hash = sig.CreateDigest(); - Assert.NotNull(hash); - rightClass = (hash.ToString().IndexOf(sig.DigestAlgorithm) > 0); - Assert.True(rightClass, "CreateDigest(SHA512)"); + AssertExtensions.Throws("name", () => sig.CreateDigest()); + + foreach (string name in new[] { "SHA1", "MD5", "SHA256", "SHA384", "SHA512" }) + { + sig.DigestAlgorithm = name; + using (HashAlgorithm hash = sig.CreateDigest()) + { + Assert.NotNull(hash); + rightClass = hash.ToString().IndexOf(sig.DigestAlgorithm) > 0; + Assert.True(rightClass, $"CreateDigest({name})"); + } + } sig.DigestAlgorithm = "bad"; - hash = sig.CreateDigest(); - Assert.Null(hash); + Assert.Null(sig.CreateDigest()); } [Fact] diff --git a/src/libraries/System.Security.Cryptography/tests/TripleDesTests.cs b/src/libraries/System.Security.Cryptography/tests/TripleDesTests.cs index f12e0b1cf510e..729f6328265e5 100644 --- a/src/libraries/System.Security.Cryptography/tests/TripleDesTests.cs +++ b/src/libraries/System.Security.Cryptography/tests/TripleDesTests.cs @@ -68,7 +68,8 @@ public static void TripleDesCreate() byte[] encryptedBytes; using (MemoryStream input = new MemoryStream(inputBytes)) - using (CryptoStream cryptoStream = new CryptoStream(input, tripleDes.CreateEncryptor(), CryptoStreamMode.Read)) + using (ICryptoTransform encryptor = tripleDes.CreateEncryptor()) + using (CryptoStream cryptoStream = new CryptoStream(input, encryptor, CryptoStreamMode.Read)) using (MemoryStream output = new MemoryStream()) { cryptoStream.CopyTo(output); @@ -79,7 +80,8 @@ public static void TripleDesCreate() byte[] decryptedBytes; using (MemoryStream input = new MemoryStream(encryptedBytes)) - using (CryptoStream cryptoStream = new CryptoStream(input, tripleDes.CreateDecryptor(), CryptoStreamMode.Read)) + using (ICryptoTransform decryptor = tripleDes.CreateDecryptor()) + using (CryptoStream cryptoStream = new CryptoStream(input, decryptor, CryptoStreamMode.Read)) using (MemoryStream output = new MemoryStream()) { cryptoStream.CopyTo(output); diff --git a/src/libraries/System.Security.Principal.Windows/src/System/Security/Principal/Win32.cs b/src/libraries/System.Security.Principal.Windows/src/System/Security/Principal/Win32.cs index af9e86c008036..b92f4e4d3e436 100644 --- a/src/libraries/System.Security.Principal.Windows/src/System/Security/Principal/Win32.cs +++ b/src/libraries/System.Security.Principal.Windows/src/System/Security/Principal/Win32.cs @@ -28,7 +28,10 @@ internal static SafeLsaPolicyHandle LsaOpenPolicy( { return policyHandle; } - else if (error == Interop.StatusOptions.STATUS_ACCESS_DENIED) + + policyHandle.Dispose(); + + if (error == Interop.StatusOptions.STATUS_ACCESS_DENIED) { throw new UnauthorizedAccessException(); } diff --git a/src/libraries/System.Security.Principal.Windows/src/System/Security/Principal/WindowsIdentity.cs b/src/libraries/System.Security.Principal.Windows/src/System/Security/Principal/WindowsIdentity.cs index 13b0e460657c7..3749ab3e9fa59 100644 --- a/src/libraries/System.Security.Principal.Windows/src/System/Security/Principal/WindowsIdentity.cs +++ b/src/libraries/System.Security.Principal.Windows/src/System/Security/Principal/WindowsIdentity.cs @@ -45,7 +45,7 @@ public class WindowsIdentity : ClaimsIdentity, IDisposable, ISerializable, IDese private SecurityIdentifier? _user; private IdentityReferenceCollection? _groups; - private SafeAccessTokenHandle _safeTokenHandle = SafeAccessTokenHandle.InvalidHandle; + private SafeAccessTokenHandle _safeTokenHandle; private readonly string? _authType; private int _isAuthenticated = -1; private volatile TokenImpersonationLevel _impersonationLevel; @@ -77,21 +77,27 @@ protected WindowsIdentity(WindowsIdentity identity) try { - if (!identity._safeTokenHandle.IsInvalid && identity._safeTokenHandle != SafeAccessTokenHandle.InvalidHandle && identity._safeTokenHandle.DangerousGetHandle() != IntPtr.Zero) + if (!identity._safeTokenHandle.IsInvalid && identity._safeTokenHandle.DangerousGetHandle() != IntPtr.Zero) { identity._safeTokenHandle.DangerousAddRef(ref mustDecrement); if (!identity._safeTokenHandle.IsInvalid && identity._safeTokenHandle.DangerousGetHandle() != IntPtr.Zero) + { CreateFromToken(identity._safeTokenHandle.DangerousGetHandle()); + } _authType = identity._authType; _isAuthenticated = identity._isAuthenticated; } + + _safeTokenHandle ??= SafeAccessTokenHandle.InvalidHandle; } finally { if (mustDecrement) + { identity._safeTokenHandle.DangerousRelease(); + } } } @@ -105,7 +111,15 @@ private WindowsIdentity(IntPtr userToken, string? authType, int isAuthenticated) private WindowsIdentity() : base(null, null, null, ClaimTypes.Name, ClaimTypes.GroupSid) - { } + { + _safeTokenHandle = SafeAccessTokenHandle.InvalidHandle; + } + + private WindowsIdentity(SafeAccessTokenHandle safeTokenHandle) + : base(null, null, null, ClaimTypes.Name, ClaimTypes.GroupSid) + { + _safeTokenHandle = safeTokenHandle; + } /// /// Initializes a new instance of the WindowsIdentity class for the user represented by the specified User Principal Name (UPN). @@ -191,14 +205,23 @@ public WindowsIdentity(string sUserPrincipalName) out QUOTA_LIMITS quota, out int subStatus); + profileBuffer.Dispose(); + if (ntStatus == unchecked((int)Interop.StatusOptions.STATUS_ACCOUNT_RESTRICTION) && subStatus < 0) + { ntStatus = subStatus; + } + if (ntStatus < 0) // non-negative numbers indicate success + { + accessTokenHandle.Dispose(); throw GetExceptionFromNtStatus(ntStatus); + } if (subStatus < 0) // non-negative numbers indicate success + { + accessTokenHandle.Dispose(); throw GetExceptionFromNtStatus(subStatus); - - profileBuffer?.Dispose(); + } _safeTokenHandle = accessTokenHandle; } @@ -252,13 +275,13 @@ private static SafeAccessTokenHandle DuplicateAccessToken(IntPtr accessToken) throw new ArgumentException(SR.Argument_InvalidImpersonationToken); } - SafeAccessTokenHandle duplicateAccessToken = SafeAccessTokenHandle.InvalidHandle; + SafeAccessTokenHandle duplicateAccessToken; IntPtr currentProcessHandle = Interop.Kernel32.GetCurrentProcess(); if (!Interop.Kernel32.DuplicateHandle( currentProcessHandle, accessToken, currentProcessHandle, - ref duplicateAccessToken, + out duplicateAccessToken, 0, true, Interop.DuplicateHandleOptions.DUPLICATE_SAME_ACCESS)) @@ -291,6 +314,7 @@ private static SafeAccessTokenHandle DuplicateAccessToken(SafeAccessTokenHandle } } + [MemberNotNull(nameof(_safeTokenHandle))] private void CreateFromToken(IntPtr userToken) { _safeTokenHandle = DuplicateAccessToken(userToken); @@ -448,38 +472,35 @@ private bool CheckNtTokenForSid(SecurityIdentifier sid) return false; // CheckTokenMembership expects an impersonation token - SafeAccessTokenHandle token = SafeAccessTokenHandle.InvalidHandle; + using SafeAccessTokenHandle invalidHandle = SafeAccessTokenHandle.InvalidHandle; + SafeAccessTokenHandle token = invalidHandle; TokenImpersonationLevel til = ImpersonationLevel; bool isMember = false; try { - if (til == TokenImpersonationLevel.None) - { - if (!Interop.Advapi32.DuplicateTokenEx(_safeTokenHandle, + if (til == TokenImpersonationLevel.None && + !Interop.Advapi32.DuplicateTokenEx(_safeTokenHandle, (uint)TokenAccessLevels.Query, IntPtr.Zero, (uint)TokenImpersonationLevel.Identification, (uint)TokenType.TokenImpersonation, ref token)) - throw new SecurityException(Marshal.GetLastPInvokeErrorMessage()); + { + throw new SecurityException(Marshal.GetLastPInvokeErrorMessage()); } - // CheckTokenMembership will check if the SID is both present and enabled in the access token. if (!Interop.Advapi32.CheckTokenMembership((til != TokenImpersonationLevel.None ? _safeTokenHandle : token), sid.BinaryForm, ref isMember)) + { throw new SecurityException(Marshal.GetLastPInvokeErrorMessage()); - - + } } finally { - if (token != SafeAccessTokenHandle.InvalidHandle) - { - token.Dispose(); - } + token.Dispose(); } return isMember; @@ -559,7 +580,8 @@ internal string GetName() if (_name == null) { // revert thread impersonation for the duration of the call to get the name. - RunImpersonated(SafeAccessTokenHandle.InvalidHandle, delegate + using SafeAccessTokenHandle invalidHandle = SafeAccessTokenHandle.InvalidHandle; + RunImpersonated(invalidHandle, delegate { NTAccount? ntAccount = User!.Translate(typeof(NTAccount)) as NTAccount; _name = ntAccount!.ToString(); @@ -731,9 +753,19 @@ private static void RunImpersonatedInternal(SafeAccessTokenHandle token, Action SafeAccessTokenHandle previousToken = GetCurrentToken(TokenAccessLevels.MaximumAllowed, false, out bool isImpersonating, out int hr); if (previousToken == null || previousToken.IsInvalid) + { + previousToken?.Dispose(); throw new SecurityException(Marshal.GetPInvokeErrorMessage(hr)); + } - s_currentImpersonatedToken.Value = isImpersonating ? previousToken : null; + if (isImpersonating) + { + s_currentImpersonatedToken.Value = previousToken; + } + else + { + previousToken.Dispose(); + } ExecutionContext? currentContext = ExecutionContext.Capture(); @@ -778,16 +810,19 @@ private static void CurrentImpersonatedTokenChanged(AsyncLocalValueChangedArgs("sidType", () => new SecurityIdentifier(sidType, currentDomainSid)); } diff --git a/src/libraries/System.Security.Principal.Windows/tests/WindowsIdentityImpersonatedTests.netcoreapp.cs b/src/libraries/System.Security.Principal.Windows/tests/WindowsIdentityImpersonatedTests.netcoreapp.cs index 0a7c1745ed9f4..5dd25c38787cd 100644 --- a/src/libraries/System.Security.Principal.Windows/tests/WindowsIdentityImpersonatedTests.netcoreapp.cs +++ b/src/libraries/System.Security.Principal.Windows/tests/WindowsIdentityImpersonatedTests.netcoreapp.cs @@ -30,7 +30,7 @@ public WindowsIdentityImpersonatedTests(WindowsIdentityFixture windowsIdentityFi [OuterLoop] public async Task RunImpersonatedAsync_TaskAndTaskOfT() { - WindowsIdentity currentWindowsIdentity = WindowsIdentity.GetCurrent(); + using WindowsIdentity currentWindowsIdentity = WindowsIdentity.GetCurrent(); await WindowsIdentity.RunImpersonatedAsync(_fixture.TestAccount.AccountTokenHandle, async () => { @@ -39,7 +39,10 @@ await WindowsIdentity.RunImpersonatedAsync(_fixture.TestAccount.AccountTokenHand Asserts(currentWindowsIdentity); }); - Assert.Equal(WindowsIdentity.GetCurrent().Name, currentWindowsIdentity.Name); + using (WindowsIdentity currentIdentity = WindowsIdentity.GetCurrent()) + { + Assert.Equal(currentIdentity.Name, currentWindowsIdentity.Name); + } int result = await WindowsIdentity.RunImpersonatedAsync(_fixture.TestAccount.AccountTokenHandle, async () => { @@ -50,13 +53,17 @@ await WindowsIdentity.RunImpersonatedAsync(_fixture.TestAccount.AccountTokenHand }); Assert.Equal(42, result); - Assert.Equal(WindowsIdentity.GetCurrent().Name, currentWindowsIdentity.Name); + using (WindowsIdentity currentIdentity = WindowsIdentity.GetCurrent()) + { + Assert.Equal(currentIdentity.Name, currentWindowsIdentity.Name); + } // Assertions void Asserts(WindowsIdentity currentWindowsIdentity) { - Assert.Equal(_fixture.TestAccount.AccountName, WindowsIdentity.GetCurrent().Name); - Assert.NotEqual(currentWindowsIdentity.Name, WindowsIdentity.GetCurrent().Name); + using WindowsIdentity currentIdentity = WindowsIdentity.GetCurrent(); + Assert.Equal(_fixture.TestAccount.AccountName, currentIdentity.Name); + Assert.NotEqual(currentWindowsIdentity.Name, currentIdentity.Name); } } @@ -64,11 +71,14 @@ void Asserts(WindowsIdentity currentWindowsIdentity) [OuterLoop] public void RunImpersonated_NameResolution() { - WindowsIdentity currentWindowsIdentity = WindowsIdentity.GetCurrent(); + using WindowsIdentity currentWindowsIdentity = WindowsIdentity.GetCurrent(); WindowsIdentity.RunImpersonated(_fixture.TestAccount.AccountTokenHandle, () => { - Assert.Equal(_fixture.TestAccount.AccountName, WindowsIdentity.GetCurrent().Name); + using (WindowsIdentity currentIdentity = WindowsIdentity.GetCurrent()) + { + Assert.Equal(_fixture.TestAccount.AccountName, currentIdentity.Name); + } IPAddress[] a1 = Dns.GetHostAddressesAsync("").GetAwaiter().GetResult(); IPAddress[] a2 = Dns.GetHostAddresses(""); @@ -82,14 +92,17 @@ public void RunImpersonated_NameResolution() [OuterLoop] public async Task RunImpersonatedAsync_NameResolution() { - WindowsIdentity currentWindowsIdentity = WindowsIdentity.GetCurrent(); + using WindowsIdentity currentWindowsIdentity = WindowsIdentity.GetCurrent(); // make sure the assembly is loaded. _ = Dns.GetHostAddresses(""); await WindowsIdentity.RunImpersonatedAsync(_fixture.TestAccount.AccountTokenHandle, async () => { - Assert.Equal(_fixture.TestAccount.AccountName, WindowsIdentity.GetCurrent().Name); + using (WindowsIdentity currentIdentity = WindowsIdentity.GetCurrent()) + { + Assert.Equal(_fixture.TestAccount.AccountName, currentIdentity.Name); + } IPAddress[] a1 = await Dns.GetHostAddressesAsync(""); IPAddress[] a2 = Dns.GetHostAddresses(""); diff --git a/src/libraries/System.Security.Principal.Windows/tests/WindowsIdentityTests.cs b/src/libraries/System.Security.Principal.Windows/tests/WindowsIdentityTests.cs index 1a073d5e57985..25b16e1b0dbb0 100644 --- a/src/libraries/System.Security.Principal.Windows/tests/WindowsIdentityTests.cs +++ b/src/libraries/System.Security.Principal.Windows/tests/WindowsIdentityTests.cs @@ -99,7 +99,7 @@ public static void CloneAndProperties() [Fact] public static void GetTokenHandle() { - WindowsIdentity id = WindowsIdentity.GetCurrent(); + using WindowsIdentity id = WindowsIdentity.GetCurrent(); Assert.Equal(id.AccessToken.DangerousGetHandle(), id.Token); } @@ -170,7 +170,8 @@ public static void RunImpersonatedAsyncTest() private static void BeginTask(RunImpersonatedAsyncTestInfo testInfo) { testInfo.continueTask = new SemaphoreSlim(0, 1); - using (SafeAccessTokenHandle token = WindowsIdentity.GetCurrent().AccessToken) + using (WindowsIdentity currentIdentity = WindowsIdentity.GetCurrent()) + using (SafeAccessTokenHandle token = currentIdentity.AccessToken) { WindowsIdentity.RunImpersonated(token, () => { @@ -217,7 +218,8 @@ private static void CheckDispose(WindowsIdentity identity, bool anonymous = fals private static void TestUsingAccessToken(Action ctorOrPropertyTest) { // Retrieve the Windows account token for the current user. - SafeAccessTokenHandle token = WindowsIdentity.GetCurrent().AccessToken; + using WindowsIdentity currentIdentity = WindowsIdentity.GetCurrent(); + SafeAccessTokenHandle token = currentIdentity.AccessToken; bool gotRef = false; try { diff --git a/src/libraries/System.Security.Principal.Windows/tests/WindowsPrincipalTests.cs b/src/libraries/System.Security.Principal.Windows/tests/WindowsPrincipalTests.cs index 43a8385ce4d73..a437fc55394c7 100644 --- a/src/libraries/System.Security.Principal.Windows/tests/WindowsPrincipalTests.cs +++ b/src/libraries/System.Security.Principal.Windows/tests/WindowsPrincipalTests.cs @@ -13,7 +13,7 @@ public class WindowsPrincipalTests [Fact] public static void WindowsPrincipalIsInRoleNeg() { - WindowsIdentity windowsIdentity = WindowsIdentity.GetAnonymous(); + using WindowsIdentity windowsIdentity = WindowsIdentity.GetAnonymous(); WindowsPrincipal windowsPrincipal = new WindowsPrincipal(windowsIdentity); try diff --git a/src/libraries/System.ServiceProcess.ServiceController/src/System/ServiceProcess/ServiceController.cs b/src/libraries/System.ServiceProcess.ServiceController/src/System/ServiceProcess/ServiceController.cs index 017c75c113bee..19149b3c47cd3 100644 --- a/src/libraries/System.ServiceProcess.ServiceController/src/System/ServiceProcess/ServiceController.cs +++ b/src/libraries/System.ServiceProcess.ServiceController/src/System/ServiceProcess/ServiceController.cs @@ -638,6 +638,7 @@ private static SafeServiceHandle GetDataBaseHandleWithAccess(string machineName, if (databaseHandle.IsInvalid) { Exception inner = new Win32Exception(); + databaseHandle.Dispose(); throw new InvalidOperationException(SR.Format(SR.OpenSC, machineName), inner); } @@ -687,6 +688,7 @@ private SafeServiceHandle GetServiceHandle(int desiredAccess) if (serviceHandle.IsInvalid) { Exception inner = new Win32Exception(); + serviceHandle.Dispose(); throw new InvalidOperationException(SR.Format(SR.OpenService, ServiceName, _machineName), inner); } diff --git a/src/libraries/System.ServiceProcess.ServiceController/tests/TestServiceProvider.cs b/src/libraries/System.ServiceProcess.ServiceController/tests/TestServiceProvider.cs index 4ea0bda4fd470..88d3735179ce7 100644 --- a/src/libraries/System.ServiceProcess.ServiceController/tests/TestServiceProvider.cs +++ b/src/libraries/System.ServiceProcess.ServiceController/tests/TestServiceProvider.cs @@ -17,8 +17,11 @@ internal sealed class TestServiceProvider private const int readTimeout = 60000; - private static readonly Lazy s_runningWithElevatedPrivileges = new Lazy( - () => new WindowsPrincipal(WindowsIdentity.GetCurrent()).IsInRole(WindowsBuiltInRole.Administrator)); + private static readonly Lazy s_runningWithElevatedPrivileges = new Lazy(() => + { + using WindowsIdentity currentIdentity = WindowsIdentity.GetCurrent(); + return new WindowsPrincipal(currentIdentity).IsInRole(WindowsBuiltInRole.Administrator); + }); private NamedPipeClientStream _client; diff --git a/src/libraries/System.Speech/src/Internal/ObjectToken/RegistryDataKey.cs b/src/libraries/System.Speech/src/Internal/ObjectToken/RegistryDataKey.cs index dddd73c8f81a8..fba6ad398d7e3 100644 --- a/src/libraries/System.Speech/src/Internal/ObjectToken/RegistryDataKey.cs +++ b/src/libraries/System.Speech/src/Internal/ObjectToken/RegistryDataKey.cs @@ -74,6 +74,7 @@ internal static RegistryDataKey Open(string registryPath, bool fCreateIfNotExist // If there's no root, we can't do anything. if (regHandle == null || regHandle.IsInvalid) { + regHandle?.Dispose(); return null; } diff --git a/src/libraries/System.Threading.AccessControl/src/System/Threading/EventWaitHandleAcl.cs b/src/libraries/System.Threading.AccessControl/src/System/Threading/EventWaitHandleAcl.cs index cf275d68f24a1..a65f1e0157952 100644 --- a/src/libraries/System.Threading.AccessControl/src/System/Threading/EventWaitHandleAcl.cs +++ b/src/libraries/System.Threading.AccessControl/src/System/Threading/EventWaitHandleAcl.cs @@ -139,6 +139,7 @@ private static OpenExistingResult OpenExistingWorker(string name, EventWaitHandl int errorCode = Marshal.GetLastWin32Error(); if (existingHandle.IsInvalid) { + existingHandle.Dispose(); return errorCode switch { Interop.Errors.ERROR_FILE_NOT_FOUND or Interop.Errors.ERROR_INVALID_NAME => OpenExistingResult.NameNotFound, diff --git a/src/libraries/System.Threading.AccessControl/src/System/Threading/MutexAcl.cs b/src/libraries/System.Threading.AccessControl/src/System/Threading/MutexAcl.cs index 5f671ceac7c98..ff881c188bec3 100644 --- a/src/libraries/System.Threading.AccessControl/src/System/Threading/MutexAcl.cs +++ b/src/libraries/System.Threading.AccessControl/src/System/Threading/MutexAcl.cs @@ -130,6 +130,7 @@ private static OpenExistingResult OpenExistingWorker(string name, MutexRights ri int errorCode = Marshal.GetLastWin32Error(); if (existingHandle.IsInvalid) { + existingHandle.Dispose(); return errorCode switch { Interop.Errors.ERROR_FILE_NOT_FOUND or Interop.Errors.ERROR_INVALID_NAME => OpenExistingResult.NameNotFound, diff --git a/src/libraries/System.Threading.AccessControl/src/System/Threading/SemaphoreAcl.cs b/src/libraries/System.Threading.AccessControl/src/System/Threading/SemaphoreAcl.cs index c689e2f784e86..35c18aeac91f9 100644 --- a/src/libraries/System.Threading.AccessControl/src/System/Threading/SemaphoreAcl.cs +++ b/src/libraries/System.Threading.AccessControl/src/System/Threading/SemaphoreAcl.cs @@ -67,6 +67,8 @@ public static unsafe Semaphore Create(int initialCount, int maximumCount, string if (handle.IsInvalid) { + handle.Dispose(); + if (!string.IsNullOrEmpty(name) && errorCode == Interop.Errors.ERROR_INVALID_HANDLE) { throw new WaitHandleCannotBeOpenedException(SR.Format(SR.Threading_WaitHandleCannotBeOpenedException_InvalidHandle, name)); @@ -143,6 +145,7 @@ private static OpenExistingResult OpenExistingWorker(string name, SemaphoreRight int errorCode = Marshal.GetLastWin32Error(); if (handle.IsInvalid) { + handle.Dispose(); return errorCode switch { Interop.Errors.ERROR_FILE_NOT_FOUND or Interop.Errors.ERROR_INVALID_NAME => OpenExistingResult.NameNotFound, diff --git a/src/libraries/System.Windows.Extensions/src/System/Security/Cryptography/X509Certificates/X509Certificate2UI.cs b/src/libraries/System.Windows.Extensions/src/System/Security/Cryptography/X509Certificates/X509Certificate2UI.cs index 702182c401e89..929119cd4d3f0 100644 --- a/src/libraries/System.Windows.Extensions/src/System/Security/Cryptography/X509Certificates/X509Certificate2UI.cs +++ b/src/libraries/System.Windows.Extensions/src/System/Security/Cryptography/X509Certificates/X509Certificate2UI.cs @@ -113,7 +113,11 @@ private static unsafe SafeCertStoreHandle SelectFromStore(SafeCertStoreHandle sa IntPtr.Zero); if (safeCertStoreHandle == null || safeCertStoreHandle.IsInvalid) - throw new CryptographicException(Marshal.GetLastWin32Error()); + { + Exception e = new CryptographicException(Marshal.GetLastWin32Error()); + safeCertStoreHandle?.Dispose(); + throw e; + } Interop.CryptUI.CRYPTUI_SELECTCERTIFICATE_STRUCTW csc = default; // Older versions of CRYPTUI do not check the size correctly, @@ -160,7 +164,10 @@ private static unsafe SafeCertStoreHandle SelectFromStore(SafeCertStoreHandle sa } if (dwErrorCode != ERROR_SUCCESS) + { + safeCertContextHandle?.Dispose(); throw new CryptographicException(dwErrorCode); + } return safeCertStoreHandle; } diff --git a/src/libraries/System.Windows.Extensions/src/System/Security/Cryptography/X509Certificates/X509Utils.cs b/src/libraries/System.Windows.Extensions/src/System/Security/Cryptography/X509Certificates/X509Utils.cs index d995154505290..3020d9103b117 100644 --- a/src/libraries/System.Windows.Extensions/src/System/Security/Cryptography/X509Certificates/X509Utils.cs +++ b/src/libraries/System.Windows.Extensions/src/System/Security/Cryptography/X509Certificates/X509Utils.cs @@ -32,7 +32,11 @@ internal static SafeCertStoreHandle ExportToMemoryStore(X509Certificate2Collecti IntPtr.Zero); if (safeCertStoreHandle == null || safeCertStoreHandle.IsInvalid) - throw new CryptographicException(Marshal.GetLastWin32Error()); + { + Exception e = new CryptographicException(Marshal.GetLastWin32Error()); + safeCertStoreHandle?.Dispose(); + throw e; + } // We use CertAddCertificateLinkToStore to keep a link to the original store, so any property changes get // applied to the original store. This has a limit of 99 links per cert context however. From afab3f97282fe42110726e4e8a050aea9ae17478 Mon Sep 17 00:00:00 2001 From: Stephen Toub Date: Tue, 12 Jul 2022 09:38:12 -0400 Subject: [PATCH 2/4] Address PR feedback and test failure --- .../IO/Compression/CompressionStreamUnitTestBase.cs | 8 +++----- .../System/IO/Pipes/AnonymousPipeServerStream.Windows.cs | 5 ++++- .../src/System/IO/Pipes/NamedPipeServerStream.Windows.cs | 3 ++- .../Internal/Cryptography/Pal/Windows/HelpersWindows.cs | 3 ++- 4 files changed, 11 insertions(+), 8 deletions(-) diff --git a/src/libraries/Common/tests/System/IO/Compression/CompressionStreamUnitTestBase.cs b/src/libraries/Common/tests/System/IO/Compression/CompressionStreamUnitTestBase.cs index e3222ea9fd69a..b693f80ee5edd 100644 --- a/src/libraries/Common/tests/System/IO/Compression/CompressionStreamUnitTestBase.cs +++ b/src/libraries/Common/tests/System/IO/Compression/CompressionStreamUnitTestBase.cs @@ -241,13 +241,11 @@ public async Task Read_BaseStreamSlowly() } } - [Theory] - [InlineData(CompressionMode.Compress)] - [InlineData(CompressionMode.Decompress)] - public void CanDisposeBaseStream(CompressionMode mode) + [Fact] + public void CanDisposeBaseStream() { var ms = new MemoryStream(); - using var compressor = CreateStream(ms, mode); + using var compressor = CreateStream(ms, CompressionMode.Decompress); ms.Dispose(); // This would throw if this was invalid } diff --git a/src/libraries/System.IO.Pipes/src/System/IO/Pipes/AnonymousPipeServerStream.Windows.cs b/src/libraries/System.IO.Pipes/src/System/IO/Pipes/AnonymousPipeServerStream.Windows.cs index 59859f0dde833..7696177584e93 100644 --- a/src/libraries/System.IO.Pipes/src/System/IO/Pipes/AnonymousPipeServerStream.Windows.cs +++ b/src/libraries/System.IO.Pipes/src/System/IO/Pipes/AnonymousPipeServerStream.Windows.cs @@ -67,9 +67,12 @@ private void Create(PipeDirection direction, HandleInheritability inheritability if (!bSuccess) { + Exception e = Win32Marshal.GetExceptionForLastWin32Error(); + serverHandle.Dispose(); clientHandle.Dispose(); - throw Win32Marshal.GetExceptionForLastWin32Error(); + + throw e; } // Duplicate the server handle to make it not inheritable. Note: We need to do this so that the child diff --git a/src/libraries/System.IO.Pipes/src/System/IO/Pipes/NamedPipeServerStream.Windows.cs b/src/libraries/System.IO.Pipes/src/System/IO/Pipes/NamedPipeServerStream.Windows.cs index 7ec4f97e1f30e..142a1b1921cde 100644 --- a/src/libraries/System.IO.Pipes/src/System/IO/Pipes/NamedPipeServerStream.Windows.cs +++ b/src/libraries/System.IO.Pipes/src/System/IO/Pipes/NamedPipeServerStream.Windows.cs @@ -141,8 +141,9 @@ private void Create(string pipeName, PipeDirection direction, int maxNumberOfSer if (handle.IsInvalid) { + Exception e = Win32Marshal.GetExceptionForLastWin32Error(); handle.Dispose(); - throw Win32Marshal.GetExceptionForLastWin32Error(); + throw e; } InitializeHandle(handle, false, (options & PipeOptions.Asynchronous) != 0); diff --git a/src/libraries/System.Security.Cryptography.Pkcs/src/Internal/Cryptography/Pal/Windows/HelpersWindows.cs b/src/libraries/System.Security.Cryptography.Pkcs/src/Internal/Cryptography/Pal/Windows/HelpersWindows.cs index f675d068c65c7..18121fa55f10d 100644 --- a/src/libraries/System.Security.Cryptography.Pkcs/src/Internal/Cryptography/Pal/Windows/HelpersWindows.cs +++ b/src/libraries/System.Security.Cryptography.Pkcs/src/Internal/Cryptography/Pal/Windows/HelpersWindows.cs @@ -60,8 +60,9 @@ public static SafeHandle GetMsgParamAsMemory(this SafeCryptMsgHandle hCryptMsg, SafeHandle pvData = SafeHeapAllocHandle.Alloc(cbData); if (!Interop.Crypt32.CryptMsgGetParam(hCryptMsg, paramType, index, pvData.DangerousGetHandle(), ref cbData)) { + Exception e = Marshal.GetLastWin32Error().ToCryptographicException(); pvData.Dispose(); - throw Marshal.GetLastWin32Error().ToCryptographicException(); + throw e; } return pvData; From 767ee06786742e1b935856cc958455db03993b26 Mon Sep 17 00:00:00 2001 From: Stephen Toub Date: Tue, 12 Jul 2022 16:52:43 -0400 Subject: [PATCH 3/4] Address PR feedback --- .../tests/File/OpenHandle.cs | 33 +++++++++++++++---- 1 file changed, 26 insertions(+), 7 deletions(-) diff --git a/src/libraries/System.IO.FileSystem/tests/File/OpenHandle.cs b/src/libraries/System.IO.FileSystem/tests/File/OpenHandle.cs index c89097197d2d8..36349e8b993b7 100644 --- a/src/libraries/System.IO.FileSystem/tests/File/OpenHandle.cs +++ b/src/libraries/System.IO.FileSystem/tests/File/OpenHandle.cs @@ -11,14 +11,22 @@ public class File_OpenHandle : FileStream_ctor_options { protected override string GetExpectedParamName(string paramName) => paramName; - protected override FileStream CreateFileStream(string path, FileMode mode) - { - FileAccess access = mode == FileMode.Append ? FileAccess.Write : FileAccess.ReadWrite; - return new FileStream(File.OpenHandle(path, mode, access), access); - } + protected override FileStream CreateFileStream(string path, FileMode mode) => + CreateFileStream(path, mode, mode == FileMode.Append ? FileAccess.Write : FileAccess.ReadWrite); protected override FileStream CreateFileStream(string path, FileMode mode, FileAccess access) - => new FileStream(File.OpenHandle(path, mode, access), access); + { + SafeFileHandle handle = File.OpenHandle(path, mode, access); + try + { + return new FileStream(handle, access); + } + catch + { + handle.Dispose(); + throw; + } + } protected override FileStream CreateFileStream(string path, FileMode mode, FileAccess access, FileShare share, int bufferSize, FileOptions options) { @@ -35,7 +43,18 @@ protected override FileStream CreateFileStream(string path, FileMode mode, FileA } protected override FileStream CreateFileStream(string path, FileMode mode, FileAccess access, FileShare share, int bufferSize, FileOptions options, long preallocationSize) - => new FileStream(File.OpenHandle(path, mode, access, share, options, preallocationSize), access, bufferSize, (options & FileOptions.Asynchronous) != 0); + { + SafeFileHandle handle = File.OpenHandle(path, mode, access, share, options, preallocationSize); + try + { + return new FileStream(handle, access, bufferSize, (options & FileOptions.Asynchronous) != 0); + } + catch + { + handle.Dispose(); + throw; + } + } [ActiveIssue("https://github.com/dotnet/runtime/issues/53432")] [Theory, MemberData(nameof(StreamSpecifiers))] From 3d9e04ee2afa4525b171c5a08d611335c3381d4a Mon Sep 17 00:00:00 2001 From: Stephen Toub Date: Tue, 12 Jul 2022 23:30:01 -0400 Subject: [PATCH 4/4] Update CertificateValidationPal.Windows.cs --- .../src/System/Net/CertificateValidationPal.Windows.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libraries/System.Net.Security/src/System/Net/CertificateValidationPal.Windows.cs b/src/libraries/System.Net.Security/src/System/Net/CertificateValidationPal.Windows.cs index f8ed7bca29e49..1ead906ec3585 100644 --- a/src/libraries/System.Net.Security/src/System/Net/CertificateValidationPal.Windows.cs +++ b/src/libraries/System.Net.Security/src/System/Net/CertificateValidationPal.Windows.cs @@ -76,7 +76,7 @@ internal static SslPolicyErrors VerifyCertificateProperties( { chain.ChainPolicy = chainPolicy; } - + UnmanagedCertificateContext.GetRemoteCertificatesFromStoreContext(remoteContext, chain.ChainPolicy.ExtraStore); } }