[edk2-devel] [PATCH v2 1/2] SecurityPkg/DxeImageVerificationLib: Fix certificate lookup algorithm

[Marvin Häuser]

The current certificate lookup code does not check the bounds of the
authentication data before accessing it. Abort if the header cannot
fit. Also, the lookup code aborts once the authetication data is
smaller than an algorithm's OID size. As OIDs are variably-sized,
this may cause unexpected authentication failure due to the early
error-exit.

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

Cc: Jiewen Yao <jiewen.yao at intel.com>
Cc: Jian J Wang <jian.j.wang at intel.com>
Cc: Min Xu <min.m.xu at intel.com>
Cc: Vitaly Cheptsov <vit9696 at protonmail.com>
Signed-off-by: Marvin Häuser <mhaeuser at posteo.de>
---
 SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c | 43 +++++++++++---------
 1 file changed, 23 insertions(+), 20 deletions(-)

diff --git a/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c b/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c
index c48861cd6496..6615099baafb 100644
--- a/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c
+++ b/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c
@@ -624,30 +624,33 @@ HashPeImageByType (
 {

   UINT8                     Index;

 

+  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 ((*(AuthData + 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 ((*(AuthData + 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 (AuthDataSize < 32 + mHash[Index].OidLength) {

-      return EFI_UNSUPPORTED;

-    }

-

     if (CompareMem (AuthData + 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 (#78940): https://edk2.groups.io/g/devel/message/78940
Mute This Topic: https://groups.io/mt/84764903/1813853
Group Owner: devel+owner at edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [edk2-devel-archive at redhat.com]
-=-=-=-=-=-=-=-=-=-=-=-