public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: "Marvin Häuser" <mhaeuser@posteo.de>
To: devel@edk2.groups.io
Cc: Jiewen Yao <jiewen.yao@intel.com>,
	Jian J Wang <jian.j.wang@intel.com>, Min Xu <min.m.xu@intel.com>,
	Vitaly Cheptsov <vit9696@protonmail.com>
Subject: [PATCH v2 2/2] SecurityPkg/SecureBootConfigDxe: Fix certificate lookup algorithm
Date: Mon,  9 Aug 2021 09:51:26 +0000	[thread overview]
Message-ID: <7cedc9b336ec5410d833b4ecac53f5b366a636a5.1628501623.git.mhaeuser@posteo.de> (raw)
In-Reply-To: <c004d6524b591d813d0f33d78a2a94263c4471ba.1628501623.git.mhaeuser@posteo.de>

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


  reply	other threads:[~2021-08-09  9:51 UTC|newest]

Thread overview: 42+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-08-09  9:51 [PATCH v2 0/7] Fix various issues regarding DebugImageInfoTable Marvin Häuser
2021-08-09  9:51 ` [PATCH v2 1/2] BaseTools: Define the read-only data section name per toolchain Marvin Häuser
2021-08-09  9:51   ` [PATCH v2 2/2] UefiCpuPkg/BaseUefiCpuLib: Use toolchain-specific rodata section name Marvin Häuser
2021-08-10  2:43     ` Ni, Ray
2021-08-10  4:40       ` [edk2-devel] " Andrew Fish
2021-08-10  8:43         ` Marvin Häuser
2021-08-10  4:19   ` [edk2-devel] [PATCH v2 1/2] BaseTools: Define the read-only data section name per toolchain Andrew Fish
2021-08-10  8:27     ` Marvin Häuser
2021-08-10 19:35       ` Andrew Fish
2021-08-10 21:30         ` Marvin Häuser
2021-08-10 21:58           ` Andrew Fish
2021-08-11  8:11             ` Marvin Häuser
2021-08-11 17:19               ` Andrew Fish
2021-08-12  7:26                 ` Marvin Häuser
2021-08-12 20:25                   ` Marvin Häuser
2021-08-12 22:53                   ` Andrew Fish
     [not found]                   ` <169AB0F8BD9C50BA.13770@groups.io>
2021-08-16 21:13                     ` Andrew Fish
     [not found]       ` <169A090BBBBE12C1.15606@groups.io>
2021-08-10 19:49         ` Andrew Fish
2021-08-10 21:24           ` Marvin Häuser
2021-08-10 21:54             ` Andrew Fish
2021-08-09  9:51 ` [PATCH v2 1/7] MdeModulePkg/DxeCore: Consistent DebugImageInfoTable updates Marvin Häuser
2021-08-09  9:51 ` [PATCH v2 1/2] MdePkg/BaseLib: Fix unaligned API prototypes Marvin Häuser
2021-08-09  9:51   ` [PATCH v2 2/2] BaseTools/CommonLib: " Marvin Häuser
2021-08-09 16:15   ` [PATCH v2 1/2] MdePkg/BaseLib: " Michael D Kinney
2021-08-09 21:32     ` [edk2-devel] " Andrew Fish
2021-08-10  8:53       ` Marvin Häuser
2021-08-10 17:36         ` Andrew Fish
2021-08-10 21:14           ` Marvin Häuser
2021-08-09  9:51 ` [PATCH v2 1/2] SecurityPkg/DxeImageVerificationLib: Fix certificate lookup algorithm Marvin Häuser
2021-08-09  9:51   ` Marvin Häuser [this message]
2021-08-12  1:12     ` [edk2-devel] [PATCH v2 2/2] SecurityPkg/SecureBootConfigDxe: " Min Xu
2021-08-12  1:11   ` [edk2-devel] [PATCH v2 1/2] SecurityPkg/DxeImageVerificationLib: " Min Xu
2021-08-09  9:51 ` [PATCH v2 2/7] MdeModulePkg/DxeCore: Fix DebugImageInfoTable size report Marvin Häuser
2021-08-09  9:51 ` [PATCH v2 3/7] EmbeddedPkg/GdbStub: Check DebugImageInfoTable type safely Marvin Häuser
2021-08-09  9:51 ` [PATCH v2 4/7] ArmPkg/DefaultExceptionHandlerLib: " Marvin Häuser
2021-08-09 11:55   ` Ard Biesheuvel
2021-08-09 12:40     ` [edk2-devel] " Marvin Häuser
2021-08-09 21:19       ` Marvin Häuser
2021-08-16  9:50         ` Ard Biesheuvel
2021-08-09  9:51 ` [PATCH v2 5/7] MdeModulePkg/CoreDxe: Mandatory LoadedImage for DebugImageInfoTable Marvin Häuser
2021-08-09  9:51 ` [PATCH v2 6/7] EmbeddedPkg/GdbStub: " Marvin Häuser
2021-08-09  9:51 ` [PATCH v2 7/7] ArmPkg/DefaultExceptionHandlerLib: " Marvin Häuser

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-list from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=7cedc9b336ec5410d833b4ecac53f5b366a636a5.1628501623.git.mhaeuser@posteo.de \
    --to=devel@edk2.groups.io \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox