Also include NetworkPkg reviewer to collect the feedback for this change. Thanks Liming > -----Original Message----- > From: Laszlo Ersek <[email protected]> > Sent: Friday, February 28, 2020 1:40 AM > To: [email protected]; [email protected]; Gao, Liming > <[email protected]>; Kinney, Michael D > <[email protected]>; Andrew Fish <[email protected]>; Leif Lindholm > (Linaro address) <[email protected]> > Cc: Ni, Ray <[email protected]>; Gao, Zhichao <[email protected]>; Armour, > Nicholas <[email protected]> > Subject: Re: [edk2-devel] [PATCH v1] ShellPkg: Fix 'ping' command Ip4 receive > flow. > > On 02/27/20 14:14, Laszlo Ersek wrote: > > (+Liming and stewards; CC Nick) > > > > On 02/27/20 12:02, Maciej Rabeda wrote: > >> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2032 > >> > >> 'ping' command's receive flow utilizes a single Rx token which it > >> attempts to reuse before recycling the previously received packet. > >> This causes a situation where under ICMP traffic, > >> Ping6OnEchoReplyReceived() function will receive an already > >> recycled packet with EFI_SUCCESS token status and finally > >> dereference invalid pointers from RxData structure. > >> > >> Cc: Ray Ni <[email protected]> > >> Cc: Zhichao Gao <[email protected]> > >> Signed-off-by: Maciej Rabeda <[email protected]> > >> --- > >> ShellPkg/Library/UefiShellNetwork1CommandsLib/Ping.c | 9 +++++---- > >> 1 file changed, 5 insertions(+), 4 deletions(-) > >> > >> diff --git a/ShellPkg/Library/UefiShellNetwork1CommandsLib/Ping.c > >> b/ShellPkg/Library/UefiShellNetwork1CommandsLib/Ping.c > >> index 23567fa2c1bb..a3fa32515192 100644 > >> --- a/ShellPkg/Library/UefiShellNetwork1CommandsLib/Ping.c > >> +++ b/ShellPkg/Library/UefiShellNetwork1CommandsLib/Ping.c > >> @@ -614,6 +614,11 @@ Ping6OnEchoReplyReceived ( > >> > >> ON_EXIT: > >> > >> + // > >> + // Recycle the packet before reusing RxToken > >> + // > >> + gBS->SignalEvent (Private->IpChoice == > >> PING_IP_CHOICE_IP6?((EFI_IP6_RECEIVE_DATA*)Private->RxToken.Packet.RxData)- > >RecycleSignal:((EFI_IP4_RECEIVE_DATA*)Private->RxToken.Packet.RxData)->RecycleSignal); > >> + > >> if (Private->RxCount < Private->SendNum) { > >> // > >> // Continue to receive icmp echo reply packets. > >> @@ -632,10 +637,6 @@ ON_EXIT: > >> // > >> Private->Status = EFI_SUCCESS; > >> } > >> - // > >> - // Singal to recycle the each rxdata here, not at the end of process. > >> - // > >> - gBS->SignalEvent (Private->IpChoice == > >> PING_IP_CHOICE_IP6?((EFI_IP6_RECEIVE_DATA*)Private->RxToken.Packet.RxData)- > >RecycleSignal:((EFI_IP4_RECEIVE_DATA*)Private->RxToken.Packet.RxData)->RecycleSignal); > >> } > >> > >> /** > >> > > > > (1) This patch proposes to fix one of the BZs (2032) that fall under > > CVE-2019-14559 (joint tracker: 2550). > > > > Consequently: > > > > (1a) Do we want to include this in the upcoming stable tag? > > > > If so, we might want to extend the hard feature freeze by a few days. > > > > (1b) Please append the string " (CVE-2019-14559)" -- note the separating > > space! -- to the subject line. > > > > (2) However: I remember from an earlier Bugzilla entry (can't tell > > off-hand, which one, sorry) that ShellPkg issues are *never* considered > > CVE-worthy, because the shell is not considered a "production element" > > of the UEFI boot path. > > I misremembered -- there is indeed a comment like that, in the TianoCore > bugzilla, but it does not refer to ShellPkg. It refers to StdLib (which > has since been split off to the edk2-libc project): > > https://bugzilla.tianocore.org/show_bug.cgi?id=1510#c1 > > StdLib is supposed to be used only by applications in shell, all of > which are meant for debug, diagnosis and/or test purpose, not for > product UEFI BIOS. Any issue in it will not be taken as security > issue but just normal bug. > > Sorry about causing confusion. So, the ShellPkg maintainers should > decide what to do about this bug (keep it under the CVE scope vs. > exclude it from the CVE scope; and then, propose it for the stable tag > or merge it afterwards). > > One data point: the bug appears to go back to the inception of the Ping > command, in historical commit 68fb05272b45 ("Add Network1 profile.", > 2011-03-25). It's not a new bug, it seems. > > Thanks > Laszlo > > > > > TianoCore#2032 was originally filed for NetworkPkg, and indeed that > > seemed to justify the CVE assignment. However, now that Nick's and > > Maciej's analysis shows that NetworkPkg is unaffected (and we know, per > > above, that ShellPkg is not CVE-worthy), should we rather *remove* this > > BZ from the CVE-2019-14559 umbrella? > > > > Because, in that case, modifying the subject line on the patch is not > > necessary; and more importantly, we might not even want to put this into > > edk2-stable202002. (It's still a bugfix, but may not be important enough.) > > > > Thanks! > > Laszlo > >
-=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#55044): https://edk2.groups.io/g/devel/message/55044 Mute This Topic: https://groups.io/mt/71584586/21656 Group Owner: [email protected] Unsubscribe: https://edk2.groups.io/g/devel/unsub [[email protected]] -=-=-=-=-=-=-=-=-=-=-=-
