From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from walk.intel-email.com (walk.intel-email.com [101.227.64.242]) by mx.groups.io with SMTP id smtpd.web12.5455.1667784765187925166 for ; Sun, 06 Nov 2022 17:32:46 -0800 Authentication-Results: mx.groups.io; dkim=pass header.i=@byosoft.com.cn header.s=cloud-union header.b=evVWgwRS; spf=pass (domain: byosoft.com.cn, ip: 101.227.64.242, mailfrom: gaoliming@byosoft.com.cn) Received: from walk.intel-email.com (localhost [127.0.0.1]) by walk.intel-email.com (Postfix) with ESMTP id 02895CD1F80C for ; Mon, 7 Nov 2022 09:32:39 +0800 (CST) DKIM-Signature: v=1; a=rsa-sha256; c=simple/simple; d=byosoft.com.cn; s=cloud-union; t=1667784759; bh=TF8p0+IH19V4M3kmf9A7iPkLjlRICxHqAtB6WMokqg4=; h=From:To:Cc:References:In-Reply-To:Subject:Date; b=evVWgwRSrtFjp/KzaGcaVe8gcfsrCjA9JlMpbOnd4126o1z95d1BfD1M+S12E5okc lv3F3F04LVfA1EqmfZd0OWz+SIcBqfWheR7PTKiI/Pk2MtVW1l6bkI0apm/5xH0wuw /PWhFwgsL2sn6PHjvowtEHeJdIPG9/FdunIqB1ko= Received: from localhost (localhost [127.0.0.1]) by walk.intel-email.com (Postfix) with ESMTP id F24EFCD1F80B for ; Mon, 7 Nov 2022 09:32:38 +0800 (CST) Received: from walk.intel-email.com (localhost [127.0.0.1]) by walk.intel-email.com (Postfix) with ESMTP id B8DC2CD1F7F2 for ; Mon, 7 Nov 2022 09:32:38 +0800 (CST) Authentication-Results: walk.intel-email.com; none Received: from mail.byosoft.com.cn (mail.byosoft.com.cn [58.240.74.242]) by walk.intel-email.com (Postfix) with SMTP id 4236DCD1F800 for ; Mon, 7 Nov 2022 09:32:33 +0800 (CST) Received: from DESKTOPS6D0PVI ([58.246.60.130]) (envelope-sender ) by 192.168.6.13 with ESMTP for ; Mon, 07 Nov 2022 09:32:32 +0800 X-WM-Sender: gaoliming@byosoft.com.cn X-Originating-IP: 58.246.60.130 X-WM-AuthFlag: YES X-WM-AuthUser: gaoliming@byosoft.com.cn From: "gaoliming" To: , Cc: "'Vitaly Cheptsov'" , =?UTF-8?Q?'Marvin_H=C3=A4user'?= , "'Michael D Kinney'" , "'Zhiguang Liu'" , "'Jiewen Yao'" References: <20221103011149.659815-1-pedro.falcato@gmail.com> <000201d8efeb$f43533b0$dc9f9b10$@byosoft.com.cn> In-Reply-To: Subject: =?UTF-8?B?5Zue5aSNOiBbZWRrMi1kZXZlbF0g5Zue5aSNOiBbUEFUQ0ggdjMgMS8xXSBNZGVQa2cvQmFzZUxpYjogRml4IG91dC1vZi1ib3VuZHMgcmVhZHMgaW4gU2FmZVN0cmluZw==?= Date: Mon, 7 Nov 2022 09:32:33 +0800 Message-ID: <016b01d8f248$cf2d28c0$6d877a40$@byosoft.com.cn> MIME-Version: 1.0 X-Mailer: Microsoft Outlook 16.0 Thread-Index: AQGxUQmPs9Y0Oj1QXXhCb6uJwyhXFgFM4/pIAnxOcZyuY6Xo4A== Sender: "gaoliming" Content-Type: multipart/alternative; boundary="----=_NextPart_000_016C_01D8F28B.DD51EF60" Content-Language: zh-cn ------=_NextPart_000_016C_01D8F28B.DD51EF60 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable Create https://github.com/tianocore/edk2/pull/3604 to merge this patch. =20 Thanks Liming =E5=8F=91=E4=BB=B6=E4=BA=BA: devel@edk2.groups.io = =E4=BB=A3=E8=A1=A8 Pedro Falcato =E5=8F=91=E9=80=81=E6=97=B6=E9=97=B4: 2022=E5=B9=B411=E6=9C=885=E6=97=A5 8:= 25 =E6=94=B6=E4=BB=B6=E4=BA=BA: devel@edk2.groups.io; gaoliming@byosoft.com.cn =E6=8A=84=E9=80=81: Vitaly Cheptsov ; Marvin H=C3= =A4user ; Michael D Kinney = ; Zhiguang Liu ; Jiewen Yao =E4=B8=BB=E9=A2=98: Re: [edk2-devel] =E5=9B=9E=E5=A4=8D: [PATCH v3 1/1] Mde= Pkg/BaseLib: Fix out-of-bounds reads in SafeString =20 Hi Liming, =20 Thank you for the review. Can we please push this in time for the stable ta= g? =20 Thanks, Pedro =20 On Fri, Nov 4, 2022 at 1:22 AM gaoliming via groups.io = > w= rote: Reviewed-by: Liming Gao > > -----=E9=82=AE=E4=BB=B6=E5=8E=9F=E4=BB=B6----- > =E5=8F=91=E4=BB=B6=E4=BA=BA: Pedro Falcato > > =E5=8F=91=E9=80=81=E6=97=B6=E9=97=B4: 2022=E5=B9=B411=E6=9C=883=E6=97=A5 = 9:12 > =E6=94=B6=E4=BB=B6=E4=BA=BA: devel@edk2.groups.io =20 > =E6=8A=84=E9=80=81: Pedro Falcato >; Vitaly Cheptsov > >; Marvin H=C3=A4= user >; > Michael D Kinney >; Liming Gao > >; Zhiguang L= iu >; Jiewen > Yao > > =E4=B8=BB=E9=A2=98: [PATCH v3 1/1] MdePkg/BaseLib: Fix out-of-bounds read= s in SafeString >=20 > There was a OOB access in *StrHexTo* functions, when passed strings like > "XDEADBEEF". >=20 > OpenCore folks established an ASAN-equipped project to fuzz Ext4Dxe, > which was able to catch these (mostly harmless) issues. >=20 > Cc: Vitaly Cheptsov > > Cc: Marvin H=C3=A4user > > Cc: Michael D Kinney > > Cc: Liming Gao > > Cc: Zhiguang Liu = > > Signed-off-by: Pedro Falcato > > Acked-by: Michael D Kinney > > Reviewed-by: Jiewen Yao > > --- > MdePkg/Library/BaseLib/SafeString.c | 25 +++++++++++++++++++++---- > 1 file changed, 21 insertions(+), 4 deletions(-) >=20 > diff --git a/MdePkg/Library/BaseLib/SafeString.c > b/MdePkg/Library/BaseLib/SafeString.c > index f338a32a3a41..b75b33381732 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); >=20 > // > @@ -892,12 +895,14 @@ StrHexToUintnS ( > // > // Ignore leading Zeros after the spaces > // > + > + FoundLeadingZero =3D *String =3D=3D L'0'; > while (*String =3D=3D L'0') { > String++; > } >=20 > if (CharToUpper (*String) =3D=3D L'X') { > - if (*(String - 1) !=3D L'0') { > + if (!FoundLeadingZero) { > *Data =3D 0; > return RETURN_SUCCESS; > } > @@ -992,6 +997,9 @@ StrHexToUint64S ( > OUT UINT64 *Data > ) > { > + BOOLEAN FoundLeadingZero; > + > + FoundLeadingZero =3D FALSE; > ASSERT (((UINTN)String & BIT0) =3D=3D 0); >=20 > // > @@ -1021,12 +1029,13 @@ StrHexToUint64S ( > // > // Ignore leading Zeros after the spaces > // > + FoundLeadingZero =3D *String =3D=3D L'0'; > while (*String =3D=3D L'0') { > String++; > } >=20 > if (CharToUpper (*String) =3D=3D L'X') { > - if (*(String - 1) !=3D L'0') { > + if (!FoundLeadingZero) { > *Data =3D 0; > return RETURN_SUCCESS; > } > @@ -2393,6 +2402,9 @@ AsciiStrHexToUintnS ( > OUT UINTN *Data > ) > { > + BOOLEAN FoundLeadingZero; > + > + FoundLeadingZero =3D FALSE; > // > // 1. Neither String nor Data shall be a null pointer. > // > @@ -2420,12 +2432,13 @@ AsciiStrHexToUintnS ( > // > // Ignore leading Zeros after the spaces > // > + FoundLeadingZero =3D *String =3D=3D '0'; > while (*String =3D=3D '0') { > String++; > } >=20 > if (AsciiCharToUpper (*String) =3D=3D 'X') { > - if (*(String - 1) !=3D '0') { > + if (!FoundLeadingZero) { > *Data =3D 0; > return RETURN_SUCCESS; > } > @@ -2517,6 +2530,9 @@ AsciiStrHexToUint64S ( > OUT UINT64 *Data > ) > { > + BOOLEAN FoundLeadingZero; > + > + FoundLeadingZero =3D FALSE; > // > // 1. Neither String nor Data shall be a null pointer. > // > @@ -2544,12 +2560,13 @@ AsciiStrHexToUint64S ( > // > // Ignore leading Zeros after the spaces > // > + FoundLeadingZero =3D *String =3D=3D '0'; > while (*String =3D=3D '0') { > String++; > } >=20 > 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 ------=_NextPart_000_016C_01D8F28B.DD51EF60 Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable

