From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail02.groups.io (mail02.groups.io [66.175.222.108]) by spool.mail.gandi.net (Postfix) with ESMTPS id 93BB77803D0 for ; Sat, 28 Oct 2023 11:06:48 +0000 (UTC) DKIM-Signature: a=rsa-sha256; bh=I7Uce5O5n1fQlaZKNyPO4X54XhXn6ufXfT9kXSopO+s=; c=relaxed/simple; d=groups.io; h=Message-ID:Date:MIME-Version:Subject:To:Cc:References:From:In-Reply-To:Precedence:List-Subscribe:List-Help:Sender:List-Id:Mailing-List:Delivered-To:Reply-To:List-Unsubscribe-Post:List-Unsubscribe:Content-Language:Content-Type:Content-Transfer-Encoding; s=20140610; t=1698491207; v=1; b=dTFxsUOSFeDuvvWglyN/rnKz6fvQAA/2Mnwopb6B2LB5F5UUDg6ECNuzFZlVwul3J/lBF0iR FCQts3GevV8RC8xxjGzmu9XNEa8/ywpYuFpp7YniMgqxYoncB2b/YmchjEGfQAiCNLsMPXkuF41 laB+aDdDl4+Gs/DsQNwzhQPM= X-Received: by 127.0.0.2 with SMTP id GrgoYY7687511xCjPe1omfqD; Sat, 28 Oct 2023 04:06:47 -0700 X-Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by mx.groups.io with SMTP id smtpd.web10.16075.1698491206394919766 for ; Sat, 28 Oct 2023 04:06:46 -0700 X-Received: from mimecast-mx02.redhat.com (mimecast-mx02.redhat.com [66.187.233.88]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-558-gU3x25riNhW_RZR0zaeYTA-1; Sat, 28 Oct 2023 07:06:34 -0400 X-MC-Unique: gU3x25riNhW_RZR0zaeYTA-1 X-Received: from smtp.corp.redhat.com (int-mx10.intmail.prod.int.rdu2.redhat.com [10.11.54.10]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by mimecast-mx02.redhat.com (Postfix) with ESMTPS id E373585A5BD; Sat, 28 Oct 2023 11:06:33 +0000 (UTC) X-Received: from [10.39.192.70] (unknown [10.39.192.70]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 0829C492BFC; Sat, 28 Oct 2023 11:06:32 +0000 (UTC) Message-ID: <3a4ec85d-9f27-1d48-c57b-463257b74a54@redhat.com> Date: Sat, 28 Oct 2023 13:06:31 +0200 MIME-Version: 1.0 Subject: Re: [edk2-devel] [PATCH 1/1] StandaloneMmPkg: Fix the failure to find uncompressed inner FV. To: "Xu, Wei6" , "devel@edk2.groups.io" Cc: Ard Biesheuvel , Sami Mujawar , "Ni, Ray" References: <74942487-4eca-6414-b3a2-60eff1195301@redhat.com> From: "Laszlo Ersek" In-Reply-To: X-Scanned-By: MIMEDefang 3.4.1 on 10.11.54.10 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Precedence: Bulk List-Subscribe: List-Help: Sender: devel@edk2.groups.io List-Id: Mailing-List: list devel@edk2.groups.io; contact devel+owner@edk2.groups.io Reply-To: devel@edk2.groups.io,lersek@redhat.com List-Unsubscribe-Post: List-Unsubscribe=One-Click List-Unsubscribe: X-Gm-Message-State: yk0EEwdposzOUqu4fyipeCkSx7686176AA= Content-Language: en-US Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable X-GND-Status: LEGIT Authentication-Results: spool.mail.gandi.net; dkim=pass header.d=groups.io header.s=20140610 header.b=dTFxsUOS; dmarc=fail reason="SPF not aligned (relaxed), DKIM not aligned (relaxed)" header.from=redhat.com (policy=none); spf=pass (spool.mail.gandi.net: domain of bounce@groups.io designates 66.175.222.108 as permitted sender) smtp.mailfrom=bounce@groups.io On 10/27/23 04:21, Xu, Wei6 wrote: > Hi Laszlo, >=20 > Thanks a lot for the review. >=20 > I send review the patch v2 to fix: > - memory leaks on error paths > - missing object size checks before casting pointers to header types > (https://edk2.groups.io/g/devel/message/110160) Thanks, will check it. >=20 > Regarding to 'unbounded recursion', I couldn't come up with a good soluti= on to fix the problem, let's fix the others first. We've had the same issue in both the PEI Core and the DXE Core. The PEI core issue was CVE-2018-12183. We have two TianoCore BZs related to that, #1137 and #1126. I don't know / remember how the issue was ultimately fixed. Presumably with the PEI Stack Guard. I don't think that's a great solution, but either way, the issue seems hardly exploitable (because it's arguably not easy for an attacker to inject FVs in the PEI phase). The DXE Core issue was CVE-2021-28210 -- TianoCore BZ#1743. The fix for that was commit range 6c8dd15c4ae4..47343af30435. We introduced PcdFwVolDxeMaxEncapsulationDepth to arbitrarily limit the depth of recursion. It's a practical fix. I think the same approach could be taken in the Standalone MM Core as well. Laszlo >=20 >=20 > BR, > Wei >=20 > -----Original Message----- > From: Laszlo Ersek =20 > Sent: Tuesday, October 24, 2023 8:03 PM > To: devel@edk2.groups.io; Xu, Wei6 > Cc: Ard Biesheuvel ; Sami Mujawar ; Ni, Ray > Subject: Re: [edk2-devel] [PATCH 1/1] StandaloneMmPkg: Fix the failure to= find uncompressed inner FV. >=20 > On 10/24/23 07:53, Xu, Wei6 wrote: >> The MmCoreFfsFindMmDriver only checks for encapsulated compressed FVs. >> When an inner FV is uncompressed, StandaloneMmCore will miss the FV=20 >> and all the MM drivers in the FV will not be dispatched. >> Add checks for uncompressed inner FV to fix this issue. >> >> Cc: Ard Biesheuvel >> Cc: Sami Mujawar >> Cc: Ray Ni >> Signed-off-by: Wei6 Xu >> --- >> StandaloneMmPkg/Core/FwVol.c | 11 +++++++++++ >> 1 file changed, 11 insertions(+) >> >> diff --git a/StandaloneMmPkg/Core/FwVol.c=20 >> b/StandaloneMmPkg/Core/FwVol.c index 1f6d7714ba97..1a85d80eb9f7 100644 >> --- a/StandaloneMmPkg/Core/FwVol.c >> +++ b/StandaloneMmPkg/Core/FwVol.c >> @@ -104,6 +104,17 @@ MmCoreFfsFindMmDriver ( >> break; >> } >> =20 >> + Status =3D FfsFindSectionData ( >> + EFI_SECTION_FIRMWARE_VOLUME_IMAGE, >> + FileHeader, >> + &SectionData, >> + &SectionDataSize >> + ); >> + if (!EFI_ERROR (Status)) { >> + InnerFvHeader =3D (EFI_FIRMWARE_VOLUME_HEADER *)SectionData; >> + MmCoreFfsFindMmDriver (InnerFvHeader); >> + } >> + >> Status =3D FfsFindSectionData ( >> EFI_SECTION_GUID_DEFINED, >> FileHeader, >=20 > I'd recommend fixing other, more foundational issues first, in this funct= ion, such as: >=20 > - memory leaks on error paths >=20 > - unbounded recursion >=20 > - missing object size checks before casting pointers to header types >=20 > At the same time I agree that this change doesn't seem to make things wor= se than they are. >=20 > Laszlo >=20 -=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#110240): https://edk2.groups.io/g/devel/message/110240 Mute This Topic: https://groups.io/mt/102152694/7686176 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/leave/12367111/7686176/19134562= 12/xyzzy [rebecca@openfw.io] -=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-