From b9d4847ec25816483d78ec94ac22836e1e8aa749 Mon Sep 17 00:00:00 2001 From: Laszlo Ersek Date: Fri, 12 Apr 2019 15:55:28 +0200 Subject: OvmfPkg/Sec: fix out-of-bounds reads RH covscan justifiedly reports that accessing "EFI_FFS_FILE_HEADER.Size" and "EFI_COMMON_SECTION_HEADER.Size", which both are of type UINT8[3], through (UINT32*), is undefined behavior: > Error: OVERRUN (CWE-119): > edk2-89910a39dcfd/OvmfPkg/Sec/SecMain.c:283: overrun-local: Overrunning > array of 3 bytes at byte offset 3 by dereferencing pointer > "(UINT32 *)File->Size". > # 281| > # 282| File = (EFI_FFS_FILE_HEADER*)(UINTN) CurrentAddress; > # 283|-> Size = *(UINT32*) File->Size & 0xffffff; > # 284| if (Size < (sizeof (*File) + sizeof (EFI_COMMON_SECTION_HEADER))) { > # 285| return EFI_VOLUME_CORRUPTED; > > Error: OVERRUN (CWE-119): > edk2-89910a39dcfd/OvmfPkg/Sec/SecMain.c:614: overrun-local: Overrunning > array of 3 bytes at byte offset 3 by dereferencing pointer > "(UINT32 *)File->Size". > # 612| > # 613| File = (EFI_FFS_FILE_HEADER*)(UINTN) CurrentAddress; > # 614|-> Size = *(UINT32*) File->Size & 0xffffff; > # 615| if (Size < sizeof (*File)) { > # 616| return EFI_NOT_FOUND; > > Error: OVERRUN (CWE-119): > edk2-89910a39dcfd/OvmfPkg/Sec/SecMain.c:639: overrun-local: Overrunning > array of 3 bytes at byte offset 3 by dereferencing pointer > "(UINT32 *)Section->Size". > # 637| Section = (EFI_COMMON_SECTION_HEADER*)(UINTN) CurrentAddress; > # 638| > # 639|-> Size = *(UINT32*) Section->Size & 0xffffff; > # 640| if (Size < sizeof (*Section)) { > # 641| return EFI_NOT_FOUND; Fix these by invoking the FFS_FILE_SIZE() and SECTION_SIZE() macros, which by now have been fixed too. Cc: Ard Biesheuvel Cc: Jordan Justen Bugzilla: https://bugzilla.tianocore.org/show_bug.cgi?id=1710 Issue: scan-1008.txt Issue: scan-1009.txt Issue: scan-1010.txt Signed-off-by: Laszlo Ersek Acked-by: Ard Biesheuvel Reviewed-by: Philippe Mathieu-Daude Reviewed-by: Michael D Kinney Reviewed-by: Jordan Justen --- OvmfPkg/Sec/SecMain.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) (limited to 'OvmfPkg') diff --git a/OvmfPkg/Sec/SecMain.c b/OvmfPkg/Sec/SecMain.c index 18a89c649f..3914355cd1 100644 --- a/OvmfPkg/Sec/SecMain.c +++ b/OvmfPkg/Sec/SecMain.c @@ -274,7 +274,7 @@ FindFfsFileAndSection ( } File = (EFI_FFS_FILE_HEADER*)(UINTN) CurrentAddress; - Size = *(UINT32*) File->Size & 0xffffff; + Size = FFS_FILE_SIZE (File); if (Size < (sizeof (*File) + sizeof (EFI_COMMON_SECTION_HEADER))) { return EFI_VOLUME_CORRUPTED; } @@ -605,7 +605,7 @@ FindImageBase ( } File = (EFI_FFS_FILE_HEADER*)(UINTN) CurrentAddress; - Size = *(UINT32*) File->Size & 0xffffff; + Size = FFS_FILE_SIZE (File); if (Size < sizeof (*File)) { return EFI_NOT_FOUND; } @@ -630,7 +630,7 @@ FindImageBase ( CurrentAddress = (EndOfSection + 3) & 0xfffffffffffffffcULL; Section = (EFI_COMMON_SECTION_HEADER*)(UINTN) CurrentAddress; - Size = *(UINT32*) Section->Size & 0xffffff; + Size = SECTION_SIZE (Section); if (Size < sizeof (*Section)) { return EFI_NOT_FOUND; } -- cgit v1.2.3