diff mbox series

ovmf: fix CVE-2024-38797

Message ID 20250613050113.3022522-1-hongxu.jia@windriver.com
State New
Headers show
Series ovmf: fix CVE-2024-38797 | expand

Commit Message

Hongxu Jia June 13, 2025, 5:01 a.m. UTC
According to [1]:

EDK2 contains a vulnerability in the HashPeImageByType(). A user may cause a read out of
bounds when a corrupted data pointer and length are sent via an adjecent network.
A successful exploit of this vulnerability may lead to a loss of Integrity and/or
Availability.

Backport fixes from upstream edk2 [2][3]

[1] https://nvd.nist.gov/vuln/detail/CVE-2024-38797
[2] https://github.com/tianocore/edk2/security/advisories/GHSA-4wjw-6xmf-44xf
[3] https://github.com/tianocore/edk2/pull/10928

Signed-off-by: Hongxu Jia <hongxu.jia@windriver.com>
---
 .../ovmf/ovmf/CVE-2024-38797-1.patch          | 43 ++++++++
 .../ovmf/ovmf/CVE-2024-38797-2.patch          | 63 ++++++++++++
 .../ovmf/ovmf/CVE-2024-38797-3.patch          | 99 +++++++++++++++++++
 .../ovmf/ovmf/CVE-2024-38797-4.patch          | 97 ++++++++++++++++++
 meta/recipes-core/ovmf/ovmf_git.bb            |  4 +
 5 files changed, 306 insertions(+)
 create mode 100644 meta/recipes-core/ovmf/ovmf/CVE-2024-38797-1.patch
 create mode 100644 meta/recipes-core/ovmf/ovmf/CVE-2024-38797-2.patch
 create mode 100644 meta/recipes-core/ovmf/ovmf/CVE-2024-38797-3.patch
 create mode 100644 meta/recipes-core/ovmf/ovmf/CVE-2024-38797-4.patch
diff mbox series

Patch

