From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-pj1-f41.google.com (mail-pj1-f41.google.com [209.85.216.41]) by mx.groups.io with SMTP id smtpd.web09.1148.1667808856289399010 for ; Mon, 07 Nov 2022 00:14:16 -0800 Authentication-Results: mx.groups.io; dkim=pass header.i=@gmail.com header.s=20210112 header.b=I6pHZ3hz; spf=pass (domain: gmail.com, ip: 209.85.216.41, mailfrom: pedro.falcato@gmail.com) Received: by mail-pj1-f41.google.com with SMTP id c15-20020a17090a1d0f00b0021365864446so9582324pjd.4 for ; Mon, 07 Nov 2022 00:14:16 -0800 (PST) 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=zO7vCOg7t0PdVsjQhoQm0VaWB4YGj5Yvc2q4EXP0cFk=; b=I6pHZ3hz+LfMP1nF+vZyVhoeqKiICLGHfgkwonRhu2XKUYE5jqOD7uGUqK5kboj1my ppkfjy2pyftac/3PfoyZH+uFKLDoxgYs1yvnzbJQmSzDYxjUzjfBQKkKnbj1DfqcxoWJ ayXf2fBBK/kteZQVrQpuZSjU9Fp7n5FRAe37JqyC6MvEaNgJJiMBpDV9lwLPbggkFCx+ 8d/CDAxMdz1Vndq6/BonlWwyAeTFj0XsDtyhapFrfmb6w44w4gPzb/dWOENaARBNHBK2 ojfCvHGhVHqoePjua5VlZabb3vmsJe2iVqg0rJ4zs8xcBTUk++GzrZY9SAYUya/L/lCX laxQ== 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=zO7vCOg7t0PdVsjQhoQm0VaWB4YGj5Yvc2q4EXP0cFk=; b=iVFS692uy6KmcsFd7RWev3AVZT371uJc7pGV27RMMGMFsCEDjQKsdHrJWCyQDt58IU wkdoDuRVu3auaTuTxHwO5gVjdYwmHc0o72LNXG/X6+zfBWFZmrpbGizNhW1j2yhABPPk bv82N6gsoYKg87rZytrwI2tkY3pd2s8oAJHnhsXtqD7TRg7crX/zxNx1Q8W7dzNbanO8 GMUuUhAkXBDpY/Xn0kx3qmqzSNtPZjnrLYNSrd6TXeoUpNXDH1YBPGjnKp18NXKEZsyH yBtjDS/Rr2L7Kg/heB4enPF3oXvTG6Fzqs13dEuTYr6mGVtNyaoyR6zdcUlZjFboYeef Jnyw== X-Gm-Message-State: ACrzQf2exjmujD6FxCDlICnInz3fdFkPvVQfpX45j9BUK0NRpMHVK1pL nuACVRKnXtUGtalmhl77+pNrdVyY27AeIzxweQUjaxsJiXQ= X-Google-Smtp-Source: AMsMyM6i/SCUWFTQWm9mZYBqHpGzL5LOBNi8WpcAj4hDcSSN2kt59cb0ecRoggHMG5P6ikMUNRltAr85nqyeqSU+dgY= X-Received: by 2002:a17:902:f685:b0:186:fa9c:2fdc with SMTP id l5-20020a170902f68500b00186fa9c2fdcmr49705413plg.25.1667808855291; Mon, 07 Nov 2022 00:14:15 -0800 (PST) MIME-Version: 1.0 References: <20221103011149.659815-1-pedro.falcato@gmail.com> <000201d8efeb$f43533b0$dc9f9b10$@byosoft.com.cn> <016b01d8f248$cf2d28c0$6d877a40$@byosoft.com.cn> In-Reply-To: <016b01d8f248$cf2d28c0$6d877a40$@byosoft.com.cn> From: "Pedro Falcato" Date: Mon, 7 Nov 2022 08:14:05 +0000 Message-ID: Subject: =?UTF-8?B?UmU6IFtlZGsyLWRldmVsXSDlm57lpI06IFtQQVRDSCB2MyAxLzFdIE1kZVBrZy9CYXNlTGliOiBGaXggb3V0LW9mLWJvdW5kcyByZWFkcyBpbiBTYWZlU3RyaW5n?= To: edk2-devel-groups-io , Liming Gao Cc: Vitaly Cheptsov , =?UTF-8?Q?Marvin_H=C3=A4user?= , Michael D Kinney , Zhiguang Liu , Jiewen Yao Content-Type: multipart/alternative; boundary="0000000000005f7ca205ecdd02b0" --0000000000005f7ca205ecdd02b0 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable Thanks! On Mon, 7 Nov 2022, 01:32 gaoliming via groups.io, wrote: > Create https://github.com/tianocore/edk2/pull/3604 to merge this patch. > > > > 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.co= m.cn > *=E6=8A=84=E9=80=81:* Vitaly Cheptsov ; Marvin H= =C3=A4user < > mhaeuser@posteo.de>; 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]= MdePkg/BaseLib: Fix > out-of-bounds reads in SafeString > > > > Hi Liming, > > > > Thank you for the review. Can we please push this in time for the stable > tag? > > > > Thanks, > > Pedro > > > > On Fri, Nov 4, 2022 at 1:22 AM gaoliming via groups.io byosoft.com.cn@groups.io> wrote: > > 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 > > =E6=8A=84=E9=80=81: Pedro Falcato ; Vitaly Che= ptsov > > ; Marvin H=C3=A4user ; > > Michael D Kinney ; Liming Gao > > ; Zhiguang Liu ; > Jiewen > > Yao > > =E4=B8=BB=E9=A2=98: [PATCH v3 1/1] MdePkg/BaseLib: Fix out-of-bounds re= ads in SafeString > > > > There was a OOB access in *StrHexTo* functions, when passed strings lik= e > > "XDEADBEEF". > > > > OpenCore folks established an ASAN-equipped project to fuzz Ext4Dxe, > > which was able to catch these (mostly harmless) issues. > > > > 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(-) > > > > 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); > > > > // > > @@ -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++; > > } > > > > 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); > > > > // > > @@ -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++; > > } > > > > 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++; > > } > > > > 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++; > > } > > > > if (AsciiCharToUpper (*String) =3D=3D 'X') { > > - if (*(String - 1) !=3D '0') { > > + if (!FoundLeadingZero) { > > *Data =3D 0; > > return RETURN_SUCCESS; > > } > > -- > > 2.38.1 > > > > > > > > > > -- > > Pedro Falcato > >=20 > --0000000000005f7ca205ecdd02b0 Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable
Thanks!

