* Re: [OE-core] [PATCH] ovmf: fix CVE-2024-38797
[not found] <18488187B35E401C.461@lists.openembedded.org>
@ 2025-06-13 5:02 ` Hongxu Jia
2025-06-13 15:47 ` Mathieu Dubois-Briand
0 siblings, 1 reply; 2+ messages in thread
From: Hongxu Jia @ 2025-06-13 5:02 UTC (permalink / raw)
To: openembedded-core
Also submitted to personal github to assure `git am --keep-cr' work
https://github.com/hongxu-jia/openembedded-core/commit/c1d0bf8c7e17eee0a3dea96db18675af62ddfc1e
//Hongxu
On 6/13/25 13:01, hongxu via lists.openembedded.org wrote:
> 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 --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"
>
> -=-=-=-=-=-=-=-=-=-=-=-
> Links: You receive all messages sent to this group.
> View/Reply Online (#218574): https://lists.openembedded.org/g/openembedded-core/message/218574
> Mute This Topic: https://lists.openembedded.org/mt/113619744/3617049
> Group Owner: openembedded-core+owner@lists.openembedded.org
> Unsubscribe: https://lists.openembedded.org/g/openembedded-core/unsub [hongxu.jia@eng.windriver.com]
> -=-=-=-=-=-=-=-=-=-=-=-
>
^ permalink raw reply [flat|nested] 2+ messages in thread