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..b693f80ee5edd 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; @@ -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(); - var compressor = CreateStream(ms, mode); + using var compressor = CreateStream(ms, CompressionMode.Decompress); ms.Dispose(); // This would throw if this was invalid } @@ -307,7 +305,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 +320,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 +435,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 +450,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..36349e8b993b7 100644 --- a/src/libraries/System.IO.FileSystem/tests/File/OpenHandle.cs +++ b/src/libraries/System.IO.FileSystem/tests/File/OpenHandle.cs @@ -11,20 +11,50 @@ 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) - => 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); + { + 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))] 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..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 @@ -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,7 +67,12 @@ private void Create(PipeDirection direction, HandleInheritability inheritability if (!bSuccess) { - throw Win32Marshal.GetExceptionForLastWin32Error(); + Exception e = Win32Marshal.GetExceptionForLastWin32Error(); + + serverHandle.Dispose(); + clientHandle.Dispose(); + + throw e; } // Duplicate the server handle to make it not inheritable. Note: We need to do this so that the child @@ -79,12 +84,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..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,7 +141,9 @@ private void Create(string pipeName, PipeDirection direction, int maxNumberOfSer if (handle.IsInvalid) { - throw Win32Marshal.GetExceptionForLastWin32Error(); + Exception e = Win32Marshal.GetExceptionForLastWin32Error(); + handle.Dispose(); + throw e; } InitializeHandle(handle, false, (options & PipeOptions.Asynchronous) != 0); 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 6df739f2045d6..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 @@ -65,17 +65,20 @@ internal static SslPolicyErrors VerifyCertificateProperties( } finally { - if (remoteContext != null && !remoteContext.IsInvalid) + if (remoteContext != null) { - if (retrieveChainCertificates) + if (!remoteContext.IsInvalid) { - chain ??= new X509Chain(); - if (chainPolicy != null) + if (retrieveChainCertificates) { - chain.ChainPolicy = chainPolicy; - } + chain ??= new X509Chain(); + if (chainPolicy != null) + { + chain.ChainPolicy = chainPolicy; + } - UnmanagedCertificateContext.GetRemoteCertificatesFromStoreContext(remoteContext, chain.ChainPolicy.ExtraStore); + UnmanagedCertificateContext.GetRemoteCertificatesFromStoreContext(remoteContext, chain.ChainPolicy.ExtraStore); + } } remoteContext.Dispose(); @@ -135,7 +138,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 6a4286f301fb5..fbea595b861f3 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 @@ -247,6 +247,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..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 @@ -53,11 +53,17 @@ 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)) - throw Marshal.GetLastWin32Error().ToCryptographicException(); + { + Exception e = Marshal.GetLastWin32Error().ToCryptographicException(); + pvData.Dispose(); + throw e; + } 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 d83530d814942..4ac54e571a52d 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.