CryptoPkg: Additional CodeQL fixes (#279)

# Preface Please ensure you have read the [contribution docs](https://github.com/microsoft/mu/blob/master/CONTRIBUTING.md) prior to submitting the pull request. In particular, [pull request guidelines](https://github.com/microsoft/mu/blob/master/CONTRIBUTING.md#pull-request-best-practices). ## Description <_Please include a description of the change and why this change was made._> For each item, place an "x" in between `[` and `]` if true. Example: `[x]`. _(you can also check items in the GitHub UI)_ - [ ] Impacts functionality? - **Functionality** - Does the change ultimately impact how firmware functions? - Examples: Add a new library, publish a new PPI, update an algorithm, ... - [ ] Impacts security? - **Security** - Does the change have a direct security impact on an application, flow, or firmware? - Examples: Crypto algorithm change, buffer overflow fix, parameter validation improvement, ... - [ ] Breaking change? - **Breaking change** - Will anyone consuming this change experience a break in build or boot behavior? - Examples: Add a new library class, move a module to a different repo, call a function in a new library class in a pre-existing module, ... - [ ] Includes tests? - **Tests** - Does the change include any explicit test code? - Examples: Unit tests, integration tests, robot tests, ... - [ ] Includes documentation? - **Documentation** - Does the change contain explicit documentation additions outside direct code modifications (and comments)? - Examples: Update readme file, add feature readme file, link to documentation on an a separate Web page, ... ## How This Was Tested <_Please describe the test(s) that were run to verify the changes._> ## Integration Instructions <_Describe how these changes should be integrated. Use N/A if nothing is required._> --------- Co-authored-by: Michael Kubacki <michael.kubacki@microsoft.com>
microsoft · Oct 20, 2023 · 25e17a7 · 25e17a7
1 parent 6960ddb
commit 25e17a7
Show file tree

Hide file tree

Showing 4 changed files with 86 additions and 26 deletions.
diff --git a/CryptoPkg/Test/UnitTest/Library/BaseCryptLib/Pkcs5Pbkdf2Tests.c b/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

diff --git a/CryptoPkg/Test/UnitTest/Library/BaseCryptLib/RsaPkcs7Tests.c b/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
@@ -369,8 +400,8 @@ TestVerifyPkcs7SignVerify (
              (CONST UINT8 *)PemPass,
              (UINT8 *)Payload,
              AsciiStrLen (Payload),
-             TestCert,            // MU_CHANGE [TCBZ3925] - Pkcs7Sign is broken
-             sizeof (TestCert),   // MU_CHANGE [TCBZ3925] - Pkcs7Sign is broken
+             TestCert,                   // MU_CHANGE [TCBZ3925] - Pkcs7Sign is broken
+             sizeof (TestCert),          // MU_CHANGE [TCBZ3925] - Pkcs7Sign is broken
              NULL,
              &P7SignedData,
              &P7SignedDataSize

diff --git a/CryptoPkg/Test/UnitTest/Library/BaseCryptLib/RsaPssTests.c b/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);
 
   //

diff --git a/CryptoPkg/Test/UnitTest/Library/BaseCryptLib/RsaTests.c b/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));
 
@@ -204,7 +214,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);
@@ -283,7 +298,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));