From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-pg1-f176.google.com (mail-pg1-f176.google.com [209.85.215.176]) by mx.groups.io with SMTP id smtpd.web09.13634.1667432544108338176 for ; Wed, 02 Nov 2022 16:42:24 -0700 Authentication-Results: mx.groups.io; dkim=pass header.i=@gmail.com header.s=20210112 header.b=LqRbxRHV; spf=pass (domain: gmail.com, ip: 209.85.215.176, mailfrom: pedro.falcato@gmail.com) Received: by mail-pg1-f176.google.com with SMTP id 64so206290pgc.5 for ; Wed, 02 Nov 2022 16:42:24 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:from:to:cc:subject:date:message-id:reply-to; bh=aq2ayAy6ufQ2yEkz5/26laBuPwIvu83GWIhzXe/B+X0=; b=LqRbxRHVx7AcpwRoEhjiAY5orvtmzPypYF8oYWg6CroWJEXU1VIhTeDgDyw5jW3p4d QTVxSovsC28A3yFy3IwGf5T0vDZfgAB06+ZnefTOiDrwepaTt1LxZFWm5I03b/rokqOJ ZG/XIECfmIipCE7phU88UKAIzU18aLWbQrl7E5LWAkp3gzla/wy9SIwHpnTn+Z3mxIKX /VSuVsKE4SlZFBMyg6r4dtUZmAVyoH6PCl9J+23qtYORPXWSQXouqx6MPRMTHsmHMxlI K+/qJkDNCYVKIaM0TCo8fulP2jKIF2/DxnmnNy2bSZWZMetLK4ReOVSqsh/cQtBjGjvA tMEw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=aq2ayAy6ufQ2yEkz5/26laBuPwIvu83GWIhzXe/B+X0=; b=YqA4jhvJ0nboufZkeZGNB8mwy474VHnuHkW4Qm4kShhgNo2vVUzMlDlkgkEmfrs17I i/oIvMVCoaRhrlDsCAe+Ck0RMduSDSceYP5usgobt6ePRLjcuP27f7pyIKC+GWFWpDUK bjl73zxK5F5Rf9ZpS6EKwp+F2tOVa0GVePPIzvBvZKgB7qyNnNOMg0R/3waYNYviYbmv JsVWh8wNC8tsU/K9Kd6hOJ6wQW99nZTBZ5cI2zHoy/4ugZjmx5IsJvKR6hLQgHLxGucO 0ZkM0Co04zJ0Jtq4lJ3VZnDuGPLaZoaz2/+xb+D5/eTzKLDisbGtTlNffO8IVh5ZHc6Z q5Aw== X-Gm-Message-State: ACrzQf1qwr+wwWpnycl4a+43Grk+ZoQfQHvesBETgvSVTo1d0l37CLo2 WP/Z4gckkYrPabGeusUke+x0u6nRKzvNvNBUrSE= X-Google-Smtp-Source: AMsMyM7nn8AArOeo35MfSiHqRDVBylY+MrCNDbg1ZvVMEbiQQ88Mp1KUHuxxO7VZY+8zNeGoJp8ZXotgSj+yEmC2SWI= X-Received: by 2002:a63:4955:0:b0:44e:d36e:4c2e with SMTP id y21-20020a634955000000b0044ed36e4c2emr23720805pgk.326.1667432543432; Wed, 02 Nov 2022 16:42:23 -0700 (PDT) MIME-Version: 1.0 References: <20221024224324.26540-1-pedro.falcato@gmail.com> In-Reply-To: From: "Pedro Falcato" Date: Wed, 2 Nov 2022 23:42:12 +0000 Message-ID: Subject: Re: [PATCH v2 1/1] MdePkg/BaseLib: Fix out-of-bounds reads in SafeString To: "Kinney, Michael D" Cc: "Yao, Jiewen" , "devel@edk2.groups.io" , Vitaly Cheptsov , =?UTF-8?Q?Marvin_H=C3=A4user?= , "Gao, Liming" , "Liu, Zhiguang" Content-Type: multipart/alternative; boundary="0000000000007034c405ec8564f8" --0000000000007034c405ec8564f8 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable Can someone push this? Is there a blocker here? On Wed, Oct 26, 2022 at 4:54 PM Kinney, Michael D < michael.d.kinney@intel.com> wrote: > Acked-by: Michael D Kinney > > > > -----Original Message----- > > From: Yao, Jiewen > > Sent: Wednesday, October 26, 2022 6:35 AM > > To: Kinney, Michael D ; Pedro Falcato < > pedro.falcato@gmail.com>; devel@edk2.groups.io > > Cc: Vitaly Cheptsov ; Marvin H=C3=A4user < > mhaeuser@posteo.de>; Gao, Liming ; Liu, > > Zhiguang > > Subject: RE: [PATCH v2 1/1] MdePkg/BaseLib: Fix out-of-bounds reads in > SafeString > > > > That is good catch. > > > > Reviewed-by: Jiewen Yao > > > > > > > -----Original Message----- > > > From: Kinney, Michael D > > > Sent: Wednesday, October 26, 2022 12:23 AM > > > To: Pedro Falcato ; devel@edk2.groups.io > > > Cc: Vitaly Cheptsov ; Marvin H=C3=A4user > > > ; Gao, Liming ; Liu, > > > Zhiguang ; Yao, Jiewen > > > Subject: RE: [PATCH v2 1/1] MdePkg/BaseLib: Fix out-of-bounds reads i= n > > > SafeString > > > > > > Adding Jiewen Yao. > > > > > > Mike > > > > > > > -----Original Message----- > > > > From: Pedro Falcato > > > > Sent: Monday, October 24, 2022 3:43 PM > > > > To: devel@edk2.groups.io > > > > Cc: Pedro Falcato ; Vitaly Cheptsov > > > ; Marvin H=C3=A4user ; > > > > Kinney, Michael D ; Gao, Liming > > > ; Liu, Zhiguang > > > > Subject: [PATCH v2 1/1] MdePkg/BaseLib: Fix out-of-bounds reads in > > > SafeString > > > > > > > > OpenCore folks established an ASAN-equipped project to fuzz Ext4Dxe= , > > > > which was able to catch these (mostly harmless) issues. > > > > > > > > Signed-off-by: Pedro Falcato > > > > Cc: Vitaly Cheptsov > > > > Cc: Marvin H=C3=A4user > > > > Cc: Michael D Kinney > > > > Cc: Liming Gao > > > > Cc: Zhiguang Liu > > > > --- > > > > MdePkg/Library/BaseLib/SafeString.c | 24 ++++++++++++++++++++---- > > > > 1 file changed, 20 insertions(+), 4 deletions(-) > > > > > > > > diff --git a/MdePkg/Library/BaseLib/SafeString.c > > > b/MdePkg/Library/BaseLib/SafeString.c > > > > index f338a32a3a41..77a2585ad56d 100644 > > > > --- a/MdePkg/Library/BaseLib/SafeString.c > > > > +++ b/MdePkg/Library/BaseLib/SafeString.c > > > > @@ -863,6 +863,9 @@ StrHexToUintnS ( > > > > OUT UINTN *Data > > > > ) > > > > { > > > > + BOOLEAN FoundLeadingZero; > > > > + > > > > + FoundLeadingZero =3D FALSE; > > > > ASSERT (((UINTN)String & BIT0) =3D=3D 0); > > > > > > > > // > > > > @@ -893,11 +896,12 @@ StrHexToUintnS ( > > > > // Ignore leading Zeros after the spaces > > > > // > > > > while (*String =3D=3D L'0') { > > > > + FoundLeadingZero =3D TRUE; > > > > String++; > > > > } > > > > > > > > if (CharToUpper (*String) =3D=3D L'X') { > > > > - if (*(String - 1) !=3D L'0') { > > > > + if (!FoundLeadingZero) { > > > > *Data =3D 0; > > > > return RETURN_SUCCESS; > > > > } > > > > @@ -992,6 +996,9 @@ StrHexToUint64S ( > > > > OUT UINT64 *Data > > > > ) > > > > { > > > > + BOOLEAN FoundLeadingZero; > > > > + > > > > + FoundLeadingZero =3D FALSE; > > > > ASSERT (((UINTN)String & BIT0) =3D=3D 0); > > > > > > > > // > > > > @@ -1022,11 +1029,12 @@ StrHexToUint64S ( > > > > // Ignore leading Zeros after the spaces > > > > // > > > > while (*String =3D=3D L'0') { > > > > + FoundLeadingZero =3D TRUE; > > > > String++; > > > > } > > > > > > > > if (CharToUpper (*String) =3D=3D L'X') { > > > > - if (*(String - 1) !=3D L'0') { > > > > + if (!FoundLeadingZero) { > > > > *Data =3D 0; > > > > return RETURN_SUCCESS; > > > > } > > > > @@ -2393,6 +2401,9 @@ AsciiStrHexToUintnS ( > > > > OUT UINTN *Data > > > > ) > > > > { > > > > + BOOLEAN FoundLeadingZero; > > > > + > > > > + FoundLeadingZero =3D FALSE; > > > > // > > > > // 1. Neither String nor Data shall be a null pointer. > > > > // > > > > @@ -2421,11 +2432,12 @@ AsciiStrHexToUintnS ( > > > > // Ignore leading Zeros after the spaces > > > > // > > > > while (*String =3D=3D '0') { > > > > + FoundLeadingZero =3D TRUE; > > > > String++; > > > > } > > > > > > > > if (AsciiCharToUpper (*String) =3D=3D 'X') { > > > > - if (*(String - 1) !=3D '0') { > > > > + if (!FoundLeadingZero) { > > > > *Data =3D 0; > > > > return RETURN_SUCCESS; > > > > } > > > > @@ -2517,6 +2529,9 @@ AsciiStrHexToUint64S ( > > > > OUT UINT64 *Data > > > > ) > > > > { > > > > + BOOLEAN FoundLeadingZero; > > > > + > > > > + FoundLeadingZero =3D FALSE; > > > > // > > > > // 1. Neither String nor Data shall be a null pointer. > > > > // > > > > @@ -2545,11 +2560,12 @@ AsciiStrHexToUint64S ( > > > > // Ignore leading Zeros after the spaces > > > > // > > > > while (*String =3D=3D '0') { > > > > + FoundLeadingZero =3D TRUE; > > > > String++; > > > > } > > > > > > > > if (AsciiCharToUpper (*String) =3D=3D 'X') { > > > > - if (*(String - 1) !=3D '0') { > > > > + if (!FoundLeadingZero) { > > > > *Data =3D 0; > > > > return RETURN_SUCCESS; > > > > } > > > > -- > > > > 2.38.1 > > --=20 Pedro Falcato --0000000000007034c405ec8564f8 Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable
Can someone push this? Is there a blocker here?
<= br>
On Wed,= Oct 26, 2022 at 4:54 PM Kinney, Michael D <michael.d.kinney@intel.com> wrote:
Acked-by: Michael D Kinney <= michael.d.k= inney@intel.com>


> -----Original Message-----
> From: Yao, Jiewen <jiewen.yao@intel.com>
> Sent: Wednesday, October 26, 2022 6:35 AM
> To: Kinney, Michael D <michael.d.kinney@intel.com>; Pedro Falcato <<= a href=3D"mailto:pedro.falcato@gmail.com" target=3D"_blank">pedro.falcato@g= mail.com>; devel@edk2.groups.io
> Cc: Vitaly Cheptsov <vit9696@protonmail.com>; Marvin H=C3=A4user <mhaeuser@posteo.de>= ;; Gao, Liming <gaoliming@byosoft.com.cn>; Liu,
> Zhiguang <zhiguang.liu@intel.com>
> Subject: RE: [PATCH v2 1/1] MdePkg/BaseLib: Fix out-of-bounds reads in= SafeString
>
> That is good catch.
>
> Reviewed-by: Jiewen Yao <Jiewen.yao@Intel.com>
>
>
> > -----Original Message-----
> > From: Kinney, Michael D <michael.d.kinney@intel.com>
> > Sent: Wednesday, October 26, 2022 12:23 AM
> > To: Pedro Falcato <pedro.falcato@gmail.com>; devel@edk2.groups.io
> > Cc: Vitaly Cheptsov <vit9696@protonmail.com>; Marvin H=C3=A4user
> > <mhaeu= ser@posteo.de>; Gao, Liming <gaoliming@byosoft.com.cn>; Liu,
> > Zhiguang <zhiguang.liu@intel.com>; Yao, Jiewen <jiewen.yao@intel.com>
> > Subject: RE: [PATCH v2 1/1] MdePkg/BaseLib: Fix out-of-bounds rea= ds in
> > SafeString
> >
> > Adding Jiewen Yao.
> >
> > Mike
> >
> > > -----Original Message-----
> > > From: Pedro Falcato <pedro.falcato@gmail.com>
> > > Sent: Monday, October 24, 2022 3:43 PM
> > > To: devel@edk2.groups.io
> > > Cc: Pedro Falcato <pedro.falcato@gmail.com>; Vitaly Cheptsov
> > <v= it9696@protonmail.com>; Marvin H=C3=A4user <mhaeuser@posteo.de>;
> > > Kinney, Michael D <michael.d.kinney@intel.com>; Gao, Liming > > <gaoliming@byosoft.com.cn>; Liu, Zhiguang <zhiguang.liu@intel.com>
> > > Subject: [PATCH v2 1/1] MdePkg/BaseLib: Fix out-of-bounds re= ads in
> > SafeString
> > >
> > > OpenCore folks established an ASAN-equipped project to fuzz = Ext4Dxe,
> > > which was able to catch these (mostly harmless) issues.
> > >
> > > Signed-off-by: Pedro Falcato <pedro.falcato@gmail.com>
> > > Cc: Vitaly Cheptsov <vit9696@protonmail.com>
> > > Cc: Marvin H=C3=A4user <mhaeuser@posteo.de>
> > > Cc: Michael D Kinney <michael.d.kinney@intel.com>
> > > Cc: Liming Gao <gaoliming@byosoft.com.cn>
> > > Cc: Zhiguang Liu <zhiguang.liu@intel.com>
> > > ---
> > >=C2=A0 MdePkg/Library/BaseLib/SafeString.c | 24 +++++++++++++= +++++++----
> > >=C2=A0 1 file changed, 20 insertions(+), 4 deletions(-)
> > >
> > > diff --git a/MdePkg/Library/BaseLib/SafeString.c
> > b/MdePkg/Library/BaseLib/SafeString.c
> > > index f338a32a3a41..77a2585ad56d 100644
> > > --- a/MdePkg/Library/BaseLib/SafeString.c
> > > +++ b/MdePkg/Library/BaseLib/SafeString.c
> > > @@ -863,6 +863,9 @@ StrHexToUintnS (
> > >=C2=A0 =C2=A0 OUT=C2=A0 =C2=A0 =C2=A0 =C2=A0UINTN=C2=A0 =C2= =A0*Data
> > >=C2=A0 =C2=A0 )
> > >=C2=A0 {
> > > +=C2=A0 BOOLEAN=C2=A0 FoundLeadingZero;
> > > +
> > > +=C2=A0 FoundLeadingZero =3D FALSE;
> > >=C2=A0 =C2=A0 ASSERT (((UINTN)String & BIT0) =3D=3D 0); > > >
> > >=C2=A0 =C2=A0 //
> > > @@ -893,11 +896,12 @@ StrHexToUintnS (
> > >=C2=A0 =C2=A0 // Ignore leading Zeros after the spaces
> > >=C2=A0 =C2=A0 //
> > >=C2=A0 =C2=A0 while (*String =3D=3D L'0') {
> > > +=C2=A0 =C2=A0 FoundLeadingZero =3D TRUE;
> > >=C2=A0 =C2=A0 =C2=A0 String++;
> > >=C2=A0 =C2=A0 }
> > >
> > >=C2=A0 =C2=A0 if (CharToUpper (*String) =3D=3D L'X') = {
> > > -=C2=A0 =C2=A0 if (*(String - 1) !=3D L'0') {
> > > +=C2=A0 =C2=A0 if (!FoundLeadingZero) {
> > >=C2=A0 =C2=A0 =C2=A0 =C2=A0 *Data =3D 0;
> > >=C2=A0 =C2=A0 =C2=A0 =C2=A0 return RETURN_SUCCESS;
> > >=C2=A0 =C2=A0 =C2=A0 }
> > > @@ -992,6 +996,9 @@ StrHexToUint64S (
> > >=C2=A0 =C2=A0 OUT=C2=A0 =C2=A0 =C2=A0 =C2=A0UINT64=C2=A0 *Dat= a
> > >=C2=A0 =C2=A0 )
> > >=C2=A0 {
> > > +=C2=A0 BOOLEAN=C2=A0 FoundLeadingZero;
> > > +
> > > +=C2=A0 FoundLeadingZero =3D FALSE;
> > >=C2=A0 =C2=A0 ASSERT (((UINTN)String & BIT0) =3D=3D 0); > > >
> > >=C2=A0 =C2=A0 //
> > > @@ -1022,11 +1029,12 @@ StrHexToUint64S (
> > >=C2=A0 =C2=A0 // Ignore leading Zeros after the spaces
> > >=C2=A0 =C2=A0 //
> > >=C2=A0 =C2=A0 while (*String =3D=3D L'0') {
> > > +=C2=A0 =C2=A0 FoundLeadingZero =3D TRUE;
> > >=C2=A0 =C2=A0 =C2=A0 String++;
> > >=C2=A0 =C2=A0 }
> > >
> > >=C2=A0 =C2=A0 if (CharToUpper (*String) =3D=3D L'X') = {
> > > -=C2=A0 =C2=A0 if (*(String - 1) !=3D L'0') {
> > > +=C2=A0 =C2=A0 if (!FoundLeadingZero) {
> > >=C2=A0 =C2=A0 =C2=A0 =C2=A0 *Data =3D 0;
> > >=C2=A0 =C2=A0 =C2=A0 =C2=A0 return RETURN_SUCCESS;
> > >=C2=A0 =C2=A0 =C2=A0 }
> > > @@ -2393,6 +2401,9 @@ AsciiStrHexToUintnS (
> > >=C2=A0 =C2=A0 OUT=C2=A0 =C2=A0 =C2=A0 =C2=A0UINTN=C2=A0 *Data=
> > >=C2=A0 =C2=A0 )
> > >=C2=A0 {
> > > +=C2=A0 BOOLEAN=C2=A0 FoundLeadingZero;
> > > +
> > > +=C2=A0 FoundLeadingZero =3D FALSE;
> > >=C2=A0 =C2=A0 //
> > >=C2=A0 =C2=A0 // 1. Neither String nor Data shall be a null p= ointer.
> > >=C2=A0 =C2=A0 //
> > > @@ -2421,11 +2432,12 @@ AsciiStrHexToUintnS (
> > >=C2=A0 =C2=A0 // Ignore leading Zeros after the spaces
> > >=C2=A0 =C2=A0 //
> > >=C2=A0 =C2=A0 while (*String =3D=3D '0') {
> > > +=C2=A0 =C2=A0 FoundLeadingZero =3D TRUE;
> > >=C2=A0 =C2=A0 =C2=A0 String++;
> > >=C2=A0 =C2=A0 }
> > >
> > >=C2=A0 =C2=A0 if (AsciiCharToUpper (*String) =3D=3D 'X= 9;) {
> > > -=C2=A0 =C2=A0 if (*(String - 1) !=3D '0') {
> > > +=C2=A0 =C2=A0 if (!FoundLeadingZero) {
> > >=C2=A0 =C2=A0 =C2=A0 =C2=A0 *Data =3D 0;
> > >=C2=A0 =C2=A0 =C2=A0 =C2=A0 return RETURN_SUCCESS;
> > >=C2=A0 =C2=A0 =C2=A0 }
> > > @@ -2517,6 +2529,9 @@ AsciiStrHexToUint64S (
> > >=C2=A0 =C2=A0 OUT=C2=A0 =C2=A0 =C2=A0 =C2=A0UINT64=C2=A0 *Dat= a
> > >=C2=A0 =C2=A0 )
> > >=C2=A0 {
> > > +=C2=A0 BOOLEAN=C2=A0 FoundLeadingZero;
> > > +
> > > +=C2=A0 FoundLeadingZero =3D FALSE;
> > >=C2=A0 =C2=A0 //
> > >=C2=A0 =C2=A0 // 1. Neither String nor Data shall be a null p= ointer.
> > >=C2=A0 =C2=A0 //
> > > @@ -2545,11 +2560,12 @@ AsciiStrHexToUint64S (
> > >=C2=A0 =C2=A0 // Ignore leading Zeros after the spaces
> > >=C2=A0 =C2=A0 //
> > >=C2=A0 =C2=A0 while (*String =3D=3D '0') {
> > > +=C2=A0 =C2=A0 FoundLeadingZero =3D TRUE;
> > >=C2=A0 =C2=A0 =C2=A0 String++;
> > >=C2=A0 =C2=A0 }
> > >
> > >=C2=A0 =C2=A0 if (AsciiCharToUpper (*String) =3D=3D 'X= 9;) {
> > > -=C2=A0 =C2=A0 if (*(String - 1) !=3D '0') {
> > > +=C2=A0 =C2=A0 if (!FoundLeadingZero) {
> > >=C2=A0 =C2=A0 =C2=A0 =C2=A0 *Data =3D 0;
> > >=C2=A0 =C2=A0 =C2=A0 =C2=A0 return RETURN_SUCCESS;
> > >=C2=A0 =C2=A0 =C2=A0 }
> > > --
> > > 2.38.1



--
Pedro Falcato
--0000000000007034c405ec8564f8--