The current certificate lookup code does not check the bounds of the
authentication data before accessing it. Abort if the header cannot
fit, and proceed to the next hashing algortihm if the OID of the
current one exceeds the authentication data bounds.

Additionally move the two-byte encoding check out of the loop as the
data is invariant.

Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Jian J Wang <jian.j.wang@intel.com>
Cc: Min Xu <min.m.xu@intel.com>
Cc: Vitaly Cheptsov <vit9696@protonmail.com>
Signed-off-by: Marvin Häuser <mhaeuser@posteo.de>
---
 SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigImpl.c | 45 ++++++++++++--------
 1 file changed, 28 insertions(+), 17 deletions(-)

diff --git a/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigImpl.c b/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigImpl.c
index 65a8188d6d03..fd7629f61862 100644
--- a/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigImpl.c
+++ b/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigImpl.c
@@ -1969,30 +1969,41 @@ HashPeImageByType (
 {

   UINT8                     Index;

   WIN_CERTIFICATE_EFI_PKCS  *PkcsCertData;

+  UINT32                    AuthDataSize;

 

   PkcsCertData = (WIN_CERTIFICATE_EFI_PKCS *) (mImageBase + mSecDataDir->Offset);

+  if (PkcsCertData->Hdr.dwLength <= sizeof (PkcsCertData->Hdr)) {

+    return EFI_UNSUPPORTED;

+  }

+

+  AuthDataSize = PkcsCertData->Hdr.dwLength - sizeof (PkcsCertData->Hdr);

+  if (AuthDataSize < 32) {

+    return EFI_UNSUPPORTED;

+  }

+  //

+  // Check the Hash algorithm in PE/COFF Authenticode.

+  //    According to PKCS#7 Definition:

+  //        SignedData ::= SEQUENCE {

+  //            version Version,

+  //            digestAlgorithms DigestAlgorithmIdentifiers,

+  //            contentInfo ContentInfo,

+  //            .... }

+  //    The DigestAlgorithmIdentifiers can be used to determine the hash algorithm in PE/COFF hashing

+  //    This field has the fixed offset (+32) in final Authenticode ASN.1 data.

+  //    Fixed offset (+32) is calculated based on two bytes of length encoding.

+  //

+  if ((*(PkcsCertData->CertData + 1) & TWO_BYTE_ENCODE) != TWO_BYTE_ENCODE) {

+    //

+    // Only support two bytes of Long Form of Length Encoding.

+    //

+    return EFI_UNSUPPORTED;

+  }

 

   for (Index = 0; Index < HASHALG_MAX; Index++) {

-    //

-    // Check the Hash algorithm in PE/COFF Authenticode.

-    //    According to PKCS#7 Definition:

-    //        SignedData ::= SEQUENCE {

-    //            version Version,

-    //            digestAlgorithms DigestAlgorithmIdentifiers,

-    //            contentInfo ContentInfo,

-    //            .... }

-    //    The DigestAlgorithmIdentifiers can be used to determine the hash algorithm in PE/COFF hashing

-    //    This field has the fixed offset (+32) in final Authenticode ASN.1 data.

-    //    Fixed offset (+32) is calculated based on two bytes of length encoding.

-     //

-    if ((*(PkcsCertData->CertData + 1) & TWO_BYTE_ENCODE) != TWO_BYTE_ENCODE) {

-      //

-      // Only support two bytes of Long Form of Length Encoding.

-      //

+    if (AuthDataSize - 32 < mHash[Index].OidLength) {

       continue;

     }

 

-    //

     if (CompareMem (PkcsCertData->CertData + 32, mHash[Index].OidValue, mHash[Index].OidLength) == 0) {

       break;

     }

-- 
2.31.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#78899): https://edk2.groups.io/g/devel/message/78899
Mute This Topic: https://groups.io/mt/84754074/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-