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:38 -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 583EBC062ED6; Fri, 12 Apr 2019 23:31:38 +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 460F56092F; Fri, 12 Apr 2019 23:31:37 +0000 (UTC) From: "Laszlo Ersek" To: edk2-devel-groups-io Cc: Liming Gao , Michael D Kinney Subject: [PATCH 02/10] MdePkg/PiFirmwareFile: fix undefined behavior in SECTION_SIZE Date: Sat, 13 Apr 2019 01:31:20 +0200 Message-Id: <20190412233128.4756-3-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.31]); Fri, 12 Apr 2019 23:31:38 +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 introducing EFI_COMMON_SECTION_HEADER_UNION, and expressing SECTION_SIZE() in terms of "EFI_COMMON_SECTION_HEADER_UNION.Uint32". 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 --- MdePkg/Include/Pi/PiFirmwareFile.h | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/MdePkg/Include/Pi/PiFirmwareFile.h b/MdePkg/Include/Pi/PiFir= mwareFile.h index a9f3bcc4eb8e..4fce8298d1c0 100644 --- a/MdePkg/Include/Pi/PiFirmwareFile.h +++ b/MdePkg/Include/Pi/PiFirmwareFile.h @@ -229,16 +229,24 @@ typedef struct { /// UINT8 Size[3]; EFI_SECTION_TYPE Type; /// /// Declares the section type. /// } EFI_COMMON_SECTION_HEADER; =20 +/// +/// Union that permits accessing EFI_COMMON_SECTION_HEADER as a UINT32 o= bject. +/// +typedef union { + EFI_COMMON_SECTION_HEADER Hdr; + UINT32 Uint32; +} EFI_COMMON_SECTION_HEADER_UNION; + typedef struct { /// /// A 24-bit unsigned integer that contains the total size of the sect= ion in bytes, /// including the EFI_COMMON_SECTION_HEADER. /// UINT8 Size[3]; =20 EFI_SECTION_TYPE Type; @@ -476,17 +484,17 @@ typedef struct { /// A UINT16 that represents a particular build. Subsequent builds hav= e monotonically /// increasing build numbers relative to earlier builds. /// UINT16 BuildNumber; CHAR16 VersionString[1]; } EFI_VERSION_SECTION2; =20 #define SECTION_SIZE(SectionHeaderPtr) \ - ((UINT32) (*((UINT32 *) ((EFI_COMMON_SECTION_HEADER *) (UINTN) Secti= onHeaderPtr)->Size) & 0x00ffffff)) + (((EFI_COMMON_SECTION_HEADER_UNION *) (UINTN) (SectionHeaderPtr))->U= int32 & 0x00ffffff) =20 #define IS_SECTION2(SectionHeaderPtr) \ (SECTION_SIZE (SectionHeaderPtr) =3D=3D 0x00ffffff) =20 #define SECTION2_SIZE(SectionHeaderPtr) \ (((EFI_COMMON_SECTION_HEADER2 *) (UINTN) SectionHeaderPtr)->Extended= Size) =20 #pragma pack() --=20 2.19.1.3.g30247aa5d201