* [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