Create https://github.com/tianocore/edk2/pull/3604 to merge this patch.

 

Thanks

Limi= ng

=E5=8F=91=E4=BB= =B6=E4=BA=BA: devel@edk2.groups.io = <devel@edk2.groups.io> =E4=BB=A3=E8=A1=A8 Pedro Falcato=E5=8F=91=E9=80=81=E6=97=B6=E9=97=B4:= 2022=E5=B9=B411=E6=9C=885=E6=97=A5 8:25
=E6=94=B6=E4=BB=B6=E4=BA= =BA: devel@edk2.groups.io;= gaoliming@byosoft.com.cn
=E6=8A=84=E9=80=81: Vitaly Cheptsov <vit9696@protonmail.com= >; Marvin H=C3=A4user <mhaeuser@posteo.de>; Michael D Kinney <m= ichael.d.kinney@intel.com>; Zhiguang Liu <zhiguang.liu@intel.com>;= Jiewen Yao <Jiewen.yao@intel.com>
=E4=B8=BB=E9=A2=98: Re: [edk2-devel] =E5= =9B=9E=E5=A4=8D: [PATCH v3 1/1] MdePkg/BaseLib: Fix out-= of-bounds reads in SafeString

 

Hi Liming,

 

<= /div>

Thank you for the review.= Can we please push this in time for the stable tag?

