Fix CVE-2024-38797

This commit is contained in:
hy 2025-04-27 18:51:59 +08:00
parent 84833afc7b
commit 461783b40e
2 changed files with 192 additions and 1 deletions

View File

@ -0,0 +1,185 @@
From 6460d06c6f028154088ea7db4a44821ffabfe9e6 Mon Sep 17 00:00:00 2001
From: hy <941973499@qq.com>
Date: Sat, 26 Apr 2025 23:38:23 +0800
Subject: [PATCH] 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>
---
.../DxeImageVerificationLib.c | 37 ++++++++++---------
SecurityPkg/SecurityFixes.yaml | 15 ++++++++
.../SecureBootConfigImpl.c | 37 +++++++++++--------
3 files changed, 55 insertions(+), 34 deletions(-)
diff --git a/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c b/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c
index 5d8dbd54..157318b1 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 ((*(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) {
diff --git a/SecurityPkg/SecurityFixes.yaml b/SecurityPkg/SecurityFixes.yaml
index ceaaa256..0b24844d 100644
--- a/SecurityPkg/SecurityFixes.yaml
+++ b/SecurityPkg/SecurityFixes.yaml
@@ -34,3 +34,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/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigImpl.c b/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigImpl.c
index 0e31502b..02aa142b 100644
--- a/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigImpl.c
+++ b/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigImpl.c
@@ -2079,30 +2079,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.33.0

View File

@ -7,7 +7,7 @@
Name: edk2
Version: %{stable_date}
Release: 23
Release: 24
Summary: EFI Development Kit II
License: BSD-2-Clause-Patent and OpenSSL and MIT
URL: https://github.com/tianocore/edk2
@ -143,6 +143,9 @@ patch86: 0086-Check-DSA-parameters-for-excessive-sizes-before-vali.patch
# Fix CVE-2024-9143
patch87: 0087-Harden-BN_GF2m_poly2arr-against-misuse.patch
# Fix CVE-2024-38797
patch88: 0088-SecurityPkg-Out-of-bound-read-in-HashPeImageByType.patch
BuildRequires: acpica-tools gcc gcc-c++ libuuid-devel python3 bc nasm python3-unversioned-command isl
%description
@ -444,6 +447,9 @@ chmod +x %{buildroot}%{_bindir}/Rsa2048Sha256GenerateKeys
%endif
%changelog
* Sun Apr 27 2025 huyu<huyu70@h-partners.com> - 202308-24
- fix CVE-2024-38797
* Fri Mar 28 2025 huyu<huyu70@h-partners.com> - 202308-23
- fix CVE-2024-9143