From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-pf1-f169.google.com (mail-pf1-f169.google.com [209.85.210.169]) by mx.groups.io with SMTP id smtpd.web09.1792.1667607895744708383 for ; Fri, 04 Nov 2022 17:24:55 -0700 Authentication-Results: mx.groups.io; dkim=pass header.i=@gmail.com header.s=20210112 header.b=Gu/fiK2r; spf=pass (domain: gmail.com, ip: 209.85.210.169, mailfrom: pedro.falcato@gmail.com) Received: by mail-pf1-f169.google.com with SMTP id v28so5840950pfi.12 for ; Fri, 04 Nov 2022 17:24:55 -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=giIlqTC2LQlNzXoN/hJW4uA3wXEVIJfG+OQk4DSaKzA=; b=Gu/fiK2rrbXnesnTHp/MfV8ym0i7wFt6sv5kzqxsHF4hCm7agt1XfIN/3ID37aOrhd L82qTC59ZPtKIi3amZR1VzhQpa00AFfKUAUO1yDJ/B047CX8hLBH7/xC4VdcfxiZlv2Z h952ZFNd8qs1mucapuM2IKOxGhuZnmdjVQh5U2Nn49QnARfZvQits174UG6axsIMDsRN PwsDaOun1i1kDHarOlzEa+88dZu51+JvJZZMvgIbSE9kMdxYAVc1mK2vZFEeK6op8njB 99wVV3Nb/Z91UhYrYj66HPlXDdKXsez7ugcSs1Emozz27SQI3ErOvtB2714FoUAojFwU YRkQ== 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=giIlqTC2LQlNzXoN/hJW4uA3wXEVIJfG+OQk4DSaKzA=; b=nDof0RJqjZYKUwv4jjvPfGZERbu2lAyjY9KrM2C85la9QMIuq140oU+Z2Ezp8uk/gq 3LCRnUx001cvHYdhH+chhZ4iRWpfURO/sf1AtaJyfwS7nGphvE75gFH8RLE/BSHrxovs ANFqLFblU0cUn52l/ky72nZAOHSPdD1roQuiHXPoGpUnqK8SdAMCAT/qnEoFa6EstJbO gcrow2GbndebzeKRcPN4WyLwyuUePt0rdIV08dys9N2TeIrwDvUOQSHpGhAg8j0+ND8R LeQ8YYRo/WIwHxcYRAQDnPJV5OpcAkJZQmJte4a0Lr+6jp9JUakx6HHb8mPXpi0UerPe 9rag== X-Gm-Message-State: ACrzQf06eKEKI2UOxixNYw8cOStThC0pM4LdgchLBxHRrZXUH/e+tl9c GHsFTFhNMrr79hSZi2b6MwUHRd3IM+cgIuOZvWLFQlJHHBw= X-Google-Smtp-Source: AMsMyM4iLStloWlatbmvRwffyrZjsOaTuJDYRwoRW84Jfy6irXNqN4/az0r72z0iBmSV7FJ1XXLY2b8ocVN9gJoank8= X-Received: by 2002:a63:5b23:0:b0:46f:6d7d:cd10 with SMTP id p35-20020a635b23000000b0046f6d7dcd10mr33022220pgb.194.1667607894842; Fri, 04 Nov 2022 17:24:54 -0700 (PDT) MIME-Version: 1.0 References: <20221103011149.659815-1-pedro.falcato@gmail.com> <000201d8efeb$f43533b0$dc9f9b10$@byosoft.com.cn> In-Reply-To: <000201d8efeb$f43533b0$dc9f9b10$@byosoft.com.cn> From: "Pedro Falcato" Date: Sat, 5 Nov 2022 00:24:43 +0000 Message-ID: Subject: =?UTF-8?B?UmU6IFtlZGsyLWRldmVsXSDlm57lpI06IFtQQVRDSCB2MyAxLzFdIE1kZVBrZy9CYXNlTGliOiBGaXggb3V0LW9mLWJvdW5kcyByZWFkcyBpbiBTYWZlU3RyaW5n?= To: devel@edk2.groups.io, gaoliming@byosoft.com.cn Cc: Vitaly Cheptsov , =?UTF-8?Q?Marvin_H=C3=A4user?= , Michael D Kinney , Zhiguang Liu , Jiewen Yao Content-Type: multipart/alternative; boundary="00000000000032622105ecae38fb" --00000000000032622105ecae38fb Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable 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 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 > > > > > >=20 > > > --=20 Pedro Falcato --00000000000032622105ecae38fb Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable
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 <gaoliming=3Dbyosoft.com.cn@groups.io> = wrote:
Reviewed-= by: Liming Gao <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.falcato@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@edk2.groups.io
> =E6=8A=84=E9=80=81: Pedro Falcato <pedro.falcato@gmail.com>; Vitaly Chepts= ov
> <vit969= 6@protonmail.com>; Marvin H=C3=A4user <mhaeuser@posteo.de>;
> Michael D Kinney <michael.d.kinney@intel.com>; Liming Gao
> <gaol= iming@byosoft.com.cn>; 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 r= eads in SafeString
>
> There was a OOB access in *StrHexTo* functions, when passed strings li= ke
> "XDEADBEEF".
>
> OpenCore folks established an ASAN-equipped project to fuzz Ext4Dxe, > which was able to catch these (mostly harmless) issues.
>
> 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>
> Signed-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>
> ---
>=C2=A0 MdePkg/Library/BaseLib/SafeString.c | 25 +++++++++++++++++++++--= --
>=C2=A0 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 (
>=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 //
> @@ -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';
>=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 (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 +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 FoundLeadingZero =3D FALSE;
>=C2=A0 =C2=A0 ASSERT (((UINTN)String & BIT0) =3D=3D 0);
>
>=C2=A0 =C2=A0 //
> @@ -1021,12 +1029,13 @@ StrHexToUint64S (
>=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';
>=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 (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=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 pointer. >=C2=A0 =C2=A0 //
> @@ -2420,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 (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 }
> @@ -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 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 (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
--00000000000032622105ecae38fb--