diff --git a/1009-CVE-2024-38797.patch b/1009-CVE-2024-38797.patch new file mode 100644 index 0000000000000000000000000000000000000000..e6b9a4ce9286e2147d7a920cafc16c1fa9bfcf31 --- /dev/null +++ b/1009-CVE-2024-38797.patch @@ -0,0 +1,277 @@ +From 2dcdb41b564aa3cb846644b4b1722a0b3ae5e06b Mon Sep 17 00:00:00 2001 +From: Doug Flick +Date: Thu, 3 Oct 2024 09:37:18 -0700 +Subject: [PATCH 1/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 +Signed-off-by: Doug Flick +--- + .../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 b05da19c2b5f..2afa2c93d9c8 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. + // + +From 5df518ec510324f48ed1cf0376150960644b41f0 Mon Sep 17 00:00:00 2001 +From: Doug Flick +Date: Thu, 3 Oct 2024 10:16:57 -0700 +Subject: [PATCH 2/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 +--- + .../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 2afa2c93d9c8..2eca39d563fe 100644 +--- a/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c ++++ b/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c +@@ -618,6 +618,7 @@ HashPeImage ( + @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) { + +From 8676572908b950dd4d1f8985006011be99c0a5b6 Mon Sep 17 00:00:00 2001 +From: Doug Flick +Date: Fri, 17 Jan 2025 11:30:17 -0800 +Subject: [PATCH 3/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 +--- + .../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 d4dc4e14020c..d262904c3138 100644 +--- a/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigImpl.c ++++ b/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigImpl.c +@@ -2105,30 +2105,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; + } + +From 519366f542e9370bee982b1c3687ffedb5cabc21 Mon Sep 17 00:00:00 2001 +From: Doug Flick +Date: Mon, 7 Apr 2025 11:23:41 -0700 +Subject: [PATCH 4/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 +--- + SecurityPkg/SecurityFixes.yaml | 15 +++++++++++++++ + 1 file changed, 15 insertions(+) + +diff --git a/SecurityPkg/SecurityFixes.yaml b/SecurityPkg/SecurityFixes.yaml +index b4006b42b89e..06b597a43ee5 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 diff --git a/edk2.spec b/edk2.spec index 29f187ffdc63bcce174f6f0ec5a8f2d3aa96c663..24921d6ab0883ba586482ba3c42c31c247ef8576 100644 --- a/edk2.spec +++ b/edk2.spec @@ -1,4 +1,4 @@ -%define anolis_release 16 +%define anolis_release 17 %undefine _auto_set_build_flags ExclusiveArch: x86_64 aarch64 loongarch64 riscv64 @@ -110,6 +110,8 @@ Patch1005: 1005-OvmfPkg-AmdSev-Integrate-grub2-x86_64-efi-modules-fr.patch Patch1006: 1006-MdePkg-Fix-overflow-issue-in-BasePeC.patch #From https://github.com/tianocore/edk2/commit/284dbac43da752ee34825c8b3f6f9e8281cb5a19 Patch1008: 1008-CVE-2024-1298.patch +# https://github.com/tianocore/edk2/pull/10928 +Patch1009: 1009-CVE-2024-38797.patch BuildRequires: python3-devel BuildRequires: libuuid-devel @@ -562,6 +564,9 @@ rm -f %{buildroot}%{_datadir}/edk2/riscv/*.raw %changelog +* Wed Aug 06 2025 wenxin - 202402-17 +- add patch to fix CVE-2024-38797 + * Thu Jul 3 2025 lzq11122 - 202402-16 - fix CVE-2024-1298