Maciej: Thanks for your analysis. I agree with you. This change is not necessary to catch edk2 stable tag 202002.
Thanks Liming > -----Original Message----- > From: Rabeda, Maciej <maciej.rab...@linux.intel.com> > Sent: Friday, February 28, 2020 8:35 PM > To: Gao, Liming <liming....@intel.com>; devel@edk2.groups.io; Laszlo Ersek > <ler...@redhat.com>; Kinney, Michael D > <michael.d.kin...@intel.com>; Andrew Fish <af...@apple.com>; Leif Lindholm > (Linaro address) <leif.lindh...@linaro.org> > Cc: Ni, Ray <ray...@intel.com>; Gao, Zhichao <zhichao....@intel.com>; Armour, > Nicholas <nicholas.arm...@intel.com>; Fu, Siyuan > <siyuan...@intel.com>; Wu, Jiaxin <jiaxin...@intel.com> > Subject: Re: [edk2-devel] [PATCH v1] ShellPkg: Fix 'ping' command Ip4 receive > flow. > > Liming, > > I assume that CVE-2019-14559 relates to problems with network drivers > within NetworkPkg. > BZ 2032 and this patch fix address incorrect usage of IP4 protocol by > ShellPkg 'ping' command (signalling packet recycling after triggering > Ip4->Receive() on the same Rx token). > > This issue was reported in July 2019 and occurs by executing a specific > fuzzing scenario (as described in BZ). > As Laszlo has mentioned, it is not a new bug (introduced in 2011). > > Based on the above, I can advise moving this issue out of CVE scope (and > from stable-202002). > However, if the CVE should be treated as "overall problems with UEFI FW > networking", then 'ping' command seems to be in CVE scope, despite the > code residing in ShellPkg. > > Thanks, > Maciej > > On 28-Feb-20 12:50, Gao, Liming wrote: > > Maciej: > > I see you submit the patch. So, you are in the loop. I don't invite you > > again. 😊 > > > > Yes. I also want to get your opinion for this change. Do you think > > whether this fix is in CVE scope? If no, this change will be merged > after this stable tag 202002. Is it OK to you? > > > > Thanks > > Liming > >> -----Original Message----- > >> From: Rabeda, Maciej <maciej.rab...@linux.intel.com> > >> Sent: Friday, February 28, 2020 7:42 PM > >> To: devel@edk2.groups.io; Gao, Liming <liming....@intel.com>; Laszlo Ersek > >> <ler...@redhat.com>; Kinney, Michael D > >> <michael.d.kin...@intel.com>; Andrew Fish <af...@apple.com>; Leif Lindholm > >> (Linaro address) <leif.lindh...@linaro.org> > >> Cc: Ni, Ray <ray...@intel.com>; Gao, Zhichao <zhichao....@intel.com>; > >> Armour, Nicholas <nicholas.arm...@intel.com>; Fu, Siyuan > >> <siyuan...@intel.com>; Wu, Jiaxin <jiaxin...@intel.com> > >> Subject: Re: [edk2-devel] [PATCH v1] ShellPkg: Fix 'ping' command Ip4 > >> receive flow. > >> > >> Laszlo, > >> > >> Thanks for the detailed response on the patch. Always happy to learn > >> about stuff from the past. > >> > >> Liming, > >> > >> I am currently the maintainer of NetworkPkg :) If you require additional > >> feedback from Siyuan or/and Jiaxin, that's ok. > >> Please let me know if any corrections to the patch (like CVE note) are > >> required from your point of view. > >> > >> Thanks, > >> Maciej > >> > >> On 28-Feb-20 03:59, Liming Gao wrote: > >>> Also include NetworkPkg reviewer to collect the feedback for this change. > >>> > >>> Thanks > >>> Liming > >>>> -----Original Message----- > >>>> From: Laszlo Ersek <ler...@redhat.com> > >>>> Sent: Friday, February 28, 2020 1:40 AM > >>>> To: devel@edk2.groups.io; maciej.rab...@linux.intel.com; Gao, Liming > >>>> <liming....@intel.com>; Kinney, Michael D > >>>> <michael.d.kin...@intel.com>; Andrew Fish <af...@apple.com>; Leif > >>>> Lindholm (Linaro address) <leif.lindh...@linaro.org> > >>>> Cc: Ni, Ray <ray...@intel.com>; Gao, Zhichao <zhichao....@intel.com>; > >>>> Armour, Nicholas <nicholas.arm...@intel.com> > >>>> 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 <ray...@intel.com> > >>>>>> Cc: Zhichao Gao <zhichao....@intel.com> > >>>>>> Signed-off-by: Maciej Rabeda <maciej.rab...@linux.intel.com> > >>>>>> --- > >>>>>> 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 (#55194): https://edk2.groups.io/g/devel/message/55194 Mute This Topic: https://groups.io/mt/71584586/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
