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; Thu, 18 Apr 2019 10:47:30 -0700 Received: from smtp.corp.redhat.com (int-mx01.intmail.prod.int.phx2.redhat.com [10.5.11.11]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 452CF5859C; Thu, 18 Apr 2019 17:47:30 +0000 (UTC) Received: from lacos-laptop-7.usersys.redhat.com (ovpn-120-179.rdu2.redhat.com [10.10.120.179]) by smtp.corp.redhat.com (Postfix) with ESMTP id 3F02E600C1; Thu, 18 Apr 2019 17:47:29 +0000 (UTC) From: "Laszlo Ersek" To: edk2-devel-groups-io Cc: Ard Biesheuvel , Jordan Justen Subject: [PATCH v2 5/5] OvmfPkg/Sec: fix out-of-bounds reads Date: Thu, 18 Apr 2019 19:47:10 +0200 Message-Id: <20190418174710.12236-6-lersek@redhat.com> In-Reply-To: <20190418174710.12236-1-lersek@redhat.com> References: <20190418174710.12236-1-lersek@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.79 on 10.5.11.11 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.39]); Thu, 18 Apr 2019 17:47:30 +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 Acked-by: Ard Biesheuvel Reviewed-by: Philippe Mathieu-Daude --- Notes: v2: =20 - pick up Ard's A-b =20 - pick up Phil's R-b 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 @@ -274,7 +274,7 @@ FindFfsFileAndSection ( } =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; } @@ -605,7 +605,7 @@ FindImageBase ( } =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; } @@ -630,7 +630,7 @@ FindImageBase ( 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 2.19.1.3.g30247aa5d201