From 806d04b02e42254b0be9b0b85119f3e9133462bd Mon Sep 17 00:00:00 2001 From: Krzysztof Wicher Date: Mon, 15 Jan 2024 20:14:30 +0000 Subject: [PATCH] Add null checks in System.Security.Cryptography --- .../tests/X509Certificates/CertTests.cs | 6 ++++++ .../tests/X509Certificates/TestData.cs | 2 ++ .../apibridge.c | 17 ++++++++++++++++- .../openssl.c | 5 +++++ .../pal_pkcs7.c | 10 ++++++++++ 5 files changed, 39 insertions(+), 1 deletion(-) diff --git a/src/libraries/System.Security.Cryptography/tests/X509Certificates/CertTests.cs b/src/libraries/System.Security.Cryptography/tests/X509Certificates/CertTests.cs index c51987b37ca3a6..838398a8b0092c 100644 --- a/src/libraries/System.Security.Cryptography/tests/X509Certificates/CertTests.cs +++ b/src/libraries/System.Security.Cryptography/tests/X509Certificates/CertTests.cs @@ -440,6 +440,12 @@ public static void UseAfterDispose() } } + [Fact] + public static void EmptyPkcs7ThrowsException() + { + Assert.ThrowsAny(() => new X509Certificate2(TestData.EmptyPkcs7)); + } + [Fact] public static void ExportPublicKeyAsPkcs12() { diff --git a/src/libraries/System.Security.Cryptography/tests/X509Certificates/TestData.cs b/src/libraries/System.Security.Cryptography/tests/X509Certificates/TestData.cs index cf14136eb6e058..0d7633f9992ef2 100644 --- a/src/libraries/System.Security.Cryptography/tests/X509Certificates/TestData.cs +++ b/src/libraries/System.Security.Cryptography/tests/X509Certificates/TestData.cs @@ -4273,5 +4273,7 @@ internal static DSAParameters GetDSA1024Params() "C0CC2B115B9D33BD6E528E35670E5A6A8D9CF52199F8D693315C60D9ADAD54EF7FDCED36" + "0C8C79E84D42AB5CB6355A70951B1ABF1F2B3FB8BEB7E3A8D6BA2293C0DB8C86B0BB060F" + "0D6DB9939E88B998662A27F092634BBF21F58EEAAA").HexToByteArray(); + + internal static readonly byte[] EmptyPkcs7 = "300B06092A864886F70D010702".HexToByteArray(); } } diff --git a/src/native/libs/System.Security.Cryptography.Native/apibridge.c b/src/native/libs/System.Security.Cryptography.Native/apibridge.c index bad65ee2b50211..64ed18961fea4c 100644 --- a/src/native/libs/System.Security.Cryptography.Native/apibridge.c +++ b/src/native/libs/System.Security.Cryptography.Native/apibridge.c @@ -112,7 +112,7 @@ int32_t local_X509_get_version(const X509* x509) X509_PUBKEY* local_X509_get_X509_PUBKEY(const X509* x509) { - if (x509) + if (x509 && x509->cert_info) { return x509->cert_info->key; } @@ -123,13 +123,28 @@ X509_PUBKEY* local_X509_get_X509_PUBKEY(const X509* x509) int32_t local_X509_PUBKEY_get0_param( ASN1_OBJECT** palgOid, const uint8_t** pkeyBytes, int* pkeyBytesLen, X509_ALGOR** palg, X509_PUBKEY* pubkey) { + if (!pubkey) + { + return 0; + } + if (palgOid) { + if (!pubkey->algor) + { + return 0; + } + *palgOid = pubkey->algor->algorithm; } if (pkeyBytes) { + if (!pubkey->public_key) + { + return 0; + } + *pkeyBytes = pubkey->public_key->data; *pkeyBytesLen = pubkey->public_key->length; } diff --git a/src/native/libs/System.Security.Cryptography.Native/openssl.c b/src/native/libs/System.Security.Cryptography.Native/openssl.c index ba713b6fdcc212..73822045895425 100644 --- a/src/native/libs/System.Security.Cryptography.Native/openssl.c +++ b/src/native/libs/System.Security.Cryptography.Native/openssl.c @@ -669,6 +669,11 @@ BIO* CryptoNative_GetX509NameInfo(X509* x509, int32_t nameType, int32_t forIssue 0 == strncmp(localOid, szOidUpn, sizeof(szOidUpn))) { // OTHERNAME->ASN1_TYPE->union.field + if (!value->value) + { + return NULL; + } + str = value->value->value.asn1_string; } } diff --git a/src/native/libs/System.Security.Cryptography.Native/pal_pkcs7.c b/src/native/libs/System.Security.Cryptography.Native/pal_pkcs7.c index efb0a738966f55..bc6c1215d632d2 100644 --- a/src/native/libs/System.Security.Cryptography.Native/pal_pkcs7.c +++ b/src/native/libs/System.Security.Cryptography.Native/pal_pkcs7.c @@ -53,9 +53,19 @@ int32_t CryptoNative_GetPkcs7Certificates(PKCS7* p7, X509Stack** certs) switch (OBJ_obj2nid(p7->type)) { case NID_pkcs7_signed: + if (!p7->d.sign) + { + return 0; + } + *certs = p7->d.sign->cert; return 1; case NID_pkcs7_signedAndEnveloped: + if (!p7->d.signed_and_enveloped) + { + return 0; + } + *certs = p7->d.signed_and_enveloped->cert; return 1; }