* [PATCH v2 1/1] MdePkg/BaseLib: Fix out-of-bounds reads in SafeString @ 2022-10-24 22:43 Pedro Falcato 2022-10-25 16:22 ` Michael D Kinney 0 siblings, 1 reply; 8+ messages in thread From: Pedro Falcato @ 2022-10-24 22:43 UTC (permalink / raw) To: devel Cc: Pedro Falcato, Vitaly Cheptsov, Marvin Häuser, Michael D Kinney, Liming Gao, Zhiguang Liu 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äuser <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> --- 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 = FALSE; ASSERT (((UINTN)String & BIT0) == 0); // @@ -893,11 +896,12 @@ StrHexToUintnS ( // Ignore leading Zeros after the spaces // while (*String == L'0') { + FoundLeadingZero = TRUE; String++; } if (CharToUpper (*String) == L'X') { - if (*(String - 1) != L'0') { + if (!FoundLeadingZero) { *Data = 0; return RETURN_SUCCESS; } @@ -992,6 +996,9 @@ StrHexToUint64S ( OUT UINT64 *Data ) { + BOOLEAN FoundLeadingZero; + + FoundLeadingZero = FALSE; ASSERT (((UINTN)String & BIT0) == 0); // @@ -1022,11 +1029,12 @@ StrHexToUint64S ( // Ignore leading Zeros after the spaces // while (*String == L'0') { + FoundLeadingZero = TRUE; String++; } if (CharToUpper (*String) == L'X') { - if (*(String - 1) != L'0') { + if (!FoundLeadingZero) { *Data = 0; return RETURN_SUCCESS; } @@ -2393,6 +2401,9 @@ AsciiStrHexToUintnS ( OUT UINTN *Data ) { + BOOLEAN FoundLeadingZero; + + FoundLeadingZero = FALSE; // // 1. Neither String nor Data shall be a null pointer. // @@ -2421,11 +2432,12 @@ AsciiStrHexToUintnS ( // Ignore leading Zeros after the spaces // while (*String == '0') { + FoundLeadingZero = TRUE; String++; } if (AsciiCharToUpper (*String) == 'X') { - if (*(String - 1) != '0') { + if (!FoundLeadingZero) { *Data = 0; return RETURN_SUCCESS; } @@ -2517,6 +2529,9 @@ AsciiStrHexToUint64S ( OUT UINT64 *Data ) { + BOOLEAN FoundLeadingZero; + + FoundLeadingZero = FALSE; // // 1. Neither String nor Data shall be a null pointer. // @@ -2545,11 +2560,12 @@ AsciiStrHexToUint64S ( // Ignore leading Zeros after the spaces // while (*String == '0') { + FoundLeadingZero = TRUE; String++; } if (AsciiCharToUpper (*String) == 'X') { - if (*(String - 1) != '0') { + if (!FoundLeadingZero) { *Data = 0; return RETURN_SUCCESS; } -- 2.38.1 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH v2 1/1] MdePkg/BaseLib: Fix out-of-bounds reads in SafeString 2022-10-24 22:43 [PATCH v2 1/1] MdePkg/BaseLib: Fix out-of-bounds reads in SafeString Pedro Falcato @ 2022-10-25 16:22 ` Michael D Kinney 2022-10-26 13:34 ` Yao, Jiewen 0 siblings, 1 reply; 8+ messages in thread From: Michael D Kinney @ 2022-10-25 16:22 UTC (permalink / raw) To: Pedro Falcato, devel@edk2.groups.io Cc: Vitaly Cheptsov, Marvin Häuser, Gao, Liming, Liu, Zhiguang, Yao, Jiewen 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 <vit9696@protonmail.com>; Marvin Häuser <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 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 <pedro.falcato@gmail.com> > Cc: Vitaly Cheptsov <vit9696@protonmail.com> > Cc: Marvin Häuser <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> > --- > 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 = FALSE; > ASSERT (((UINTN)String & BIT0) == 0); > > // > @@ -893,11 +896,12 @@ StrHexToUintnS ( > // Ignore leading Zeros after the spaces > // > while (*String == L'0') { > + FoundLeadingZero = TRUE; > String++; > } > > if (CharToUpper (*String) == L'X') { > - if (*(String - 1) != L'0') { > + if (!FoundLeadingZero) { > *Data = 0; > return RETURN_SUCCESS; > } > @@ -992,6 +996,9 @@ StrHexToUint64S ( > OUT UINT64 *Data > ) > { > + BOOLEAN FoundLeadingZero; > + > + FoundLeadingZero = FALSE; > ASSERT (((UINTN)String & BIT0) == 0); > > // > @@ -1022,11 +1029,12 @@ StrHexToUint64S ( > // Ignore leading Zeros after the spaces > // > while (*String == L'0') { > + FoundLeadingZero = TRUE; > String++; > } > > if (CharToUpper (*String) == L'X') { > - if (*(String - 1) != L'0') { > + if (!FoundLeadingZero) { > *Data = 0; > return RETURN_SUCCESS; > } > @@ -2393,6 +2401,9 @@ AsciiStrHexToUintnS ( > OUT UINTN *Data > ) > { > + BOOLEAN FoundLeadingZero; > + > + FoundLeadingZero = FALSE; > // > // 1. Neither String nor Data shall be a null pointer. > // > @@ -2421,11 +2432,12 @@ AsciiStrHexToUintnS ( > // Ignore leading Zeros after the spaces > // > while (*String == '0') { > + FoundLeadingZero = TRUE; > String++; > } > > if (AsciiCharToUpper (*String) == 'X') { > - if (*(String - 1) != '0') { > + if (!FoundLeadingZero) { > *Data = 0; > return RETURN_SUCCESS; > } > @@ -2517,6 +2529,9 @@ AsciiStrHexToUint64S ( > OUT UINT64 *Data > ) > { > + BOOLEAN FoundLeadingZero; > + > + FoundLeadingZero = FALSE; > // > // 1. Neither String nor Data shall be a null pointer. > // > @@ -2545,11 +2560,12 @@ AsciiStrHexToUint64S ( > // Ignore leading Zeros after the spaces > // > while (*String == '0') { > + FoundLeadingZero = TRUE; > String++; > } > > if (AsciiCharToUpper (*String) == 'X') { > - if (*(String - 1) != '0') { > + if (!FoundLeadingZero) { > *Data = 0; > return RETURN_SUCCESS; > } > -- > 2.38.1 ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2 1/1] MdePkg/BaseLib: Fix out-of-bounds reads in SafeString 2022-10-25 16:22 ` Michael D Kinney @ 2022-10-26 13:34 ` Yao, Jiewen 2022-10-26 13:41 ` Marvin Häuser 2022-10-26 15:54 ` Michael D Kinney 0 siblings, 2 replies; 8+ messages in thread From: Yao, Jiewen @ 2022-10-26 13:34 UTC (permalink / raw) To: Kinney, Michael D, Pedro Falcato, devel@edk2.groups.io Cc: Vitaly Cheptsov, Marvin Häuser, Gao, Liming, Liu, Zhiguang 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äuser > <mhaeuser@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 reads 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 > <vit9696@protonmail.com>; Marvin Häuser <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 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 <pedro.falcato@gmail.com> > > Cc: Vitaly Cheptsov <vit9696@protonmail.com> > > Cc: Marvin Häuser <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> > > --- > > 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 = FALSE; > > ASSERT (((UINTN)String & BIT0) == 0); > > > > // > > @@ -893,11 +896,12 @@ StrHexToUintnS ( > > // Ignore leading Zeros after the spaces > > // > > while (*String == L'0') { > > + FoundLeadingZero = TRUE; > > String++; > > } > > > > if (CharToUpper (*String) == L'X') { > > - if (*(String - 1) != L'0') { > > + if (!FoundLeadingZero) { > > *Data = 0; > > return RETURN_SUCCESS; > > } > > @@ -992,6 +996,9 @@ StrHexToUint64S ( > > OUT UINT64 *Data > > ) > > { > > + BOOLEAN FoundLeadingZero; > > + > > + FoundLeadingZero = FALSE; > > ASSERT (((UINTN)String & BIT0) == 0); > > > > // > > @@ -1022,11 +1029,12 @@ StrHexToUint64S ( > > // Ignore leading Zeros after the spaces > > // > > while (*String == L'0') { > > + FoundLeadingZero = TRUE; > > String++; > > } > > > > if (CharToUpper (*String) == L'X') { > > - if (*(String - 1) != L'0') { > > + if (!FoundLeadingZero) { > > *Data = 0; > > return RETURN_SUCCESS; > > } > > @@ -2393,6 +2401,9 @@ AsciiStrHexToUintnS ( > > OUT UINTN *Data > > ) > > { > > + BOOLEAN FoundLeadingZero; > > + > > + FoundLeadingZero = FALSE; > > // > > // 1. Neither String nor Data shall be a null pointer. > > // > > @@ -2421,11 +2432,12 @@ AsciiStrHexToUintnS ( > > // Ignore leading Zeros after the spaces > > // > > while (*String == '0') { > > + FoundLeadingZero = TRUE; > > String++; > > } > > > > if (AsciiCharToUpper (*String) == 'X') { > > - if (*(String - 1) != '0') { > > + if (!FoundLeadingZero) { > > *Data = 0; > > return RETURN_SUCCESS; > > } > > @@ -2517,6 +2529,9 @@ AsciiStrHexToUint64S ( > > OUT UINT64 *Data > > ) > > { > > + BOOLEAN FoundLeadingZero; > > + > > + FoundLeadingZero = FALSE; > > // > > // 1. Neither String nor Data shall be a null pointer. > > // > > @@ -2545,11 +2560,12 @@ AsciiStrHexToUint64S ( > > // Ignore leading Zeros after the spaces > > // > > while (*String == '0') { > > + FoundLeadingZero = TRUE; > > String++; > > } > > > > if (AsciiCharToUpper (*String) == 'X') { > > - if (*(String - 1) != '0') { > > + if (!FoundLeadingZero) { > > *Data = 0; > > return RETURN_SUCCESS; > > } > > -- > > 2.38.1 ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2 1/1] MdePkg/BaseLib: Fix out-of-bounds reads in SafeString 2022-10-26 13:34 ` Yao, Jiewen @ 2022-10-26 13:41 ` Marvin Häuser 2022-10-26 15:54 ` Michael D Kinney 1 sibling, 0 replies; 8+ messages in thread From: Marvin Häuser @ 2022-10-26 13:41 UTC (permalink / raw) To: Yao, Jiewen Cc: Kinney, Michael D, Pedro Falcato, devel, Vitaly Cheptsov, Gao, Liming, Liu, Zhiguang Comment inline. > On 26. Oct 2022, at 15:35, Yao, Jiewen <jiewen.yao@intel.com> wrote: > > 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äuser >> <mhaeuser@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 reads 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 >> <vit9696@protonmail.com>; Marvin Häuser <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 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 <pedro.falcato@gmail.com> >>> Cc: Vitaly Cheptsov <vit9696@protonmail.com> >>> Cc: Marvin Häuser <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> >>> --- >>> 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 = FALSE; >>> ASSERT (((UINTN)String & BIT0) == 0); >>> >>> // >>> @@ -893,11 +896,12 @@ StrHexToUintnS ( >>> // Ignore leading Zeros after the spaces >>> // >>> while (*String == L'0') { >>> + FoundLeadingZero = TRUE; I would just assign these outside the loop. Simpler, easier to read and understand, might emit better bins. Best regards, Marvin >>> String++; >>> } >>> >>> if (CharToUpper (*String) == L'X') { >>> - if (*(String - 1) != L'0') { >>> + if (!FoundLeadingZero) { >>> *Data = 0; >>> return RETURN_SUCCESS; >>> } >>> @@ -992,6 +996,9 @@ StrHexToUint64S ( >>> OUT UINT64 *Data >>> ) >>> { >>> + BOOLEAN FoundLeadingZero; >>> + >>> + FoundLeadingZero = FALSE; >>> ASSERT (((UINTN)String & BIT0) == 0); >>> >>> // >>> @@ -1022,11 +1029,12 @@ StrHexToUint64S ( >>> // Ignore leading Zeros after the spaces >>> // >>> while (*String == L'0') { >>> + FoundLeadingZero = TRUE; >>> String++; >>> } >>> >>> if (CharToUpper (*String) == L'X') { >>> - if (*(String - 1) != L'0') { >>> + if (!FoundLeadingZero) { >>> *Data = 0; >>> return RETURN_SUCCESS; >>> } >>> @@ -2393,6 +2401,9 @@ AsciiStrHexToUintnS ( >>> OUT UINTN *Data >>> ) >>> { >>> + BOOLEAN FoundLeadingZero; >>> + >>> + FoundLeadingZero = FALSE; >>> // >>> // 1. Neither String nor Data shall be a null pointer. >>> // >>> @@ -2421,11 +2432,12 @@ AsciiStrHexToUintnS ( >>> // Ignore leading Zeros after the spaces >>> // >>> while (*String == '0') { >>> + FoundLeadingZero = TRUE; >>> String++; >>> } >>> >>> if (AsciiCharToUpper (*String) == 'X') { >>> - if (*(String - 1) != '0') { >>> + if (!FoundLeadingZero) { >>> *Data = 0; >>> return RETURN_SUCCESS; >>> } >>> @@ -2517,6 +2529,9 @@ AsciiStrHexToUint64S ( >>> OUT UINT64 *Data >>> ) >>> { >>> + BOOLEAN FoundLeadingZero; >>> + >>> + FoundLeadingZero = FALSE; >>> // >>> // 1. Neither String nor Data shall be a null pointer. >>> // >>> @@ -2545,11 +2560,12 @@ AsciiStrHexToUint64S ( >>> // Ignore leading Zeros after the spaces >>> // >>> while (*String == '0') { >>> + FoundLeadingZero = TRUE; >>> String++; >>> } >>> >>> if (AsciiCharToUpper (*String) == 'X') { >>> - if (*(String - 1) != '0') { >>> + if (!FoundLeadingZero) { >>> *Data = 0; >>> return RETURN_SUCCESS; >>> } >>> -- >>> 2.38.1 > ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2 1/1] MdePkg/BaseLib: Fix out-of-bounds reads in SafeString 2022-10-26 13:34 ` Yao, Jiewen 2022-10-26 13:41 ` Marvin Häuser @ 2022-10-26 15:54 ` Michael D Kinney 2022-11-02 23:42 ` Pedro Falcato 1 sibling, 1 reply; 8+ messages in thread From: Michael D Kinney @ 2022-10-26 15:54 UTC (permalink / raw) To: Yao, Jiewen, Pedro Falcato, devel@edk2.groups.io, Kinney, Michael D Cc: Vitaly Cheptsov, Marvin Häuser, Gao, Liming, Liu, Zhiguang Acked-by: Michael D Kinney <michael.d.kinney@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 <pedro.falcato@gmail.com>; devel@edk2.groups.io > Cc: Vitaly Cheptsov <vit9696@protonmail.com>; Marvin Häuser <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äuser > > <mhaeuser@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 reads 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 > > <vit9696@protonmail.com>; Marvin Häuser <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 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 <pedro.falcato@gmail.com> > > > Cc: Vitaly Cheptsov <vit9696@protonmail.com> > > > Cc: Marvin Häuser <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> > > > --- > > > 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 = FALSE; > > > ASSERT (((UINTN)String & BIT0) == 0); > > > > > > // > > > @@ -893,11 +896,12 @@ StrHexToUintnS ( > > > // Ignore leading Zeros after the spaces > > > // > > > while (*String == L'0') { > > > + FoundLeadingZero = TRUE; > > > String++; > > > } > > > > > > if (CharToUpper (*String) == L'X') { > > > - if (*(String - 1) != L'0') { > > > + if (!FoundLeadingZero) { > > > *Data = 0; > > > return RETURN_SUCCESS; > > > } > > > @@ -992,6 +996,9 @@ StrHexToUint64S ( > > > OUT UINT64 *Data > > > ) > > > { > > > + BOOLEAN FoundLeadingZero; > > > + > > > + FoundLeadingZero = FALSE; > > > ASSERT (((UINTN)String & BIT0) == 0); > > > > > > // > > > @@ -1022,11 +1029,12 @@ StrHexToUint64S ( > > > // Ignore leading Zeros after the spaces > > > // > > > while (*String == L'0') { > > > + FoundLeadingZero = TRUE; > > > String++; > > > } > > > > > > if (CharToUpper (*String) == L'X') { > > > - if (*(String - 1) != L'0') { > > > + if (!FoundLeadingZero) { > > > *Data = 0; > > > return RETURN_SUCCESS; > > > } > > > @@ -2393,6 +2401,9 @@ AsciiStrHexToUintnS ( > > > OUT UINTN *Data > > > ) > > > { > > > + BOOLEAN FoundLeadingZero; > > > + > > > + FoundLeadingZero = FALSE; > > > // > > > // 1. Neither String nor Data shall be a null pointer. > > > // > > > @@ -2421,11 +2432,12 @@ AsciiStrHexToUintnS ( > > > // Ignore leading Zeros after the spaces > > > // > > > while (*String == '0') { > > > + FoundLeadingZero = TRUE; > > > String++; > > > } > > > > > > if (AsciiCharToUpper (*String) == 'X') { > > > - if (*(String - 1) != '0') { > > > + if (!FoundLeadingZero) { > > > *Data = 0; > > > return RETURN_SUCCESS; > > > } > > > @@ -2517,6 +2529,9 @@ AsciiStrHexToUint64S ( > > > OUT UINT64 *Data > > > ) > > > { > > > + BOOLEAN FoundLeadingZero; > > > + > > > + FoundLeadingZero = FALSE; > > > // > > > // 1. Neither String nor Data shall be a null pointer. > > > // > > > @@ -2545,11 +2560,12 @@ AsciiStrHexToUint64S ( > > > // Ignore leading Zeros after the spaces > > > // > > > while (*String == '0') { > > > + FoundLeadingZero = TRUE; > > > String++; > > > } > > > > > > if (AsciiCharToUpper (*String) == 'X') { > > > - if (*(String - 1) != '0') { > > > + if (!FoundLeadingZero) { > > > *Data = 0; > > > return RETURN_SUCCESS; > > > } > > > -- > > > 2.38.1 ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2 1/1] MdePkg/BaseLib: Fix out-of-bounds reads in SafeString 2022-10-26 15:54 ` Michael D Kinney @ 2022-11-02 23:42 ` Pedro Falcato 2022-11-03 1:00 ` 回复: " gaoliming 0 siblings, 1 reply; 8+ messages in thread From: Pedro Falcato @ 2022-11-02 23:42 UTC (permalink / raw) To: Kinney, Michael D Cc: Yao, Jiewen, devel@edk2.groups.io, Vitaly Cheptsov, Marvin Häuser, Gao, Liming, Liu, Zhiguang [-- Attachment #1: Type: text/plain, Size: 5890 bytes --] 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 <michael.d.kinney@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 < > pedro.falcato@gmail.com>; devel@edk2.groups.io > > Cc: Vitaly Cheptsov <vit9696@protonmail.com>; Marvin Häuser < > 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äuser > > > <mhaeuser@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 reads 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 > > > <vit9696@protonmail.com>; Marvin Häuser <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 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 <pedro.falcato@gmail.com> > > > > Cc: Vitaly Cheptsov <vit9696@protonmail.com> > > > > Cc: Marvin Häuser <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> > > > > --- > > > > 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 = FALSE; > > > > ASSERT (((UINTN)String & BIT0) == 0); > > > > > > > > // > > > > @@ -893,11 +896,12 @@ StrHexToUintnS ( > > > > // Ignore leading Zeros after the spaces > > > > // > > > > while (*String == L'0') { > > > > + FoundLeadingZero = TRUE; > > > > String++; > > > > } > > > > > > > > if (CharToUpper (*String) == L'X') { > > > > - if (*(String - 1) != L'0') { > > > > + if (!FoundLeadingZero) { > > > > *Data = 0; > > > > return RETURN_SUCCESS; > > > > } > > > > @@ -992,6 +996,9 @@ StrHexToUint64S ( > > > > OUT UINT64 *Data > > > > ) > > > > { > > > > + BOOLEAN FoundLeadingZero; > > > > + > > > > + FoundLeadingZero = FALSE; > > > > ASSERT (((UINTN)String & BIT0) == 0); > > > > > > > > // > > > > @@ -1022,11 +1029,12 @@ StrHexToUint64S ( > > > > // Ignore leading Zeros after the spaces > > > > // > > > > while (*String == L'0') { > > > > + FoundLeadingZero = TRUE; > > > > String++; > > > > } > > > > > > > > if (CharToUpper (*String) == L'X') { > > > > - if (*(String - 1) != L'0') { > > > > + if (!FoundLeadingZero) { > > > > *Data = 0; > > > > return RETURN_SUCCESS; > > > > } > > > > @@ -2393,6 +2401,9 @@ AsciiStrHexToUintnS ( > > > > OUT UINTN *Data > > > > ) > > > > { > > > > + BOOLEAN FoundLeadingZero; > > > > + > > > > + FoundLeadingZero = FALSE; > > > > // > > > > // 1. Neither String nor Data shall be a null pointer. > > > > // > > > > @@ -2421,11 +2432,12 @@ AsciiStrHexToUintnS ( > > > > // Ignore leading Zeros after the spaces > > > > // > > > > while (*String == '0') { > > > > + FoundLeadingZero = TRUE; > > > > String++; > > > > } > > > > > > > > if (AsciiCharToUpper (*String) == 'X') { > > > > - if (*(String - 1) != '0') { > > > > + if (!FoundLeadingZero) { > > > > *Data = 0; > > > > return RETURN_SUCCESS; > > > > } > > > > @@ -2517,6 +2529,9 @@ AsciiStrHexToUint64S ( > > > > OUT UINT64 *Data > > > > ) > > > > { > > > > + BOOLEAN FoundLeadingZero; > > > > + > > > > + FoundLeadingZero = FALSE; > > > > // > > > > // 1. Neither String nor Data shall be a null pointer. > > > > // > > > > @@ -2545,11 +2560,12 @@ AsciiStrHexToUint64S ( > > > > // Ignore leading Zeros after the spaces > > > > // > > > > while (*String == '0') { > > > > + FoundLeadingZero = TRUE; > > > > String++; > > > > } > > > > > > > > if (AsciiCharToUpper (*String) == 'X') { > > > > - if (*(String - 1) != '0') { > > > > + if (!FoundLeadingZero) { > > > > *Data = 0; > > > > return RETURN_SUCCESS; > > > > } > > > > -- > > > > 2.38.1 > > -- Pedro Falcato [-- Attachment #2: Type: text/html, Size: 10161 bytes --] ^ permalink raw reply [flat|nested] 8+ messages in thread
* 回复: [PATCH v2 1/1] MdePkg/BaseLib: Fix out-of-bounds reads in SafeString 2022-11-02 23:42 ` Pedro Falcato @ 2022-11-03 1:00 ` gaoliming 2022-11-03 1:13 ` Pedro Falcato 0 siblings, 1 reply; 8+ messages in thread From: gaoliming @ 2022-11-03 1:00 UTC (permalink / raw) To: 'Pedro Falcato', 'Kinney, Michael D' Cc: 'Yao, Jiewen', devel, 'Vitaly Cheptsov', 'Marvin Häuser', 'Liu, Zhiguang' [-- Attachment #1: Type: text/plain, Size: 7292 bytes --] Pedro: Marvin gave one suggestion for the code change (https://edk2.groups.io/g/devel/message/95635). Can you response it? Thanks Liming 发件人: Pedro Falcato <pedro.falcato@gmail.com> 发送时间: 2022年11月3日 7:42 收件人: Kinney, Michael D <michael.d.kinney@intel.com> 抄送: Yao, Jiewen <jiewen.yao@intel.com>; devel@edk2.groups.io; Vitaly Cheptsov <vit9696@protonmail.com>; Marvin Häuser <mhaeuser@posteo.de>; Gao, Liming <gaoliming@byosoft.com.cn>; Liu, Zhiguang <zhiguang.liu@intel.com> 主题: Re: [PATCH v2 1/1] MdePkg/BaseLib: Fix out-of-bounds reads in SafeString 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 <mailto:michael.d.kinney@intel.com> > wrote: Acked-by: Michael D Kinney <michael.d.kinney@intel.com <mailto:michael.d.kinney@intel.com> > > -----Original Message----- > From: Yao, Jiewen <jiewen.yao@intel.com <mailto:jiewen.yao@intel.com> > > Sent: Wednesday, October 26, 2022 6:35 AM > To: Kinney, Michael D <michael.d.kinney@intel.com <mailto:michael.d.kinney@intel.com> >; Pedro Falcato <pedro.falcato@gmail.com <mailto:pedro.falcato@gmail.com> >; devel@edk2.groups.io <mailto:devel@edk2.groups.io> > Cc: Vitaly Cheptsov <vit9696@protonmail.com <mailto:vit9696@protonmail.com> >; Marvin Häuser <mhaeuser@posteo.de <mailto:mhaeuser@posteo.de> >; Gao, Liming <gaoliming@byosoft.com.cn <mailto:gaoliming@byosoft.com.cn> >; Liu, > Zhiguang <zhiguang.liu@intel.com <mailto: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 <mailto:Jiewen.yao@Intel.com> > > > > > -----Original Message----- > > From: Kinney, Michael D <michael.d.kinney@intel.com <mailto:michael.d.kinney@intel.com> > > > Sent: Wednesday, October 26, 2022 12:23 AM > > To: Pedro Falcato <pedro.falcato@gmail.com <mailto:pedro.falcato@gmail.com> >; devel@edk2.groups.io <mailto:devel@edk2.groups.io> > > Cc: Vitaly Cheptsov <vit9696@protonmail.com <mailto:vit9696@protonmail.com> >; Marvin Häuser > > <mhaeuser@posteo.de <mailto:mhaeuser@posteo.de> >; Gao, Liming <gaoliming@byosoft.com.cn <mailto:gaoliming@byosoft.com.cn> >; Liu, > > Zhiguang <zhiguang.liu@intel.com <mailto:zhiguang.liu@intel.com> >; Yao, Jiewen <jiewen.yao@intel.com <mailto:jiewen.yao@intel.com> > > > Subject: RE: [PATCH v2 1/1] MdePkg/BaseLib: Fix out-of-bounds reads in > > SafeString > > > > Adding Jiewen Yao. > > > > Mike > > > > > -----Original Message----- > > > From: Pedro Falcato <pedro.falcato@gmail.com <mailto:pedro.falcato@gmail.com> > > > > Sent: Monday, October 24, 2022 3:43 PM > > > To: devel@edk2.groups.io <mailto:devel@edk2.groups.io> > > > Cc: Pedro Falcato <pedro.falcato@gmail.com <mailto:pedro.falcato@gmail.com> >; Vitaly Cheptsov > > <vit9696@protonmail.com <mailto:vit9696@protonmail.com> >; Marvin Häuser <mhaeuser@posteo.de <mailto:mhaeuser@posteo.de> >; > > > Kinney, Michael D <michael.d.kinney@intel.com <mailto:michael.d.kinney@intel.com> >; Gao, Liming > > <gaoliming@byosoft.com.cn <mailto:gaoliming@byosoft.com.cn> >; Liu, Zhiguang <zhiguang.liu@intel.com <mailto:zhiguang.liu@intel.com> > > > > 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 <pedro.falcato@gmail.com <mailto:pedro.falcato@gmail.com> > > > > Cc: Vitaly Cheptsov <vit9696@protonmail.com <mailto:vit9696@protonmail.com> > > > > Cc: Marvin Häuser <mhaeuser@posteo.de <mailto:mhaeuser@posteo.de> > > > > Cc: Michael D Kinney <michael.d.kinney@intel.com <mailto:michael.d.kinney@intel.com> > > > > Cc: Liming Gao <gaoliming@byosoft.com.cn <mailto:gaoliming@byosoft.com.cn> > > > > Cc: Zhiguang Liu <zhiguang.liu@intel.com <mailto:zhiguang.liu@intel.com> > > > > --- > > > 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 = FALSE; > > > ASSERT (((UINTN)String & BIT0) == 0); > > > > > > // > > > @@ -893,11 +896,12 @@ StrHexToUintnS ( > > > // Ignore leading Zeros after the spaces > > > // > > > while (*String == L'0') { > > > + FoundLeadingZero = TRUE; > > > String++; > > > } > > > > > > if (CharToUpper (*String) == L'X') { > > > - if (*(String - 1) != L'0') { > > > + if (!FoundLeadingZero) { > > > *Data = 0; > > > return RETURN_SUCCESS; > > > } > > > @@ -992,6 +996,9 @@ StrHexToUint64S ( > > > OUT UINT64 *Data > > > ) > > > { > > > + BOOLEAN FoundLeadingZero; > > > + > > > + FoundLeadingZero = FALSE; > > > ASSERT (((UINTN)String & BIT0) == 0); > > > > > > // > > > @@ -1022,11 +1029,12 @@ StrHexToUint64S ( > > > // Ignore leading Zeros after the spaces > > > // > > > while (*String == L'0') { > > > + FoundLeadingZero = TRUE; > > > String++; > > > } > > > > > > if (CharToUpper (*String) == L'X') { > > > - if (*(String - 1) != L'0') { > > > + if (!FoundLeadingZero) { > > > *Data = 0; > > > return RETURN_SUCCESS; > > > } > > > @@ -2393,6 +2401,9 @@ AsciiStrHexToUintnS ( > > > OUT UINTN *Data > > > ) > > > { > > > + BOOLEAN FoundLeadingZero; > > > + > > > + FoundLeadingZero = FALSE; > > > // > > > // 1. Neither String nor Data shall be a null pointer. > > > // > > > @@ -2421,11 +2432,12 @@ AsciiStrHexToUintnS ( > > > // Ignore leading Zeros after the spaces > > > // > > > while (*String == '0') { > > > + FoundLeadingZero = TRUE; > > > String++; > > > } > > > > > > if (AsciiCharToUpper (*String) == 'X') { > > > - if (*(String - 1) != '0') { > > > + if (!FoundLeadingZero) { > > > *Data = 0; > > > return RETURN_SUCCESS; > > > } > > > @@ -2517,6 +2529,9 @@ AsciiStrHexToUint64S ( > > > OUT UINT64 *Data > > > ) > > > { > > > + BOOLEAN FoundLeadingZero; > > > + > > > + FoundLeadingZero = FALSE; > > > // > > > // 1. Neither String nor Data shall be a null pointer. > > > // > > > @@ -2545,11 +2560,12 @@ AsciiStrHexToUint64S ( > > > // Ignore leading Zeros after the spaces > > > // > > > while (*String == '0') { > > > + FoundLeadingZero = TRUE; > > > String++; > > > } > > > > > > if (AsciiCharToUpper (*String) == 'X') { > > > - if (*(String - 1) != '0') { > > > + if (!FoundLeadingZero) { > > > *Data = 0; > > > return RETURN_SUCCESS; > > > } > > > -- > > > 2.38.1 -- Pedro Falcato [-- Attachment #2: Type: text/html, Size: 14510 bytes --] ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2 1/1] MdePkg/BaseLib: Fix out-of-bounds reads in SafeString 2022-11-03 1:00 ` 回复: " gaoliming @ 2022-11-03 1:13 ` Pedro Falcato 0 siblings, 0 replies; 8+ messages in thread From: Pedro Falcato @ 2022-11-03 1:13 UTC (permalink / raw) To: gaoliming Cc: Kinney, Michael D, Yao, Jiewen, devel, Vitaly Cheptsov, Marvin Häuser, Liu, Zhiguang [-- Attachment #1: Type: text/plain, Size: 6749 bytes --] Hi Liming, I've just sent out the v3. Thanks, Pedro On Thu, Nov 3, 2022 at 1:01 AM gaoliming <gaoliming@byosoft.com.cn> wrote: > Pedro: > > Marvin gave one suggestion for the code change ( > https://edk2.groups.io/g/devel/message/95635). Can you response it? > > > > Thanks > > Liming > > *发件人:* Pedro Falcato <pedro.falcato@gmail.com> > *发送时间:* 2022年11月3日 7:42 > *收件人:* Kinney, Michael D <michael.d.kinney@intel.com> > *抄送:* Yao, Jiewen <jiewen.yao@intel.com>; devel@edk2.groups.io; Vitaly > Cheptsov <vit9696@protonmail.com>; Marvin Häuser <mhaeuser@posteo.de>; > Gao, Liming <gaoliming@byosoft.com.cn>; Liu, Zhiguang < > zhiguang.liu@intel.com> > *主题:* Re: [PATCH v2 1/1] MdePkg/BaseLib: Fix out-of-bounds reads in > SafeString > > > > 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 <michael.d.kinney@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 < > pedro.falcato@gmail.com>; devel@edk2.groups.io > > Cc: Vitaly Cheptsov <vit9696@protonmail.com>; Marvin Häuser < > 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äuser > > > <mhaeuser@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 reads 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 > > > <vit9696@protonmail.com>; Marvin Häuser <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 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 <pedro.falcato@gmail.com> > > > > Cc: Vitaly Cheptsov <vit9696@protonmail.com> > > > > Cc: Marvin Häuser <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> > > > > --- > > > > 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 = FALSE; > > > > ASSERT (((UINTN)String & BIT0) == 0); > > > > > > > > // > > > > @@ -893,11 +896,12 @@ StrHexToUintnS ( > > > > // Ignore leading Zeros after the spaces > > > > // > > > > while (*String == L'0') { > > > > + FoundLeadingZero = TRUE; > > > > String++; > > > > } > > > > > > > > if (CharToUpper (*String) == L'X') { > > > > - if (*(String - 1) != L'0') { > > > > + if (!FoundLeadingZero) { > > > > *Data = 0; > > > > return RETURN_SUCCESS; > > > > } > > > > @@ -992,6 +996,9 @@ StrHexToUint64S ( > > > > OUT UINT64 *Data > > > > ) > > > > { > > > > + BOOLEAN FoundLeadingZero; > > > > + > > > > + FoundLeadingZero = FALSE; > > > > ASSERT (((UINTN)String & BIT0) == 0); > > > > > > > > // > > > > @@ -1022,11 +1029,12 @@ StrHexToUint64S ( > > > > // Ignore leading Zeros after the spaces > > > > // > > > > while (*String == L'0') { > > > > + FoundLeadingZero = TRUE; > > > > String++; > > > > } > > > > > > > > if (CharToUpper (*String) == L'X') { > > > > - if (*(String - 1) != L'0') { > > > > + if (!FoundLeadingZero) { > > > > *Data = 0; > > > > return RETURN_SUCCESS; > > > > } > > > > @@ -2393,6 +2401,9 @@ AsciiStrHexToUintnS ( > > > > OUT UINTN *Data > > > > ) > > > > { > > > > + BOOLEAN FoundLeadingZero; > > > > + > > > > + FoundLeadingZero = FALSE; > > > > // > > > > // 1. Neither String nor Data shall be a null pointer. > > > > // > > > > @@ -2421,11 +2432,12 @@ AsciiStrHexToUintnS ( > > > > // Ignore leading Zeros after the spaces > > > > // > > > > while (*String == '0') { > > > > + FoundLeadingZero = TRUE; > > > > String++; > > > > } > > > > > > > > if (AsciiCharToUpper (*String) == 'X') { > > > > - if (*(String - 1) != '0') { > > > > + if (!FoundLeadingZero) { > > > > *Data = 0; > > > > return RETURN_SUCCESS; > > > > } > > > > @@ -2517,6 +2529,9 @@ AsciiStrHexToUint64S ( > > > > OUT UINT64 *Data > > > > ) > > > > { > > > > + BOOLEAN FoundLeadingZero; > > > > + > > > > + FoundLeadingZero = FALSE; > > > > // > > > > // 1. Neither String nor Data shall be a null pointer. > > > > // > > > > @@ -2545,11 +2560,12 @@ AsciiStrHexToUint64S ( > > > > // Ignore leading Zeros after the spaces > > > > // > > > > while (*String == '0') { > > > > + FoundLeadingZero = TRUE; > > > > String++; > > > > } > > > > > > > > if (AsciiCharToUpper (*String) == 'X') { > > > > - if (*(String - 1) != '0') { > > > > + if (!FoundLeadingZero) { > > > > *Data = 0; > > > > return RETURN_SUCCESS; > > > > } > > > > -- > > > > 2.38.1 > > > > -- > > Pedro Falcato > -- Pedro Falcato [-- Attachment #2: Type: text/html, Size: 13869 bytes --] ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2022-11-03 1:14 UTC | newest] Thread overview: 8+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2022-10-24 22:43 [PATCH v2 1/1] MdePkg/BaseLib: Fix out-of-bounds reads in SafeString Pedro Falcato 2022-10-25 16:22 ` Michael D Kinney 2022-10-26 13:34 ` Yao, Jiewen 2022-10-26 13:41 ` Marvin Häuser 2022-10-26 15:54 ` Michael D Kinney 2022-11-02 23:42 ` Pedro Falcato 2022-11-03 1:00 ` 回复: " gaoliming 2022-11-03 1:13 ` Pedro Falcato
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox