From mboxrd@z Thu Jan 1 00:00:00 1970 Authentication-Results: mx.groups.io; dkim=missing; spf=pass (domain: redhat.com, ip: 209.132.183.28, mailfrom: lersek@redhat.com) Received: from mx1.redhat.com (mx1.redhat.com [209.132.183.28]) by groups.io with SMTP; Fri, 12 Apr 2019 16:31:43 -0700 Received: from smtp.corp.redhat.com (int-mx03.intmail.prod.int.phx2.redhat.com [10.5.11.13]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id E6484309266A; Fri, 12 Apr 2019 23:31:42 +0000 (UTC) Received: from lacos-laptop-7.usersys.redhat.com (ovpn-120-65.rdu2.redhat.com [10.10.120.65]) by smtp.corp.redhat.com (Postfix) with ESMTP id D715E6090C; Fri, 12 Apr 2019 23:31:41 +0000 (UTC) From: "Laszlo Ersek" To: edk2-devel-groups-io Cc: Ard Biesheuvel , Jordan Justen Subject: [PATCH 05/10] OvmfPkg/Sec: fix out-of-bounds reads Date: Sat, 13 Apr 2019 01:31:23 +0200 Message-Id: <20190412233128.4756-6-lersek@redhat.com> In-Reply-To: <20190412233128.4756-1-lersek@redhat.com> References: <20190412233128.4756-1-lersek@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.79 on 10.5.11.13 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.43]); Fri, 12 Apr 2019 23:31:42 +0000 (UTC) Content-Transfer-Encoding: quoted-printable 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 =3D (EFI_FFS_FILE_HEADER*)(UINTN) CurrentAddress; > # 283|-> Size =3D *(UINT32*) File->Size & 0xffffff; > # 284| if (Size < (sizeof (*File) + sizeof (EFI_COMMON_SECTION_H= EADER))) { > # 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 =3D (EFI_FFS_FILE_HEADER*)(UINTN) CurrentAddress; > # 614|-> Size =3D *(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 =3D (EFI_COMMON_SECTION_HEADER*)(UINTN) Current= Address; > # 638| > # 639|-> Size =3D *(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, whic= h by now have been fixed too. Cc: Ard Biesheuvel Cc: Jordan Justen Bugzilla: https://bugzilla.tianocore.org/show_bug.cgi?id=3D1710 Issue: scan-1008.txt Issue: scan-1009.txt Issue: scan-1010.txt Signed-off-by: Laszlo Ersek --- OvmfPkg/Sec/SecMain.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/OvmfPkg/Sec/SecMain.c b/OvmfPkg/Sec/SecMain.c index 18a89c649fd4..3914355cd17b 100644 --- a/OvmfPkg/Sec/SecMain.c +++ b/OvmfPkg/Sec/SecMain.c @@ -269,17 +269,17 @@ FindFfsFileAndSection ( for (EndOfFile =3D CurrentAddress + Fv->HeaderLength; ; ) { =20 CurrentAddress =3D (EndOfFile + 7) & ~(7ULL); if (CurrentAddress > EndOfFirmwareVolume) { return EFI_VOLUME_CORRUPTED; } =20 File =3D (EFI_FFS_FILE_HEADER*)(UINTN) CurrentAddress; - Size =3D *(UINT32*) File->Size & 0xffffff; + Size =3D FFS_FILE_SIZE (File); if (Size < (sizeof (*File) + sizeof (EFI_COMMON_SECTION_HEADER))) { return EFI_VOLUME_CORRUPTED; } =20 EndOfFile =3D CurrentAddress + Size; if (EndOfFile > EndOfFirmwareVolume) { return EFI_VOLUME_CORRUPTED; } @@ -600,17 +600,17 @@ FindImageBase ( for (EndOfFile =3D CurrentAddress + BootFirmwareVolumePtr->HeaderLengt= h; ; ) { =20 CurrentAddress =3D (EndOfFile + 7) & 0xfffffffffffffff8ULL; if (CurrentAddress > EndOfFirmwareVolume) { return EFI_NOT_FOUND; } =20 File =3D (EFI_FFS_FILE_HEADER*)(UINTN) CurrentAddress; - Size =3D *(UINT32*) File->Size & 0xffffff; + Size =3D FFS_FILE_SIZE (File); if (Size < sizeof (*File)) { return EFI_NOT_FOUND; } =20 EndOfFile =3D CurrentAddress + Size; if (EndOfFile > EndOfFirmwareVolume) { return EFI_NOT_FOUND; } @@ -625,17 +625,17 @@ FindImageBase ( // // Loop through the FFS file sections within the FFS file // EndOfSection =3D (EFI_PHYSICAL_ADDRESS)(UINTN) (File + 1); for (;;) { CurrentAddress =3D (EndOfSection + 3) & 0xfffffffffffffffcULL; Section =3D (EFI_COMMON_SECTION_HEADER*)(UINTN) CurrentAddress; =20 - Size =3D *(UINT32*) Section->Size & 0xffffff; + Size =3D SECTION_SIZE (Section); if (Size < sizeof (*Section)) { return EFI_NOT_FOUND; } =20 EndOfSection =3D CurrentAddress + Size; if (EndOfSection > EndOfFile) { return EFI_NOT_FOUND; } --=20 2.19.1.3.g30247aa5d201