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:26 -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 D5DE088ABE; Thu, 18 Apr 2019 17:47:25 +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 D086A600C1; Thu, 18 Apr 2019 17:47:24 +0000 (UTC) From: "Laszlo Ersek" To: edk2-devel-groups-io Cc: Liming Gao , Michael D Kinney Subject: [PATCH v2 2/5] MdePkg/PiFirmwareFile: fix undefined behavior in SECTION_SIZE Date: Thu, 18 Apr 2019 19:47:07 +0200 Message-Id: <20190418174710.12236-3-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.26]); Thu, 18 Apr 2019 17:47:25 +0000 (UTC) Content-Transfer-Encoding: quoted-printable RH covscan justifiedly reports that accessing "EFI_COMMON_SECTION_HEADER.Size", which is of type UINT8[3], through a (UINT32*), is undefined behavior: > Error: OVERRUN (CWE-119): > edk2-89910a39dcfd/OvmfPkg/Sec/SecMain.c:178: overrun-local: Overrunning > array of 3 bytes at byte offset 3 by dereferencing pointer > "(UINT32 *)((EFI_COMMON_SECTION_HEADER *)(UINTN)Section)->Size". > # 176| Section =3D (EFI_COMMON_SECTION_HEADER*)(UINTN) CurrentAd= dress; > # 177| > # 178|-> Size =3D SECTION_SIZE (Section); > # 179| if (Size < sizeof (*Section)) { > # 180| return EFI_VOLUME_CORRUPTED; Fix this by accessing the array elements individually. Cc: Liming Gao Cc: Michael D Kinney Bugzilla: https://bugzilla.tianocore.org/show_bug.cgi?id=3D1710 Issue: scan-1007.txt Signed-off-by: Laszlo Ersek --- Notes: v2: =20 - replace EFI_COMMON_SECTION_HEADER_UNION with individual array eleme= nt access [Jordan, Phil, Mike] MdePkg/Include/Pi/PiFirmwareFile.h | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/MdePkg/Include/Pi/PiFirmwareFile.h b/MdePkg/Include/Pi/PiFir= mwareFile.h index a9f3bcc4eb8e..05470538de42 100644 --- a/MdePkg/Include/Pi/PiFirmwareFile.h +++ b/MdePkg/Include/Pi/PiFirmwareFile.h @@ -480,8 +480,15 @@ typedef struct { CHAR16 VersionString[1]; } EFI_VERSION_SECTION2; =20 -#define SECTION_SIZE(SectionHeaderPtr) \ - ((UINT32) (*((UINT32 *) ((EFI_COMMON_SECTION_HEADER *) (UINTN) Secti= onHeaderPtr)->Size) & 0x00ffffff)) +/// +/// The argument passed as the SectionHeaderPtr parameter to the SECTION= _SIZE() +/// and IS_SECTION2() function-like macros below must not have side effe= cts: +/// SectionHeaderPtr is evaluated multiple times. +/// +#define SECTION_SIZE(SectionHeaderPtr) ((UINT32) ( \ + (((EFI_COMMON_SECTION_HEADER *) (UINTN) (SectionHeaderPtr))->Size[0]= ) | \ + (((EFI_COMMON_SECTION_HEADER *) (UINTN) (SectionHeaderPtr))->Size[1]= << 8) | \ + (((EFI_COMMON_SECTION_HEADER *) (UINTN) (SectionHeaderPtr))->Size[2]= << 16))) =20 #define IS_SECTION2(SectionHeaderPtr) \ (SECTION_SIZE (SectionHeaderPtr) =3D=3D 0x00ffffff) --=20 2.19.1.3.g30247aa5d201