diff --git a/meta/recipes-core/ovmf/ovmf/CVE-2024-38797-1.patch b/meta/recipes-core/ovmf/ovmf/CVE-2024-38797-1.patch
new file mode 100644
index 0000000000..066dfa0ff0
--- /dev/null
+++ b/meta/recipes-core/ovmf/ovmf/CVE-2024-38797-1.patch
@@ -0,0 +1,43 @@ 
+From 2c8fb3e5164effc8a370e800fe91db7341e69116 Mon Sep 17 00:00:00 2001
+From: Doug Flick <dougflick@microsoft.com>
+Date: Mon, 7 Apr 2025 11:23:41 -0700
+Subject: [PATCH 1/4] SecurityPkg: Update SecurityFixes.yaml for CVE-2024-38797
+
+This commit updates the SecurityFixes.yaml file to include
+information about the CVE-2024-38797 vulnerability.
+
+Signed-off-by: Doug Flick <DougFlick@microsoft.com>
+
+CVE: CVE-2024-38797
+Upstream-Status: Backport [https://github.com/tianocore/edk2/pull/10928/commits/519366f542e9370bee982b1c3687ffedb5cabc21]
+Signed-off-by: Hongxu Jia <hongxu.jia@windriver.com>
+---
+ SecurityPkg/SecurityFixes.yaml | 15 +++++++++++++++
+ 1 file changed, 15 insertions(+)
+
+diff --git a/SecurityPkg/SecurityFixes.yaml b/SecurityPkg/SecurityFixes.yaml
+index b4006b4..06b597a 100644
+--- a/SecurityPkg/SecurityFixes.yaml
++++ b/SecurityPkg/SecurityFixes.yaml
+@@ -40,3 +40,18 @@ CVE_2022_36764:
+     - Library\DxeTpmMeasureBootLib\DxeTpmMeasureBootLib.c
+   links:
+     - https://bugzilla.tianocore.org/show_bug.cgi?id=4118
++CVE_2024_38797:
++  commit-titles:
++    - "SecurityPkg: Out of bound read in HashPeImageByType()"
++    - "SecurityPkg: Improving HashPeImageByType () logic"
++    - "SecurityPkg: Improving SecureBootConfigImpl:HashPeImageByType () logic"
++  cve: CVE-2024-38797
++  date_reported: 2024-06-04 12:00 UTC
++  description: Out of bound read in HashPeImageByType()
++  note:
++  files_impacted:
++    - SecurityPkg\Library\DxeImageVerificationLib\DxeImageVerificationLib.c
++    - SecurityPkg\VariableAuthenticated\SecureBootConfigDxe\SecureBootConfigImpl.c
++  links:
++    - https://bugzilla.tianocore.org/show_bug.cgi?id=2214
++    - https://github.com/tianocore/edk2/security/advisories/GHSA-4wjw-6xmf-44xf
+-- 
+2.34.1
+
diff --git a/meta/recipes-core/ovmf/ovmf/CVE-2024-38797-2.patch b/meta/recipes-core/ovmf/ovmf/CVE-2024-38797-2.patch
new file mode 100644
index 0000000000..9bf6645681
--- /dev/null
+++ b/meta/recipes-core/ovmf/ovmf/CVE-2024-38797-2.patch
@@ -0,0 +1,63 @@ 
+From 1a7be26382c4a34504875f094e15fe371d44192e Mon Sep 17 00:00:00 2001
+From: Doug Flick <dougflick@microsoft.com>
+Date: Thu, 3 Oct 2024 09:37:18 -0700
+Subject: [PATCH 2/4] SecurityPkg: Out of bound read in HashPeImageByType()
+
+In HashPeImageByType(), the hash of PE/COFF image is calculated.
+This function may get untrusted input.
+
+Inside this function, the following code verifies the loaded image has
+the correct format, by reading the second byte of the buffer.
+
+```c
+  if ((*(AuthData + 1) & TWO_BYTE_ENCODE) != TWO_BYTE_ENCODE) {
+  	...
+  }
+```
+
+The input image is not trusted and that may not have the second byte to
+read. So this poses an out of bound read error.
+
+With below fix we are assuring that we don't do out of bound read. i.e,
+we make sure that AuthDataSize is greater than 1.
+
+```c
+  if (AuthDataSize > 1
+      && (*(AuthData + 1) & TWO_BYTE_ENCODE) != TWO_BYTE_ENCODE){
+    ...
+  }
+```
+
+AuthDataSize size is verified before reading the second byte.
+So if AuthDataSize is less than 2, the second byte will not be read, and
+the out of bound read situation won't occur.
+
+Tested the patch on real platform with and without TPM connected and
+verified image is booting fine.
+
+Authored-by: Raj AlwinX Selvaraj <Alw...@intel.com>
+Signed-off-by: Doug Flick <DougFlick@microsoft.com>
+
+CVE: CVE-2024-38797
+Upstream-Status: Backport [https://github.com/tianocore/edk2/pull/10928/commits/2dcdb41b564aa3cb846644b4b1722a0b3ae5e06b]
+Signed-off-by: Hongxu Jia <hongxu.jia@windriver.com>
+---
+ .../Library/DxeImageVerificationLib/DxeImageVerificationLib.c   | 2 +-
+ 1 file changed, 1 insertion(+), 1 deletion(-)
+
+diff --git a/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c b/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c
+index b05da19..2afa2c9 100644
+--- a/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c
++++ b/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c
+@@ -642,7 +642,7 @@ HashPeImageByType (
+     //    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) {
++    if ((AuthDataSize > 1) && ((*(AuthData + 1) & TWO_BYTE_ENCODE) != TWO_BYTE_ENCODE)) {
+       //
+       // Only support two bytes of Long Form of Length Encoding.
+       //
+-- 
+2.34.1
+
diff --git a/meta/recipes-core/ovmf/ovmf/CVE-2024-38797-3.patch b/meta/recipes-core/ovmf/ovmf/CVE-2024-38797-3.patch
new file mode 100644
index 0000000000..169c78daab
--- /dev/null
+++ b/meta/recipes-core/ovmf/ovmf/CVE-2024-38797-3.patch
@@ -0,0 +1,99 @@ 
+From 4db363db013a92937431234252fc9d84e44fc120 Mon Sep 17 00:00:00 2001
+From: Doug Flick <dougflick@microsoft.com>
+Date: Thu, 3 Oct 2024 10:16:57 -0700
+Subject: [PATCH 3/4] SecurityPkg: Improving HashPeImageByType () logic
+
+Namely:
+
+(1) The TWO_BYTE_ENCODE check is independent of Index. If it evalutes
+    to TRUE for Index==0, then it will evaluate to TRUE for all other
+    Index values as well. As a result, the (Index == HASHALG_MAX)
+    condition will fire after the loop, and we'll return
+    EFI_UNSUPPORTED.
+
+    While this is correct, functionally speaking, it is wasteful to
+    keep re-checking TWO_BYTE_ENCODE in the loop body. The check
+    should be made at the top of the function, and EFI_UNSUPPORTED
+    should be returned at once, if appropriate.
+
+(2) If the hash algorithm selected by Index has such a large OID that
+    the OID comparison cannot even be performed (because AuthDataSize
+    is not large enough for containing the OID in question, starting
+    at offset 32), then the function returns EFI_UNSUPPORTED at once.
+
+    This is bogus; this case should simply be treated as an OID
+    mismatch, and the loop should advance to the next Index value /
+    hash algorithm candidate. A remaining hash algo may have a shorter
+    OID and yield an OID match.
+
+Signed-off-by: Doug Flick <DougFlick@microsoft.com>
+
+CVE: CVE-2024-38797
+Upstream-Status: Backport [https://github.com/tianocore/edk2/pull/10928/commits/5df518ec510324f48ed1cf0376150960644b41f0]
+Signed-off-by: Hongxu Jia <hongxu.jia@windriver.com>
+---
+ .../DxeImageVerificationLib.c                 | 37 ++++++++++---------
+ 1 file changed, 19 insertions(+), 18 deletions(-)
+
+diff --git a/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c b/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c
+index 2afa2c9..2eca39d 100644
+--- a/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c
++++ b/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c
+@@ -618,6 +618,7 @@ Done:
+   @param[in]  AuthDataSize        Size of the Authenticode Signature in bytes.
+ 
+   @retval EFI_UNSUPPORTED             Hash algorithm is not supported.
++  @retval EFI_BAD_BUFFER_SIZE         AuthData provided is invalid size.
+   @retval EFI_SUCCESS                 Hash successfully.
+ 
+ **/
+@@ -629,28 +630,28 @@ HashPeImageByType (
+ {
+   UINT8  Index;
+ 
+-  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 ((AuthDataSize > 1) && ((*(AuthData + 1) & TWO_BYTE_ENCODE) != TWO_BYTE_ENCODE)) {
+     //
+-    // 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.
++    // Only support two bytes of Long Form of Length Encoding.
+     //
+-    if ((AuthDataSize > 1) && ((*(AuthData + 1) & TWO_BYTE_ENCODE) != TWO_BYTE_ENCODE)) {
+-      //
+-      // Only support two bytes of Long Form of Length Encoding.
+-      //
+-      continue;
+-    }
++    return EFI_BAD_BUFFER_SIZE;
++  }
+ 
++  for (Index = 0; Index < HASHALG_MAX; Index++) {
+     if (AuthDataSize < 32 + mHash[Index].OidLength) {
+-      return EFI_UNSUPPORTED;
++      continue;
+     }
+ 
+     if (CompareMem (AuthData + 32, mHash[Index].OidValue, mHash[Index].OidLength) == 0) {
+-- 
+2.34.1
+
diff --git a/meta/recipes-core/ovmf/ovmf/CVE-2024-38797-4.patch b/meta/recipes-core/ovmf/ovmf/CVE-2024-38797-4.patch
new file mode 100644
index 0000000000..86bc950e7d
--- /dev/null
+++ b/meta/recipes-core/ovmf/ovmf/CVE-2024-38797-4.patch
@@ -0,0 +1,97 @@ 
+From cb3342702c5c1f8a4ddbb6d503a98ed720d14eb3 Mon Sep 17 00:00:00 2001
+From: Doug Flick <dougflick@microsoft.com>
+Date: Fri, 17 Jan 2025 11:30:17 -0800
+Subject: [PATCH 4/4] SecurityPkg: Improving
+ SecureBootConfigImpl:HashPeImageByType () logic
+
+Namely:
+
+(1) The TWO_BYTE_ENCODE check is independent of Index. If it evalutes
+    to TRUE for Index==0, then it will evaluate to TRUE for all other
+    Index values as well. As a result, the (Index == HASHALG_MAX)
+    condition will fire after the loop, and we'll return
+    EFI_UNSUPPORTED.
+
+    While this is correct, functionally speaking, it is wasteful to
+    keep re-checking TWO_BYTE_ENCODE in the loop body. The check
+    should be made at the top of the function, and EFI_UNSUPPORTED
+    should be returned at once, if appropriate.
+
+(2) If the hash algorithm selected by Index has such a large OID that
+    the OID comparison cannot even be performed (because AuthDataSize
+    is not large enough for containing the OID in question, starting
+    at offset 32), then the function returns EFI_UNSUPPORTED at once.
+
+    This is bogus; this case should simply be treated as an OID
+    mismatch, and the loop should advance to the next Index value /
+    hash algorithm candidate. A remaining hash algo may have a shorter
+    OID and yield an OID match.
+
+Signed-off-by: Doug Flick <DougFlick@microsoft.com>
+
+CVE: CVE-2024-38797
+Upstream-Status: Backport [https://github.com/tianocore/edk2/pull/10928/commits/8676572908b950dd4d1f8985006011be99c0a5b6]
+Signed-off-by: Hongxu Jia <hongxu.jia@windriver.com>
+---
+ .../SecureBootConfigImpl.c                    | 37 +++++++++++--------
+ 1 file changed, 21 insertions(+), 16 deletions(-)
+
+diff --git a/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigImpl.c b/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigImpl.c
+index 6d4560c..155e755 100644
+--- a/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigImpl.c
++++ b/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigImpl.c
+@@ -2096,30 +2096,35 @@ HashPeImageByType (
+ {
+   UINT8                     Index;
+   WIN_CERTIFICATE_EFI_PKCS  *PkcsCertData;
++  UINT32                    PkcsCertSize;
+ 
+   PkcsCertData = (WIN_CERTIFICATE_EFI_PKCS *)(mImageBase + mSecDataDir->Offset);
++  PkcsCertSize = mSecDataDir->SizeOfCert;
+ 
+-  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 ((PkcsCertSize > 1) && ((*(PkcsCertData->CertData + 1) & TWO_BYTE_ENCODE) != TWO_BYTE_ENCODE)) {
+     //
+-    // 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.
++    // Only support two bytes of Long Form 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_BAD_BUFFER_SIZE;
++  }
++
++  for (Index = 0; Index < HASHALG_MAX; Index++) {
++    if (PkcsCertSize < 32 + mHash[Index].OidLength) {
+       continue;
+     }
+ 
+-    //
+     if (CompareMem (PkcsCertData->CertData + 32, mHash[Index].OidValue, mHash[Index].OidLength) == 0) {
+       break;
+     }
+-- 
+2.34.1
+
diff --git a/meta/recipes-core/ovmf/ovmf_git.bb b/meta/recipes-core/ovmf/ovmf_git.bb
index aa7de3af2b..ab6c580722 100644
--- a/meta/recipes-core/ovmf/ovmf_git.bb
+++ b/meta/recipes-core/ovmf/ovmf_git.bb
@@ -27,6 +27,10 @@  SRC_URI = "gitsm://github.com/tianocore/edk2.git;branch=master;protocol=https \
            file://0003-debug-prefix-map.patch \
            file://0004-reproducible.patch \
            file://CVE-2025-2295.patch \
+           file://CVE-2024-38797-1.patch \
+           file://CVE-2024-38797-2.patch \
+           file://CVE-2024-38797-3.patch \
+           file://CVE-2024-38797-4.patch \
            "
 
 PV = "edk2-stable202502"