On Mon, 7 Nov 2022, 01:32 gaoliming via groups.io, <gaoliming=3Dbyosoft.com.cn@groups.io> wrote:

Create https://github.co= m/tianocore/edk2/pull/3604 to merge this patch.

=C2=A0

Thanks

Liming=

=E5=8F=91=E4= =BB=B6=E4=BA=BA: devel@edk2.gr= oups.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=88= 5=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 <michael.d.kinney@intel.com>; Zhiguang Liu <zhiguang.liu@inte= l.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

=C2=A0

Hi Liming,

=C2=A0

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

=C2=A0

<= div>

Thanks,

Pedro=

<= /u>=C2=A0

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

Reviewed-by: Liming Gao &= lt;gaoliming@byosoft.com.cn>

> -----=E9=82=AE= =E4=BB=B6=E5=8E=9F=E4=BB=B6-----
> =E5=8F= =91=E4=BB=B6=E4=BA=BA: Pedro Falcato <pedro.fa= lcato@gmail.com>
> =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@edk= 2.groups.io
> =E6=8A=84=E9=80=81: Ped= ro Falcato <pedro.falcato@gmail.com>; Vitaly Cheptsov
>= ; <vit9696@protonmail.com>; Marvin H=C3=A4user <mhaeuser@p= osteo.de>;
> Michael D Kinney <michael.d.kinney@int= el.com>; Liming Gao
> <gaoliming@byosoft.com.cn&g= t;; Zhiguang Liu <zhiguang.liu@intel.com>; 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
>
> There was a OOB access in *StrHexTo* functions,= when passed strings like
> "XDEADBEEF".
>
> O= penCore folks established an ASAN-equipped project to fuzz Ext4Dxe,
>= which was able to catch these (mostly harmless) issues.
>
> C= c: Vitaly Cheptsov <vit9696@protonmail.com>
> Cc: Marvi= n H=C3=A4user <mhaeuser@posteo.de>
> Cc: Michael D Kinney &= lt;michael.d.kinney@intel.com>
> Cc: Liming Gao <gaoliming@byosoft.com.cn>
> Cc: Zhiguang Liu <zhi= guang.liu@intel.com>
> Signed-off-by: Pedro Falcato <p= edro.falcato@gmail.com>
> Acked-by: Michael D Kinney <michael.d.kinney@intel.com>
> Reviewed-by: Jiewen Yao <<= a href=3D"mailto:Jiewen.yao@Intel.com" target=3D"_blank" rel=3D"noreferrer"= >Jiewen.yao@Intel.com>
> ---
>=C2=A0 MdePkg/Library/Base= Lib/SafeString.c | 25 +++++++++++++++++++++----
>=C2=A0 1 file change= d, 21 insertions(+), 4 deletions(-)
>
> diff --git a/MdePkg/Li= brary/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
&= gt; @@ -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)Str= ing & BIT0) =3D=3D 0);
>
>=C2=A0 =C2=A0 //
> @@ -892= ,12 +895,14 @@ StrHexToUintnS (
>=C2=A0 =C2=A0 //
>=C2=A0 =C2= =A0 // Ignore leading Zeros after the spaces
>=C2=A0 =C2=A0 //
>= ; +
> +=C2=A0 FoundLeadingZero =3D *String =3D=3D L'0';
&g= t;=C2=A0 =C2=A0 while (*String =3D=3D L'0') {
>=C2=A0 =C2=A0 = =C2=A0 String++;
>=C2=A0 =C2=A0 }
>
>=C2=A0 =C2=A0 if (C= harToUpper (*String) =3D=3D L'X') {
> -=C2=A0 =C2=A0 if (*(St= ring - 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 +997,9 @@ StrHexToUint64S (
>=C2=A0 =C2=A0 OUT=C2=A0 =C2=A0= =C2=A0 =C2=A0UINT64=C2=A0 *Data
>=C2=A0 =C2=A0 )
>=C2=A0 {
= > +=C2=A0 BOOLEAN=C2=A0 FoundLeadingZero;
> +
> +=C2=A0 Foun= dLeadingZero =3D FALSE;
>=C2=A0 =C2=A0 ASSERT (((UINTN)String & B= IT0) =3D=3D 0);
>
>=C2=A0 =C2=A0 //
> @@ -1021,12 +1029,= 13 @@ StrHexToUint64S (
>=C2=A0 =C2=A0 //
>=C2=A0 =C2=A0 // Ign= ore leading Zeros after the spaces
>=C2=A0 =C2=A0 //
> +=C2=A0 = FoundLeadingZero =3D *String =3D=3D L'0';
>=C2=A0 =C2=A0 whil= e (*String =3D=3D L'0') {
>=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 +2402,9 @= @ AsciiStrHexToUintnS (
>=C2=A0 =C2=A0 OUT=C2=A0 =C2=A0 =C2=A0 =C2=A0= UINTN=C2=A0 *Data
>=C2=A0 =C2=A0 )
>=C2=A0 {
> +=C2=A0 BO= OLEAN=C2=A0 FoundLeadingZero;
> +
> +=C2=A0 FoundLeadingZero = =3D FALSE;
>=C2=A0 =C2=A0 //
>=C2=A0 =C2=A0 // 1. Neither Strin= g nor Data shall be a null pointer.
>=C2=A0 =C2=A0 //
> @@ -242= 0,12 +2432,13 @@ AsciiStrHexToUintnS (
>=C2=A0 =C2=A0 //
>=C2= =A0 =C2=A0 // Ignore leading Zeros after the spaces
>=C2=A0 =C2=A0 //=
> +=C2=A0 FoundLeadingZero =3D *String =3D=3D '0';
>= =C2=A0 =C2=A0 while (*String =3D=3D '0') {
>=C2=A0 =C2=A0 =C2= =A0 String++;
>=C2=A0 =C2=A0 }
>
>=C2=A0 =C2=A0 if (Asci= iCharToUpper (*String) =3D=3D 'X') {
> -=C2=A0 =C2=A0 if (*(S= tring - 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 +2530,9 @@ AsciiStrHexToUint64S (
>=C2=A0 =C2=A0 OUT=C2=A0= =C2=A0 =C2=A0 =C2=A0UINT64=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 pointer.
>=C2=A0= =C2=A0 //
> @@ -2544,12 +2560,13 @@ AsciiStrHexToUint64S (
>= =C2=A0 =C2=A0 //
>=C2=A0 =C2=A0 // Ignore leading Zeros after the spa= ces
>=C2=A0 =C2=A0 //
> +=C2=A0 FoundLeadingZero =3D *String = =3D=3D '0';
>=C2=A0 =C2=A0 while (*String =3D=3D '0')= {
>=C2=A0 =C2=A0 =C2=A0 String++;
>=C2=A0 =C2=A0 }
> >=C2=A0 =C2=A0 if (AsciiCharToUpper (*String) =3D=3D 'X') {
= > -=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

=20

--0000000000005f7ca205ecdd02b0--