From 7c2d8f8ed9f419a984f77c418b3da860dc6579b9 Mon Sep 17 00:00:00 2001
From: Krzysztof Wicher <kwicher@microsoft.com>
Date: Mon, 15 Jan 2024 20:16:35 +0000
Subject: [PATCH] Add null checks in System.Security.Cryptography

---
 .../apibridge.c                                 | 17 ++++++++++++++++-
 .../openssl.c                                   |  5 +++++
 .../pal_pkcs7.c                                 | 10 ++++++++++
 .../tests/CertTests.cs                          |  6 ++++++
 .../tests/TestData.cs                           |  2 ++
 5 files changed, 39 insertions(+), 1 deletion(-)

diff --git a/src/libraries/Native/Unix/System.Security.Cryptography.Native/apibridge.c b/src/libraries/Native/Unix/System.Security.Cryptography.Native/apibridge.c
index 15df621f7a7dfa..78d34ff2ac3fcf 100644
--- a/src/libraries/Native/Unix/System.Security.Cryptography.Native/apibridge.c
+++ b/src/libraries/Native/Unix/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/libraries/Native/Unix/System.Security.Cryptography.Native/openssl.c b/src/libraries/Native/Unix/System.Security.Cryptography.Native/openssl.c
index c18eb0ffb7c6bb..764178f4940130 100644
--- a/src/libraries/Native/Unix/System.Security.Cryptography.Native/openssl.c
+++ b/src/libraries/Native/Unix/System.Security.Cryptography.Native/openssl.c
@@ -635,6 +635,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/libraries/Native/Unix/System.Security.Cryptography.Native/pal_pkcs7.c b/src/libraries/Native/Unix/System.Security.Cryptography.Native/pal_pkcs7.c
index ae1b33d8b4782f..3521a8073f9b4c 100644
--- a/src/libraries/Native/Unix/System.Security.Cryptography.Native/pal_pkcs7.c
+++ b/src/libraries/Native/Unix/System.Security.Cryptography.Native/pal_pkcs7.c
@@ -46,9 +46,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;
     }
diff --git a/src/libraries/System.Security.Cryptography.X509Certificates/tests/CertTests.cs b/src/libraries/System.Security.Cryptography.X509Certificates/tests/CertTests.cs
index abb78cc2392081..c5e62fb835e5ea 100644
--- a/src/libraries/System.Security.Cryptography.X509Certificates/tests/CertTests.cs
+++ b/src/libraries/System.Security.Cryptography.X509Certificates/tests/CertTests.cs
@@ -399,6 +399,12 @@ public static void UseAfterDispose()
             }
         }
 
+        [Fact]
+        public static void EmptyPkcs7ThrowsException()
+        {
+            Assert.ThrowsAny<CryptographicException>(() => new X509Certificate2(TestData.EmptyPkcs7));
+        }
+
         [Fact]
         public static void ExportPublicKeyAsPkcs12()
         {
diff --git a/src/libraries/System.Security.Cryptography.X509Certificates/tests/TestData.cs b/src/libraries/System.Security.Cryptography.X509Certificates/tests/TestData.cs
index c2908231ac1733..f20741f99737c0 100644
--- a/src/libraries/System.Security.Cryptography.X509Certificates/tests/TestData.cs
+++ b/src/libraries/System.Security.Cryptography.X509Certificates/tests/TestData.cs
@@ -4175,5 +4175,7 @@ internal static DSAParameters GetDSA1024Params()
             "C0CC2B115B9D33BD6E528E35670E5A6A8D9CF52199F8D693315C60D9ADAD54EF7FDCED36" +
             "0C8C79E84D42AB5CB6355A70951B1ABF1F2B3FB8BEB7E3A8D6BA2293C0DB8C86B0BB060F" +
             "0D6DB9939E88B998662A27F092634BBF21F58EEAAA").HexToByteArray();
+
+        internal static readonly byte[] EmptyPkcs7 = "300B06092A864886F70D010702".HexToByteArray();
     }
 }