SecurityPkg/DxeImageVerificationLib: refactor db/dbx fetching code (CVE-2019-14575)

[mirror_edk2.git] / SecurityPkg / Library / DxeImageVerificationLib / DxeImageVerificationLib.c

diff --git a/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c b/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c
index 74dbffa1227f657db50004feba97e61d32c1ac5b..8739d1fa294612ebb930143bac7d157532ec6550 100644 (file)
--- a/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c
+++ b/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c
@@ -1326,7 +1326,7 @@ IsForbiddenByDbx (
   //       UINT8  Certn[];\r
   //\r
   Pkcs7GetSigners (AuthData, AuthDataSize, &CertBuffer, &BufferLength, &TrustedCert, &TrustedCertLength);\r
-  if ((BufferLength == 0) || (CertBuffer == NULL)) {\r
+  if ((BufferLength == 0) || (CertBuffer == NULL) || (*CertBuffer) == 0) {\r
     IsForbidden = TRUE;\r
     goto Done;\r
   }\r
@@ -1412,64 +1412,92 @@ IsAllowedByDb (
   RootCertSize      = 0;\r
   VerifyStatus      = FALSE;\r
 \r
+  //\r
+  // Fetch 'db' content. If 'db' doesn't exist or encounters problem to get the\r
+  // data, return not-allowed-by-db (FALSE).\r
+  //\r
   DataSize = 0;\r
   Status   = gRT->GetVariable (EFI_IMAGE_SECURITY_DATABASE, &gEfiImageSecurityDatabaseGuid, NULL, &DataSize, NULL);\r
-  if (Status == EFI_BUFFER_TOO_SMALL) {\r
-    Data = (UINT8 *) AllocateZeroPool (DataSize);\r
-    if (Data == NULL) {\r
-      return VerifyStatus;\r
+  ASSERT (EFI_ERROR (Status));\r
+  if (Status != EFI_BUFFER_TOO_SMALL) {\r
+    return VerifyStatus;\r
+  }\r
+\r
+  Data = (UINT8 *) AllocateZeroPool (DataSize);\r
+  if (Data == NULL) {\r
+    return VerifyStatus;\r
+  }\r
+\r
+  Status = gRT->GetVariable (EFI_IMAGE_SECURITY_DATABASE, &gEfiImageSecurityDatabaseGuid, NULL, &DataSize, (VOID *) Data);\r
+  if (EFI_ERROR (Status)) {\r
+    goto Done;\r
+  }\r
+\r
+  //\r
+  // Fetch 'dbx' content. If 'dbx' doesn't exist, continue to check 'db'.\r
+  // If any other errors occured, no need to check 'db' but just return\r
+  // not-allowed-by-db (FALSE) to avoid bypass.\r
+  //\r
+  DbxDataSize = 0;\r
+  Status      = gRT->GetVariable (EFI_IMAGE_SECURITY_DATABASE1, &gEfiImageSecurityDatabaseGuid, NULL, &DbxDataSize, NULL);\r
+  ASSERT (EFI_ERROR (Status));\r
+  if (Status != EFI_BUFFER_TOO_SMALL) {\r
+    if (Status != EFI_NOT_FOUND) {\r
+      goto Done;\r
+    }\r
+    //\r
+    // 'dbx' does not exist. Continue to check 'db'.\r
+    //\r
+  } else {\r
+    //\r
+    // 'dbx' exists. Get its content.\r
+    //\r
+    DbxData = (UINT8 *) AllocateZeroPool (DbxDataSize);\r
+    if (DbxData == NULL) {\r
+      goto Done;\r
     }\r
 \r
-    Status = gRT->GetVariable (EFI_IMAGE_SECURITY_DATABASE, &gEfiImageSecurityDatabaseGuid, NULL, &DataSize, (VOID *) Data);\r
+    Status = gRT->GetVariable (EFI_IMAGE_SECURITY_DATABASE1, &gEfiImageSecurityDatabaseGuid, NULL, &DbxDataSize, (VOID *) DbxData);\r
     if (EFI_ERROR (Status)) {\r
       goto Done;\r
     }\r
+  }\r
 \r
-    //\r
-    // Find X509 certificate in Signature List to verify the signature in pkcs7 signed data.\r
-    //\r
-    CertList = (EFI_SIGNATURE_LIST *) Data;\r
-    while ((DataSize > 0) && (DataSize >= CertList->SignatureListSize)) {\r
-      if (CompareGuid (&CertList->SignatureType, &gEfiCertX509Guid)) {\r
-        CertData  = (EFI_SIGNATURE_DATA *) ((UINT8 *) CertList + sizeof (EFI_SIGNATURE_LIST) + CertList->SignatureHeaderSize);\r
-        CertCount = (CertList->SignatureListSize - sizeof (EFI_SIGNATURE_LIST) - CertList->SignatureHeaderSize) / CertList->SignatureSize;\r
+  //\r
+  // Find X509 certificate in Signature List to verify the signature in pkcs7 signed data.\r
+  //\r
+  CertList = (EFI_SIGNATURE_LIST *) Data;\r
+  while ((DataSize > 0) && (DataSize >= CertList->SignatureListSize)) {\r
+    if (CompareGuid (&CertList->SignatureType, &gEfiCertX509Guid)) {\r
+      CertData  = (EFI_SIGNATURE_DATA *) ((UINT8 *) CertList + sizeof (EFI_SIGNATURE_LIST) + CertList->SignatureHeaderSize);\r
+      CertCount = (CertList->SignatureListSize - sizeof (EFI_SIGNATURE_LIST) - CertList->SignatureHeaderSize) / CertList->SignatureSize;\r
 \r
-        for (Index = 0; Index < CertCount; Index++) {\r
-          //\r
-          // Iterate each Signature Data Node within this CertList for verify.\r
-          //\r
-          RootCert     = CertData->SignatureData;\r
-          RootCertSize = CertList->SignatureSize - sizeof (EFI_GUID);\r
+      for (Index = 0; Index < CertCount; Index++) {\r
+        //\r
+        // Iterate each Signature Data Node within this CertList for verify.\r
+        //\r
+        RootCert     = CertData->SignatureData;\r
+        RootCertSize = CertList->SignatureSize - sizeof (EFI_GUID);\r
 \r
+        //\r
+        // Call AuthenticodeVerify library to Verify Authenticode struct.\r
+        //\r
+        VerifyStatus = AuthenticodeVerify (\r
+                         AuthData,\r
+                         AuthDataSize,\r
+                         RootCert,\r
+                         RootCertSize,\r
+                         mImageDigest,\r
+                         mImageDigestSize\r
+                         );\r
+        if (VerifyStatus) {\r
           //\r
-          // Call AuthenticodeVerify library to Verify Authenticode struct.\r
+          // The image is signed and its signature is found in 'db'.\r
           //\r
-          VerifyStatus = AuthenticodeVerify (\r
-                           AuthData,\r
-                           AuthDataSize,\r
-                           RootCert,\r
-                           RootCertSize,\r
-                           mImageDigest,\r
-                           mImageDigestSize\r
-                           );\r
-          if (VerifyStatus) {\r
+          if (DbxData != NULL) {\r
             //\r
             // Here We still need to check if this RootCert's Hash is revoked\r
             //\r
-            Status   = gRT->GetVariable (EFI_IMAGE_SECURITY_DATABASE1, &gEfiImageSecurityDatabaseGuid, NULL, &DbxDataSize, NULL);\r
-            if (Status == EFI_BUFFER_TOO_SMALL) {\r
-              goto Done;\r
-            }\r
-            DbxData = (UINT8 *) AllocateZeroPool (DbxDataSize);\r
-            if (DbxData == NULL) {\r
-              goto Done;\r
-            }\r
-\r
-            Status = gRT->GetVariable (EFI_IMAGE_SECURITY_DATABASE1, &gEfiImageSecurityDatabaseGuid, NULL, &DbxDataSize, (VOID *) DbxData);\r
-            if (EFI_ERROR (Status)) {\r
-              goto Done;\r
-            }\r
-\r
             if (IsCertHashFoundInDatabase (RootCert, RootCertSize, (EFI_SIGNATURE_LIST *)DbxData, DbxDataSize, &RevocationTime)) {\r
               //\r
               // Check the timestamp signature and signing time to determine if the RootCert can be trusted.\r
@@ -1479,17 +1507,23 @@ IsAllowedByDb (
                 DEBUG ((DEBUG_INFO, "DxeImageVerificationLib: Image is signed and signature is accepted by DB, but its root cert failed the timestamp check.\n"));\r
               }\r
             }\r
-\r
-            goto Done;\r
           }\r
 \r
-          CertData = (EFI_SIGNATURE_DATA *) ((UINT8 *) CertData + CertList->SignatureSize);\r
+          //\r
+          // There's no 'dbx' to check revocation time against (must-be pass),\r
+          // or, there's revocation time found in 'dbx' and checked againt 'dbt'\r
+          // (maybe pass or fail, depending on timestamp compare result). Either\r
+          // way the verification job has been completed at this point.\r
+          //\r
+          goto Done;\r
         }\r
-      }\r
 \r
-      DataSize -= CertList->SignatureListSize;\r
-      CertList = (EFI_SIGNATURE_LIST *) ((UINT8 *) CertList + CertList->SignatureListSize);\r
+        CertData = (EFI_SIGNATURE_DATA *) ((UINT8 *) CertData + CertList->SignatureSize);\r
+      }\r
     }\r
+\r
+    DataSize -= CertList->SignatureListSize;\r
+    CertList = (EFI_SIGNATURE_LIST *) ((UINT8 *) CertList + CertList->SignatureListSize);\r
   }\r
 \r
 Done:\r