From 09af5422d75957952097a76860a03bfb88df8234 Mon Sep 17 00:00:00 2001 From: MaxPatri Date: Mon, 16 Dec 2024 16:34:20 +0300 Subject: [PATCH 1/4] Add Dispose for X509Chain instance --- .../src/System/Security/Cryptography/Pkcs/SignerInfo.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libraries/System.Security.Cryptography.Pkcs/src/System/Security/Cryptography/Pkcs/SignerInfo.cs b/src/libraries/System.Security.Cryptography.Pkcs/src/System/Security/Cryptography/Pkcs/SignerInfo.cs index 383b9ddc3011d8..604b66ff3753aa 100644 --- a/src/libraries/System.Security.Cryptography.Pkcs/src/System/Security/Cryptography/Pkcs/SignerInfo.cs +++ b/src/libraries/System.Security.Cryptography.Pkcs/src/System/Security/Cryptography/Pkcs/SignerInfo.cs @@ -706,7 +706,7 @@ private void Verify( if (!verifySignatureOnly) { - X509Chain chain = new X509Chain(); + using X509Chain chain = new X509Chain(); chain.ChainPolicy.ExtraStore.AddRange(extraStore); chain.ChainPolicy.RevocationMode = X509RevocationMode.Online; chain.ChainPolicy.RevocationFlag = X509RevocationFlag.ExcludeRoot; From 16557453cd755a8ce6375bf7d75df262758c3b5c Mon Sep 17 00:00:00 2001 From: MaxPatri Date: Tue, 14 Jan 2025 12:26:41 +0300 Subject: [PATCH 2/4] Review fix. Add try/finally --- .../Security/Cryptography/Pkcs/SignerInfo.cs | 30 ++++++++++++++----- 1 file changed, 23 insertions(+), 7 deletions(-) diff --git a/src/libraries/System.Security.Cryptography.Pkcs/src/System/Security/Cryptography/Pkcs/SignerInfo.cs b/src/libraries/System.Security.Cryptography.Pkcs/src/System/Security/Cryptography/Pkcs/SignerInfo.cs index 604b66ff3753aa..f1670db9a3161f 100644 --- a/src/libraries/System.Security.Cryptography.Pkcs/src/System/Security/Cryptography/Pkcs/SignerInfo.cs +++ b/src/libraries/System.Security.Cryptography.Pkcs/src/System/Security/Cryptography/Pkcs/SignerInfo.cs @@ -706,15 +706,31 @@ private void Verify( if (!verifySignatureOnly) { - using X509Chain chain = new X509Chain(); - chain.ChainPolicy.ExtraStore.AddRange(extraStore); - chain.ChainPolicy.RevocationMode = X509RevocationMode.Online; - chain.ChainPolicy.RevocationFlag = X509RevocationFlag.ExcludeRoot; + X509Chain chain = null; - if (!chain.Build(certificate)) + try { - X509ChainStatus status = chain.ChainStatus.FirstOrDefault(); - throw new CryptographicException(SR.Cryptography_Cms_TrustFailure, status.StatusInformation); + chain = new X509Chain(); + chain.ChainPolicy.ExtraStore.AddRange(extraStore); + chain.ChainPolicy.RevocationMode = X509RevocationMode.Online; + chain.ChainPolicy.RevocationFlag = X509RevocationFlag.ExcludeRoot; + + if (!chain.Build(certificate)) + { + X509ChainStatus status = chain.ChainStatus.FirstOrDefault(); + throw new CryptographicException(SR.Cryptography_Cms_TrustFailure, status.StatusInformation); + } + } + finally + { + if (chain != null) + { + for (int i = 0; i < chain.ChainElements.Count; i++) + { + chain.ChainElements[i].Certificate.Dispose(); + } + chain.Dispose(); + } } // .NET Framework checks for either of these From 785577fdac936ee79af7f022438b17e240b243a1 Mon Sep 17 00:00:00 2001 From: MaxPatri Date: Wed, 15 Jan 2025 10:05:22 +0300 Subject: [PATCH 3/4] Fix chain initialization --- .../Security/Cryptography/Pkcs/SignerInfo.cs | 15 +++++---------- 1 file changed, 5 insertions(+), 10 deletions(-) diff --git a/src/libraries/System.Security.Cryptography.Pkcs/src/System/Security/Cryptography/Pkcs/SignerInfo.cs b/src/libraries/System.Security.Cryptography.Pkcs/src/System/Security/Cryptography/Pkcs/SignerInfo.cs index f1670db9a3161f..18d279bc129a97 100644 --- a/src/libraries/System.Security.Cryptography.Pkcs/src/System/Security/Cryptography/Pkcs/SignerInfo.cs +++ b/src/libraries/System.Security.Cryptography.Pkcs/src/System/Security/Cryptography/Pkcs/SignerInfo.cs @@ -706,11 +706,9 @@ private void Verify( if (!verifySignatureOnly) { - X509Chain chain = null; - + X509Chain chain = new X509Chain(); try { - chain = new X509Chain(); chain.ChainPolicy.ExtraStore.AddRange(extraStore); chain.ChainPolicy.RevocationMode = X509RevocationMode.Online; chain.ChainPolicy.RevocationFlag = X509RevocationFlag.ExcludeRoot; @@ -721,16 +719,13 @@ private void Verify( throw new CryptographicException(SR.Cryptography_Cms_TrustFailure, status.StatusInformation); } } - finally + finally { - if (chain != null) + for (int i = 0; i < chain.ChainElements.Count; i++) { - for (int i = 0; i < chain.ChainElements.Count; i++) - { - chain.ChainElements[i].Certificate.Dispose(); - } - chain.Dispose(); + chain.ChainElements[i].Certificate.Dispose(); } + chain.Dispose(); } // .NET Framework checks for either of these From 9fd6c3c4534bd5593310df35b81547242968fd53 Mon Sep 17 00:00:00 2001 From: MaxPatri Date: Wed, 15 Jan 2025 12:02:12 +0300 Subject: [PATCH 4/4] Add empty line --- .../src/System/Security/Cryptography/Pkcs/SignerInfo.cs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/libraries/System.Security.Cryptography.Pkcs/src/System/Security/Cryptography/Pkcs/SignerInfo.cs b/src/libraries/System.Security.Cryptography.Pkcs/src/System/Security/Cryptography/Pkcs/SignerInfo.cs index 18d279bc129a97..7fd01594310732 100644 --- a/src/libraries/System.Security.Cryptography.Pkcs/src/System/Security/Cryptography/Pkcs/SignerInfo.cs +++ b/src/libraries/System.Security.Cryptography.Pkcs/src/System/Security/Cryptography/Pkcs/SignerInfo.cs @@ -725,6 +725,7 @@ private void Verify( { chain.ChainElements[i].Certificate.Dispose(); } + chain.Dispose(); }