<= /div>

 <= /p>

Thanks,

Pedro=

 <= /o:p>

On Fri, N= ov 4, 2022 at 1:22 AM gaoliming via groups.io<= /a> <gaoliming=3Dbyosoft.com= .cn@groups.io> wrote:

Reviewed-by: Liming Gao <gaoliming@byosoft.com.cn&= gt;

> -----=E9=82=AE=E4=BB=B6=E5=8E=9F=E4=BB=B6-----
> =E5=8F=91=E4=BB=B6=E4=BA=BA: Pedro Falcato <pedro.falcato@gmail.com>
> =E5=8F=91=E9=80=81=E6= =97=B6=E9=97=B4: 2022=E5=B9=B4= 11=E6=9C=883=E6=97=A5 9= :12
> =E6=94=B6=E4=BB=B6=E4=BA=BA: devel@edk2.groups.io=
> =E6=8A=84=E9=80=81: Pedro Falcato <pedro.falcato@gm= ail.com>; Vitaly Cheptsov
> <vit9696@protonmail.com>; Marvin H=C3= =A4user <mhaeuse= r@posteo.de>;
> Michael D Kinney <michael.d.kinney@intel.com>; = Liming Gao
> <gaoliming@byosoft.com.cn>; Zhiguang Liu <zhiguang.liu@intel.com&= gt;; Jiewen
> Yao <Jiewen.= yao@Intel.com>
> =E4=B8=BB=E9=A2=98:= [PATCH v3 1/1] MdePkg/BaseLib: Fix out-of-bounds reads in SafeString
&g= t;
> There was a OOB access in *StrHexTo* functions, when passed str= ings like
> "XDEADBEEF".
>
> OpenCore folks es= tablished an ASAN-equipped project to fuzz Ext4Dxe,
> which was able = to catch these (mostly harmless) issues.
>
> Cc: Vitaly Chepts= ov <vit9696@= protonmail.com>
> Cc: Marvin H=C3=A4user <mhaeuser@posteo.de>
> C= c: Michael D Kinney <michael.d.kinney@intel.com>
> Cc: Liming Gao <= gaoliming@byo= soft.com.cn>
> Cc: Zhiguang Liu <zhiguang.liu@intel.com>
> Si= gned-off-by: Pedro Falcato <pedro.falcato@gmail.com>
> Acked-by: Michael = D Kinney <michael.d.kinney@intel.com>
> Reviewed-by: Jiewen Yao <Jiewen.yao@Intel.com>
> = ---
>  MdePkg/Library/BaseLib/SafeString.c | 25 ++++++++++++++++= +++++----
>  1 file changed, 21 insertions(+), 4 deletions(-)>
> diff --git a/MdePkg/Library/BaseLib/SafeString.c
> b/M= dePkg/Library/BaseLib/SafeString.c
> index f338a32a3a41..b75b33381732= 100644
> --- a/MdePkg/Library/BaseLib/SafeString.c
> +++ b/Mde= Pkg/Library/BaseLib/SafeString.c
> @@ -863,6 +863,9 @@ StrHexToUintnS= (
>    OUT       UINTN   *Da= ta
>    )
>  {
> +  BOOLEAN  Fou= ndLeadingZero;
> +
> +  FoundLeadingZero =3D FALSE;
>= ;    ASSERT (((UINTN)String & BIT0) =3D=3D 0);
>
&g= t;    //
> @@ -892,12 +895,14 @@ StrHexToUintnS (
>&n= bsp;   //
>    // Ignore leading Zeros after the space= s
>    //
> +
> +  FoundLeadingZero =3D *S= tring =3D=3D L'0';
>    while (*String =3D=3D L'0') {
&g= t;      String++;
>    }
>
>&nb= sp;   if (CharToUpper (*String) =3D=3D L'X') {
> -    = if (*(String - 1) !=3D L'0') {
> +    if (!FoundLeadingZero= ) {
>        *Data =3D 0;
>    &n= bsp;   return RETURN_SUCCESS;
>      }
> @@= -992,6 +997,9 @@ StrHexToUint64S (
>    OUT    &= nbsp;  UINT64  *Data
>    )
>  {
&g= t; +  BOOLEAN  FoundLeadingZero;
> +
> +  FoundL= eadingZero =3D FALSE;
>    ASSERT (((UINTN)String & BIT= 0) =3D=3D 0);
>
>    //
> @@ -1021,12 +1029,13= @@ StrHexToUint64S (
>    //
>    // Ignor= e leading Zeros after the spaces
>    //
> +  Fo= undLeadingZero =3D *String =3D=3D L'0';
>    while (*String= =3D=3D L'0') {
>      String++;
>    = }
>
>    if (CharToUpper (*String) =3D=3D L'X') {> -    if (*(String - 1) !=3D L'0') {
> +    = if (!FoundLeadingZero) {
>        *Data =3D 0;>        return RETURN_SUCCESS;
>   = ;   }
> @@ -2393,6 +2402,9 @@ AsciiStrHexToUintnS (
> = ;   OUT       UINTN  *Data
>  &nbs= p; )
>  {
> +  BOOLEAN  FoundLeadingZero;
>= ; +
> +  FoundLeadingZero =3D FALSE;
>    //
= >    // 1. Neither String nor Data shall be a null pointer.>    //
> @@ -2420,12 +2432,13 @@ AsciiStrHexToUintnS (=
>    //
>    // Ignore leading Zeros after= the spaces
>    //
> +  FoundLeadingZero =3D *S= tring =3D=3D '0';
>    while (*String =3D=3D '0') {
>=       String++;
>    }
>
> = ;   if (AsciiCharToUpper (*String) =3D=3D 'X') {
> -   = ; if (*(String - 1) !=3D '0') {
> +    if (!FoundLeadingZer= o) {
>        *Data =3D 0;
>    &= nbsp;   return RETURN_SUCCESS;
>      }
> @= @ -2517,6 +2530,9 @@ AsciiStrHexToUint64S (
>    OUT  =      UINT64  *Data
>    )
> = {
> +  BOOLEAN  FoundLeadingZero;
> +
> + = ; FoundLeadingZero =3D FALSE;
>    //
>    = // 1. Neither String nor Data shall be a null pointer.
>   = //
> @@ -2544,12 +2560,13 @@ AsciiStrHexToUint64S (
>  &n= bsp; //
>    // Ignore leading Zeros after the spaces
&g= t;    //
> +  FoundLeadingZero =3D *String =3D=3D '0';=
>    while (*String =3D=3D '0') {
>    &nb= sp; String++;
>    }
>
>    if (Asci= iCharToUpper (*String) =3D=3D 'X') {
> -    if (*(String - = 1) !=3D '0') {
> +    if (!FoundLeadingZero) {
> = ;       *Data =3D 0;
>        retu= rn RETURN_SUCCESS;
>      }
> --
> 2.38.1<= br>








--

Pedro Falcato=

------=_NextPart_000_016C_01D8F28B.DD51EF60--