CryptoPkg: Resolve CodeQL Errors

This patch updates several CodeQL errors for potential null pointer
access and unguarded header conclusion across production and test
code that have been flagged in the build/security tab in GitHub.

Signed-off-by: Oliver Smith-Denny <osde@microsoft.com>

CryptoPkg/Library/BaseCryptLib/Hash/CryptParallelHash.h

@@ -21,6 +21,9 @@ and related or neighboring rights to the source code in this file.
 http://creativecommons.org/publicdomain/zero/1.0/
 **/
 
+#ifndef CRYPT_PARALLEL_HASH_H_
+#define CRYPT_PARALLEL_HASH_H_
+
 #include "InternalCryptLib.h"
 
 #define KECCAK1600_WIDTH  1600
@@ -225,3 +228,5 @@ EFIAPI
 DispatchBlockToAp (
   VOID
   );
+
+#endif // CRYPT_PARALLEL_HASH_H_

CryptoPkg/Library/BaseCryptLib/Pk/CryptAuthenticode.c

@@ -110,6 +110,10 @@ AuthenticodeVerify (
   //       PKCS#7 ContentInfo here.
   //
   SpcIndirectDataOid = OBJ_get0_data (Pkcs7->d.sign->contents->type);
+  if (SpcIndirectDataOid == NULL) {
+    goto _Exit;
+  }
+
   if ((OBJ_length (Pkcs7->d.sign->contents->type) != sizeof (mSpcIndirectOidValue)) ||
       (CompareMem (
          SpcIndirectDataOid,

CryptoPkg/Library/BaseCryptLib/Pk/CryptDh.c

@@ -163,8 +163,13 @@ DhSetParameter (
   return TRUE;
 
 Error:
-  BN_free (BnP);
-  BN_free (BnG);
+  if (BnP != NULL) {
+    BN_free (BnP);
+  }
+
+  if (BnG != NULL) {
+    BN_free (BnG);
+  }
 
   return FALSE;
 }

CryptoPkg/Library/BaseCryptLib/Pk/CryptEc.c

@@ -588,6 +588,9 @@ EcGetPubKey (
     return FALSE;
   }
 
+  BnX = NULL;
+  BnY = NULL;
+
   EcKey    = (EC_KEY *)EcContext;
   Group    = EC_KEY_get0_group (EcKey);
   HalfSize = (EC_GROUP_get_degree (Group) + 7) / 8;
@@ -631,8 +634,14 @@ EcGetPubKey (
   RetVal = TRUE;
 
 fail:
-  BN_free (BnX);
-  BN_free (BnY);
+  if (BnX != NULL) {
+    BN_free (BnX);
+  }
+
+  if (BnY != NULL) {
+    BN_free (BnY);
+  }
+
   return RetVal;
 }
 

CryptoPkg/Library/BaseCryptLib/Pk/CryptTs.c

@@ -288,6 +288,9 @@ CheckTSTInfo (
   //
   Imprint  = TstInfo->MessageImprint;
   HashAlgo = X509_ALGOR_dup (Imprint->HashAlgorithm);
+  if ((HashAlgo == NULL) || (HashAlgo->algorithm == NULL)) {
+    goto _Exit;
+  }
 
   Md = EVP_get_digestbyobj (HashAlgo->algorithm);
   if (Md == NULL) {

CryptoPkg/Library/BaseCryptLib/Pk/CryptX509.c

@@ -447,6 +447,14 @@ InternalX509GetNIDName (
   }
 
   EntryData = X509_NAME_ENTRY_get_data (Entry);
+  if (EntryData == NULL) {
+    //
+    // Fail to retrieve name entry data
+    //
+    *CommonNameSize = 0;
+    ReturnStatus    = RETURN_NOT_FOUND;
+    goto _Exit;
+  }
 
   Length = ASN1_STRING_to_UTF8 (&UTF8Name, EntryData);
   if (Length < 0) {
@@ -808,6 +816,8 @@ X509GetTBSCert (
   UINTN        Length;
   UINTN        Inf;
 
+  Asn1Tag = (UINT32)V_ASN1_UNDEF;
+
   //
   // Check input parameters.
   //

CryptoPkg/Library/BaseCryptLib/SysCall/CrtWrapper.c

@@ -460,7 +460,10 @@ qsort (
   // Use CRT-style malloc to cover BS and RT memory allocation.
   //
   Buffer = malloc (width);
-  ASSERT (Buffer != NULL);
+  if (Buffer == NULL) {
+    ASSERT (Buffer != NULL);
+    return;
+  }
 
   //
   // Re-use PerformQuickSort() function Implementation in EDKII BaseSortLib.

CryptoPkg/Test/UnitTest/Library/BaseCryptLib/Pkcs5Pbkdf2Tests.c

@@ -33,6 +33,10 @@ TestVerifyPkcs5Pbkdf2 (
   UINT8    *OutKey;
 
   OutKey = AllocatePool (KeyLen);
+  if (OutKey == NULL) {
+    UT_LOG_ERROR ("Failed to allocate memory for OutKey.\n");
+    return UNIT_TEST_ERROR_TEST_FAILED;
+  }
 
   //
   // Verify PKCS#5 PBKDF2 Key Derivation Function

CryptoPkg/Test/UnitTest/Library/BaseCryptLib/RsaPkcs7Tests.c

@@ -246,18 +246,26 @@ TestVerifyRsaCertPkcs1SignVerify (
   IN UNIT_TEST_CONTEXT  Context
   )
 {
-  BOOLEAN        Status;
-  VOID           *RsaPrivKey;
-  VOID           *RsaPubKey;
-  UINT8          *Signature;
-  UINTN          SigSize;
-  UINT8          *Subject;
-  UINTN          SubjectSize;
-  RETURN_STATUS  ReturnStatus;
-  CHAR8          CommonName[64];
-  UINTN          CommonNameSize;
-  CHAR8          OrgName[64];
-  UINTN          OrgNameSize;
+  BOOLEAN           Status;
+  VOID              *RsaPrivKey;
+  VOID              *RsaPubKey;
+  UINT8             *Signature;
+  UINTN             SigSize;
+  UINT8             *Subject;
+  UINTN             SubjectSize;
+  RETURN_STATUS     ReturnStatus;
+  CHAR8             CommonName[64];
+  UINTN             CommonNameSize;
+  CHAR8             OrgName[64];
+  UINTN             OrgNameSize;
+  UNIT_TEST_STATUS  TestStatus;
+
+  RsaPrivKey = NULL;
+  RsaPubKey  = NULL;
+  Signature  = NULL;
+  Subject    = NULL;
+
+  TestStatus = UNIT_TEST_ERROR_TEST_FAILED;
 
   //
   // Retrieve RSA private key from encrypted PEM data.
@@ -281,7 +289,12 @@ TestVerifyRsaCertPkcs1SignVerify (
   UT_ASSERT_NOT_EQUAL (SigSize, 0);
 
   Signature = AllocatePool (SigSize);
-  Status    = RsaPkcs1Sign (RsaPrivKey, MsgHash, SHA1_DIGEST_SIZE, Signature, &SigSize);
+  if (Signature == NULL) {
+    UT_LOG_ERROR ("Failed to allocate memory for Signature.\n");
+    goto Exit;
+  }
+
+  Status = RsaPkcs1Sign (RsaPrivKey, MsgHash, SHA1_DIGEST_SIZE, Signature, &SigSize);
   UT_ASSERT_TRUE (Status);
 
   //
@@ -296,7 +309,12 @@ TestVerifyRsaCertPkcs1SignVerify (
   SubjectSize = 0;
   Status      = X509GetSubjectName (TestCert, sizeof (TestCert), NULL, &SubjectSize);
   Subject     = (UINT8 *)AllocatePool (SubjectSize);
-  Status      = X509GetSubjectName (TestCert, sizeof (TestCert), Subject, &SubjectSize);
+  if (Subject == NULL) {
+    UT_LOG_ERROR ("Failed to allocate memory for Subject.\n");
+    goto Exit;
+  }
+
+  Status = X509GetSubjectName (TestCert, sizeof (TestCert), Subject, &SubjectSize);
   UT_ASSERT_TRUE (Status);
 
   //
@@ -324,15 +342,28 @@ TestVerifyRsaCertPkcs1SignVerify (
   Status = X509VerifyCert (TestCert, sizeof (TestCert), TestCACert, sizeof (TestCACert));
   UT_ASSERT_TRUE (Status);
 
+  TestStatus = UNIT_TEST_PASSED;
+Exit:
   //
   // Release Resources.
   //
-  RsaFree (RsaPubKey);
-  RsaFree (RsaPrivKey);
-  FreePool (Signature);
-  FreePool (Subject);
+  if (Subject != NULL) {
+    FreePool (Subject);
+  }
 
-  return UNIT_TEST_PASSED;
+  if (Signature != NULL) {
+    FreePool (Signature);
+  }
+
+  if (RsaPubKey != NULL) {
+    RsaFree (RsaPubKey);
+  }
+
+  if (RsaPrivKey != NULL) {
+    RsaFree (RsaPrivKey);
+  }
+
+  return TestStatus;
 }
 
 UNIT_TEST_STATUS

CryptoPkg/Test/UnitTest/Library/BaseCryptLib/RsaPssTests.c

@@ -159,7 +159,12 @@ TestVerifyRsaPssSignVerify (
   UT_ASSERT_NOT_EQUAL (SigSize, 0);
 
   Signature = AllocatePool (SigSize);
-  Status    = RsaPssSign (mRsa, PssMessage, sizeof (PssMessage), SHA256_DIGEST_SIZE, SHA256_DIGEST_SIZE, Signature, &SigSize);
+  if (Signature == NULL) {
+    UT_LOG_ERROR ("Failed to allocate memory for Signature");
+    return UNIT_TEST_ERROR_TEST_FAILED;
+  }
+
+  Status = RsaPssSign (mRsa, PssMessage, sizeof (PssMessage), SHA256_DIGEST_SIZE, SHA256_DIGEST_SIZE, Signature, &SigSize);
   UT_ASSERT_TRUE (Status);
 
   //

CryptoPkg/Test/UnitTest/Library/BaseCryptLib/RsaTests.c

@@ -128,7 +128,12 @@ TestVerifyRsaSetGetKeyComponents (
   UT_ASSERT_EQUAL (KeySize, sizeof (RsaN));
 
   KeyBuffer = AllocatePool (KeySize);
-  Status    = RsaGetKey (mRsa, RsaKeyN, KeyBuffer, &KeySize);
+  if (KeyBuffer == NULL) {
+    UT_LOG_ERROR ("Failed to allocate memory for KeyBuffer");
+    return UNIT_TEST_ERROR_TEST_FAILED;
+  }
+
+  Status = RsaGetKey (mRsa, RsaKeyN, KeyBuffer, &KeySize);
   UT_ASSERT_TRUE (Status);
   UT_ASSERT_EQUAL (KeySize, sizeof (RsaN));
 
@@ -148,7 +153,12 @@ TestVerifyRsaSetGetKeyComponents (
   UT_ASSERT_EQUAL (KeySize, sizeof (RsaE));
 
   KeyBuffer = AllocatePool (KeySize);
-  Status    = RsaGetKey (mRsa, RsaKeyE, KeyBuffer, &KeySize);
+  if (KeyBuffer == NULL) {
+    UT_LOG_ERROR ("Failed to allocate memory for KeyBuffer");
+    return UNIT_TEST_ERROR_TEST_FAILED;
+  }
+
+  Status = RsaGetKey (mRsa, RsaKeyE, KeyBuffer, &KeySize);
   UT_ASSERT_TRUE (Status);
   UT_ASSERT_EQUAL (KeySize, sizeof (RsaE));
 
@@ -214,7 +224,12 @@ TestVerifyRsaGenerateKeyComponents (
 
   KeySize   = RSA_MODULUS_LENGTH / 8;
   KeyBuffer = AllocatePool (KeySize);
-  Status    = RsaGetKey (mRsa, RsaKeyE, KeyBuffer, &KeySize);
+  if (KeyBuffer == NULL) {
+    UT_LOG_ERROR ("Failed to allocate memory for KeyBuffer");
+    return UNIT_TEST_ERROR_TEST_FAILED;
+  }
+
+  Status = RsaGetKey (mRsa, RsaKeyE, KeyBuffer, &KeySize);
   UT_ASSERT_TRUE (Status);
   UT_ASSERT_EQUAL (KeySize, 3);
   UT_ASSERT_MEM_EQUAL (KeyBuffer, DefaultPublicKey, 3);
@@ -293,7 +308,12 @@ TestVerifyRsaPkcs1SignVerify (
   UT_ASSERT_NOT_EQUAL (SigSize, 0);
 
   Signature = AllocatePool (SigSize);
-  Status    = RsaPkcs1Sign (mRsa, HashValue, HashSize, Signature, &SigSize);
+  if (Signature == NULL) {
+    UT_LOG_ERROR ("Failed to allocate memory for Signature");
+    return UNIT_TEST_ERROR_TEST_FAILED;
+  }
+
+  Status = RsaPkcs1Sign (mRsa, HashValue, HashSize, Signature, &SigSize);
   UT_ASSERT_TRUE (Status);
   UT_ASSERT_EQUAL (SigSize, sizeof (RsaPkcs1Signature));
 

CryptoPkg/Test/UnitTest/Library/BaseCryptLib/UnitTestMain.c

@@ -26,13 +26,18 @@ UefiTestMain (
   UNIT_TEST_FRAMEWORK_HANDLE  Framework;
 
   DEBUG ((DEBUG_INFO, "%a v%a\n", UNIT_TEST_NAME, UNIT_TEST_VERSION));
-  CreateUnitTest (UNIT_TEST_NAME, UNIT_TEST_VERSION, &Framework);
+  Status = CreateUnitTest (UNIT_TEST_NAME, UNIT_TEST_VERSION, &Framework);
+  if (EFI_ERROR (Status)) {
+    DEBUG ((DEBUG_ERROR, "Failed in CreateUnitTestsfor BaseCryptLib Tests!  Status = %r\n", Status));
+    goto Done;
+  }
 
   //
   // Execute the tests.
   //
   Status = RunAllTestSuites (Framework);
 
+Done:
   if (Framework) {
     FreeUnitTestFramework (Framework);
   }

CryptoPkg/Test/UnitTest/Library/BaseCryptLib/X509Tests.c

@@ -573,7 +573,12 @@ TestVerifyX509 (
   Status      = X509GetIssuerName (mTestCert, sizeof (mTestCert), NULL, &SubjectSize);
   UT_ASSERT_TRUE (!Status);
   Subject = AllocatePool (SubjectSize);
-  Status  = X509GetIssuerName (mTestCert, sizeof (mTestCert), Subject, &SubjectSize);
+  if (Subject == NULL) {
+    ASSERT (Subject != NULL);
+    return UNIT_TEST_ERROR_TEST_FAILED;
+  }
+
+  Status = X509GetIssuerName (mTestCert, sizeof (mTestCert), Subject, &SubjectSize);
   UT_ASSERT_TRUE (Status);
   FreePool (Subject);