From 9989454219e751a05bfac40605b1d22008c7bc29 Mon Sep 17 00:00:00 2001 From: Gowtham M Date: Tue, 4 Nov 2025 11:10:16 +0530 Subject: [PATCH] EmulatorPkg/SecPeiServicesLib: Prevent overread with available size macros MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Symptom:Unsafe typecasting may lead to out‑of‑bound memory access RootCause: FileSize, FileLength and SectionLength are declared as UINT32 and masked with 0x00FFFFFF to store only the lower 24 bits. Although this approach yields the correct result, it introduces a potential risk due to unsafe typecasting and dereferencing. Solution: Using the predefined macro FFS_FILE_SIZE() and SECTION_SIZE from MdePkg\Include\Pi\PiFirmwareFile.h, which safely performs the same operation by reconstructing the size using individual byte access. This commit also addresses the fix for coverity issue "OVERRUN" Cc: Sachin Ganesh Signed-off-by: Gowtham M --- EmulatorPkg/Library/SecPeiServicesLib/FwVol.c | 15 ++++++--------- 1 file changed, 6 insertions(+), 9 deletions(-) diff --git a/EmulatorPkg/Library/SecPeiServicesLib/FwVol.c b/EmulatorPkg/Library/SecPeiServicesLib/FwVol.c index e1cc6c6a51..a3297c14c0 100644 --- a/EmulatorPkg/Library/SecPeiServicesLib/FwVol.c +++ b/EmulatorPkg/Library/SecPeiServicesLib/FwVol.c @@ -8,6 +8,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent **/ #include +#include #define GET_OCCUPIED_SIZE(ActualSize, Alignment) \ (ActualSize) + (((Alignment) - ((ActualSize) & ((Alignment) - 1))) & ((Alignment) - 1)) @@ -158,10 +159,9 @@ Returns: FfsFileHeader = (EFI_FFS_FILE_HEADER *)((UINT8 *)FwVolHeader + FwVolHeader->HeaderLength); } else { // - // Length is 24 bits wide so mask upper 8 bits // FileLength is adjusted to FileOccupiedSize as it is 8 byte aligned. // - FileLength = *(UINT32 *)(*FileHeader)->Size & 0x00FFFFFF; + FileLength = FFS_FILE_SIZE (*FileHeader); FileOccupiedSize = GET_OCCUPIED_SIZE (FileLength, 8); FfsFileHeader = (EFI_FFS_FILE_HEADER *)((UINT8 *)*FileHeader + FileOccupiedSize); } @@ -183,7 +183,7 @@ Returns: case EFI_FILE_DATA_VALID: case EFI_FILE_MARKED_FOR_UPDATE: if (CalculateHeaderChecksum (FfsFileHeader) == 0) { - FileLength = *(UINT32 *)(FfsFileHeader->Size) & 0x00FFFFFF; + FileLength = FFS_FILE_SIZE (FfsFileHeader); FileOccupiedSize = GET_OCCUPIED_SIZE (FileLength, 8); if ((SearchType == FfsFileHeader->Type) || (SearchType == EFI_FV_FILETYPE_ALL)) { @@ -201,7 +201,7 @@ Returns: break; case EFI_FILE_DELETED: - FileLength = *(UINT32 *)(FfsFileHeader->Size) & 0x00FFFFFF; + FileLength = FFS_FILE_SIZE (FfsFileHeader); FileOccupiedSize = GET_OCCUPIED_SIZE (FileLength, 8); FileOffset += FileOccupiedSize; FfsFileHeader = (EFI_FFS_FILE_HEADER *)((UINT8 *)FfsFileHeader + FileOccupiedSize); @@ -246,12 +246,10 @@ Returns: UINT32 ParsedLength; // - // Size is 24 bits wide so mask upper 8 bits. - // Does not include FfsFileHeader header size // FileSize is adjusted to FileOccupiedSize as it is 8 byte aligned. // Section = (EFI_COMMON_SECTION_HEADER *)(FfsFileHeader + 1); - FileSize = *(UINT32 *)(FfsFileHeader->Size) & 0x00FFFFFF; + FileSize = FFS_FILE_SIZE (FfsFileHeader); FileSize -= sizeof (EFI_FFS_FILE_HEADER); *SectionData = NULL; @@ -263,11 +261,10 @@ Returns: } // - // Size is 24 bits wide so mask upper 8 bits. // SectionLength is adjusted it is 4 byte aligned. // Go to the next section // - SectionLength = *(UINT32 *)Section->Size & 0x00FFFFFF; + SectionLength = SECTION_SIZE (Section); SectionLength = GET_OCCUPIED_SIZE (SectionLength, 4); ParsedLength += SectionLength;