* [PATCH v3 1/1] MdePkg/BaseLib: Fix out-of-bounds reads in SafeString
@ 2022-11-03 1:11 Pedro Falcato
2022-11-04 1:22 ` 回复: " gaoliming
0 siblings, 1 reply; 7+ messages in thread
From: Pedro Falcato @ 2022-11-03 1:11 UTC (permalink / raw)
To: devel
Cc: Pedro Falcato, Vitaly Cheptsov, Marvin Häuser,
Michael D Kinney, Liming Gao, Zhiguang Liu, Jiewen Yao
There was a OOB access in *StrHexTo* functions, when passed strings like
"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ä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>
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>
---
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 = FALSE;
ASSERT (((UINTN)String & BIT0) == 0);
//
@@ -892,12 +895,14 @@ StrHexToUintnS (
//
// Ignore leading Zeros after the spaces
//
+
+ FoundLeadingZero = *String == L'0';
while (*String == L'0') {
String++;
}
if (CharToUpper (*String) == L'X') {
- if (*(String - 1) != L'0') {
+ if (!FoundLeadingZero) {
*Data = 0;
return RETURN_SUCCESS;
}
@@ -992,6 +997,9 @@ StrHexToUint64S (
OUT UINT64 *Data
)
{
+ BOOLEAN FoundLeadingZero;
+
+ FoundLeadingZero = FALSE;
ASSERT (((UINTN)String & BIT0) == 0);
//
@@ -1021,12 +1029,13 @@ StrHexToUint64S (
//
// Ignore leading Zeros after the spaces
//
+ FoundLeadingZero = *String == L'0';
while (*String == L'0') {
String++;
}
if (CharToUpper (*String) == L'X') {
- if (*(String - 1) != L'0') {
+ if (!FoundLeadingZero) {
*Data = 0;
return RETURN_SUCCESS;
}
@@ -2393,6 +2402,9 @@ AsciiStrHexToUintnS (
OUT UINTN *Data
)
{
+ BOOLEAN FoundLeadingZero;
+
+ FoundLeadingZero = FALSE;
//
// 1. Neither String nor Data shall be a null pointer.
//
@@ -2420,12 +2432,13 @@ AsciiStrHexToUintnS (
//
// Ignore leading Zeros after the spaces
//
+ FoundLeadingZero = *String == '0';
while (*String == '0') {
String++;
}
if (AsciiCharToUpper (*String) == 'X') {
- if (*(String - 1) != '0') {
+ if (!FoundLeadingZero) {
*Data = 0;
return RETURN_SUCCESS;
}
@@ -2517,6 +2530,9 @@ AsciiStrHexToUint64S (
OUT UINT64 *Data
)
{
+ BOOLEAN FoundLeadingZero;
+
+ FoundLeadingZero = FALSE;
//
// 1. Neither String nor Data shall be a null pointer.
//
@@ -2544,12 +2560,13 @@ AsciiStrHexToUint64S (
//
// Ignore leading Zeros after the spaces
//
+ FoundLeadingZero = *String == '0';
while (*String == '0') {
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] 7+ messages in thread
* 回复: [PATCH v3 1/1] MdePkg/BaseLib: Fix out-of-bounds reads in SafeString
2022-11-03 1:11 [PATCH v3 1/1] MdePkg/BaseLib: Fix out-of-bounds reads in SafeString Pedro Falcato
@ 2022-11-04 1:22 ` gaoliming
2022-11-05 0:24 ` [edk2-devel] " Pedro Falcato
0 siblings, 1 reply; 7+ messages in thread
From: gaoliming @ 2022-11-04 1:22 UTC (permalink / raw)
To: 'Pedro Falcato', devel
Cc: 'Vitaly Cheptsov', 'Marvin Häuser',
'Michael D Kinney', 'Zhiguang Liu',
'Jiewen Yao'
Reviewed-by: Liming Gao <gaoliming@byosoft.com.cn>
> -----邮件原件-----
> 发件人: Pedro Falcato <pedro.falcato@gmail.com>
> 发送时间: 2022年11月3日 9:12
> 收件人: devel@edk2.groups.io
> 抄送: Pedro Falcato <pedro.falcato@gmail.com>; Vitaly Cheptsov
> <vit9696@protonmail.com>; Marvin Häuser <mhaeuser@posteo.de>;
> Michael D Kinney <michael.d.kinney@intel.com>; Liming Gao
> <gaoliming@byosoft.com.cn>; Zhiguang Liu <zhiguang.liu@intel.com>; Jiewen
> Yao <Jiewen.yao@Intel.com>
> 主题: [PATCH v3 1/1] MdePkg/BaseLib: Fix out-of-bounds reads in SafeString
>
> There was a OOB access in *StrHexTo* functions, when passed strings like
> "XDEADBEEF".
>
> 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ä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>
> 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>
> ---
> 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 = FALSE;
> ASSERT (((UINTN)String & BIT0) == 0);
>
> //
> @@ -892,12 +895,14 @@ StrHexToUintnS (
> //
> // Ignore leading Zeros after the spaces
> //
> +
> + FoundLeadingZero = *String == L'0';
> while (*String == L'0') {
> String++;
> }
>
> if (CharToUpper (*String) == L'X') {
> - if (*(String - 1) != L'0') {
> + if (!FoundLeadingZero) {
> *Data = 0;
> return RETURN_SUCCESS;
> }
> @@ -992,6 +997,9 @@ StrHexToUint64S (
> OUT UINT64 *Data
> )
> {
> + BOOLEAN FoundLeadingZero;
> +
> + FoundLeadingZero = FALSE;
> ASSERT (((UINTN)String & BIT0) == 0);
>
> //
> @@ -1021,12 +1029,13 @@ StrHexToUint64S (
> //
> // Ignore leading Zeros after the spaces
> //
> + FoundLeadingZero = *String == L'0';
> while (*String == L'0') {
> String++;
> }
>
> if (CharToUpper (*String) == L'X') {
> - if (*(String - 1) != L'0') {
> + if (!FoundLeadingZero) {
> *Data = 0;
> return RETURN_SUCCESS;
> }
> @@ -2393,6 +2402,9 @@ AsciiStrHexToUintnS (
> OUT UINTN *Data
> )
> {
> + BOOLEAN FoundLeadingZero;
> +
> + FoundLeadingZero = FALSE;
> //
> // 1. Neither String nor Data shall be a null pointer.
> //
> @@ -2420,12 +2432,13 @@ AsciiStrHexToUintnS (
> //
> // Ignore leading Zeros after the spaces
> //
> + FoundLeadingZero = *String == '0';
> while (*String == '0') {
> String++;
> }
>
> if (AsciiCharToUpper (*String) == 'X') {
> - if (*(String - 1) != '0') {
> + if (!FoundLeadingZero) {
> *Data = 0;
> return RETURN_SUCCESS;
> }
> @@ -2517,6 +2530,9 @@ AsciiStrHexToUint64S (
> OUT UINT64 *Data
> )
> {
> + BOOLEAN FoundLeadingZero;
> +
> + FoundLeadingZero = FALSE;
> //
> // 1. Neither String nor Data shall be a null pointer.
> //
> @@ -2544,12 +2560,13 @@ AsciiStrHexToUint64S (
> //
> // Ignore leading Zeros after the spaces
> //
> + FoundLeadingZero = *String == '0';
> while (*String == '0') {
> String++;
> }
>
> if (AsciiCharToUpper (*String) == 'X') {
> - if (*(String - 1) != '0') {
> + if (!FoundLeadingZero) {
> *Data = 0;
> return RETURN_SUCCESS;
> }
> --
> 2.38.1
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [edk2-devel] 回复: [PATCH v3 1/1] MdePkg/BaseLib: Fix out-of-bounds reads in SafeString
2022-11-04 1:22 ` 回复: " gaoliming
@ 2022-11-05 0:24 ` Pedro Falcato
2022-11-07 1:32 ` 回复: " gaoliming
0 siblings, 1 reply; 7+ messages in thread
From: Pedro Falcato @ 2022-11-05 0:24 UTC (permalink / raw)
To: devel, gaoliming
Cc: Vitaly Cheptsov, Marvin Häuser, Michael D Kinney,
Zhiguang Liu, Jiewen Yao
[-- Attachment #1: Type: text/plain, Size: 4621 bytes --]
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=
byosoft.com.cn@groups.io> wrote:
> Reviewed-by: Liming Gao <gaoliming@byosoft.com.cn>
>
> > -----邮件原件-----
> > 发件人: Pedro Falcato <pedro.falcato@gmail.com>
> > 发送时间: 2022年11月3日 9:12
> > 收件人: devel@edk2.groups.io
> > 抄送: Pedro Falcato <pedro.falcato@gmail.com>; Vitaly Cheptsov
> > <vit9696@protonmail.com>; Marvin Häuser <mhaeuser@posteo.de>;
> > Michael D Kinney <michael.d.kinney@intel.com>; Liming Gao
> > <gaoliming@byosoft.com.cn>; Zhiguang Liu <zhiguang.liu@intel.com>;
> Jiewen
> > Yao <Jiewen.yao@Intel.com>
> > 主题: [PATCH v3 1/1] MdePkg/BaseLib: Fix out-of-bounds reads in SafeString
> >
> > There was a OOB access in *StrHexTo* functions, when passed strings like
> > "XDEADBEEF".
> >
> > 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ä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>
> > 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>
> > ---
> > 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 = FALSE;
> > ASSERT (((UINTN)String & BIT0) == 0);
> >
> > //
> > @@ -892,12 +895,14 @@ StrHexToUintnS (
> > //
> > // Ignore leading Zeros after the spaces
> > //
> > +
> > + FoundLeadingZero = *String == L'0';
> > while (*String == L'0') {
> > String++;
> > }
> >
> > if (CharToUpper (*String) == L'X') {
> > - if (*(String - 1) != L'0') {
> > + if (!FoundLeadingZero) {
> > *Data = 0;
> > return RETURN_SUCCESS;
> > }
> > @@ -992,6 +997,9 @@ StrHexToUint64S (
> > OUT UINT64 *Data
> > )
> > {
> > + BOOLEAN FoundLeadingZero;
> > +
> > + FoundLeadingZero = FALSE;
> > ASSERT (((UINTN)String & BIT0) == 0);
> >
> > //
> > @@ -1021,12 +1029,13 @@ StrHexToUint64S (
> > //
> > // Ignore leading Zeros after the spaces
> > //
> > + FoundLeadingZero = *String == L'0';
> > while (*String == L'0') {
> > String++;
> > }
> >
> > if (CharToUpper (*String) == L'X') {
> > - if (*(String - 1) != L'0') {
> > + if (!FoundLeadingZero) {
> > *Data = 0;
> > return RETURN_SUCCESS;
> > }
> > @@ -2393,6 +2402,9 @@ AsciiStrHexToUintnS (
> > OUT UINTN *Data
> > )
> > {
> > + BOOLEAN FoundLeadingZero;
> > +
> > + FoundLeadingZero = FALSE;
> > //
> > // 1. Neither String nor Data shall be a null pointer.
> > //
> > @@ -2420,12 +2432,13 @@ AsciiStrHexToUintnS (
> > //
> > // Ignore leading Zeros after the spaces
> > //
> > + FoundLeadingZero = *String == '0';
> > while (*String == '0') {
> > String++;
> > }
> >
> > if (AsciiCharToUpper (*String) == 'X') {
> > - if (*(String - 1) != '0') {
> > + if (!FoundLeadingZero) {
> > *Data = 0;
> > return RETURN_SUCCESS;
> > }
> > @@ -2517,6 +2530,9 @@ AsciiStrHexToUint64S (
> > OUT UINT64 *Data
> > )
> > {
> > + BOOLEAN FoundLeadingZero;
> > +
> > + FoundLeadingZero = FALSE;
> > //
> > // 1. Neither String nor Data shall be a null pointer.
> > //
> > @@ -2544,12 +2560,13 @@ AsciiStrHexToUint64S (
> > //
> > // Ignore leading Zeros after the spaces
> > //
> > + FoundLeadingZero = *String == '0';
> > while (*String == '0') {
> > 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: 7216 bytes --]
^ permalink raw reply [flat|nested] 7+ messages in thread
* 回复: [edk2-devel] 回复: [PATCH v3 1/1] MdePkg/BaseLib: Fix out-of-bounds reads in SafeString
2022-11-05 0:24 ` [edk2-devel] " Pedro Falcato
@ 2022-11-07 1:32 ` gaoliming
2022-11-07 8:14 ` Pedro Falcato
0 siblings, 1 reply; 7+ messages in thread
From: gaoliming @ 2022-11-07 1:32 UTC (permalink / raw)
To: devel, pedro.falcato
Cc: 'Vitaly Cheptsov', 'Marvin Häuser',
'Michael D Kinney', 'Zhiguang Liu',
'Jiewen Yao'
[-- Attachment #1: Type: text/plain, Size: 5612 bytes --]
Create https://github.com/tianocore/edk2/pull/3604 to merge this patch.
Thanks
Liming
发件人: devel@edk2.groups.io <devel@edk2.groups.io> 代表 Pedro Falcato
发送时间: 2022年11月5日 8:25
收件人: devel@edk2.groups.io; gaoliming@byosoft.com.cn
抄送: Vitaly Cheptsov <vit9696@protonmail.com>; Marvin Häuser <mhaeuser@posteo.de>; Michael D Kinney <michael.d.kinney@intel.com>; Zhiguang Liu <zhiguang.liu@intel.com>; Jiewen Yao <Jiewen.yao@intel.com>
主题: Re: [edk2-devel] 回复: [PATCH v3 1/1] MdePkg/BaseLib: Fix out-of-bounds reads in SafeString
Hi Liming,
Thank you for the review. Can we please push this in time for the stable tag?
Thanks,
Pedro
On Fri, Nov 4, 2022 at 1:22 AM gaoliming via groups.io <http://groups.io> <gaoliming=byosoft.com.cn@groups.io <mailto:byosoft.com.cn@groups.io> > wrote:
Reviewed-by: Liming Gao <gaoliming@byosoft.com.cn <mailto:gaoliming@byosoft.com.cn> >
> -----邮件原件-----
> 发件人: Pedro Falcato <pedro.falcato@gmail.com <mailto:pedro.falcato@gmail.com> >
> 发送时间: 2022年11月3日 9:12
> 收件人: devel@edk2.groups.io <mailto:devel@edk2.groups.io>
> 抄送: 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> >;
> Michael D Kinney <michael.d.kinney@intel.com <mailto:michael.d.kinney@intel.com> >; Liming Gao
> <gaoliming@byosoft.com.cn <mailto:gaoliming@byosoft.com.cn> >; Zhiguang Liu <zhiguang.liu@intel.com <mailto:zhiguang.liu@intel.com> >; Jiewen
> Yao <Jiewen.yao@Intel.com <mailto:Jiewen.yao@Intel.com> >
> 主题: [PATCH v3 1/1] MdePkg/BaseLib: Fix out-of-bounds reads in SafeString
>
> There was a OOB access in *StrHexTo* functions, when passed strings like
> "XDEADBEEF".
>
> 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 <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> >
> Signed-off-by: Pedro Falcato <pedro.falcato@gmail.com <mailto:pedro.falcato@gmail.com> >
> Acked-by: Michael D Kinney <michael.d.kinney@intel.com <mailto:michael.d.kinney@intel.com> >
> Reviewed-by: Jiewen Yao <Jiewen.yao@Intel.com <mailto:Jiewen.yao@Intel.com> >
> ---
> MdePkg/Library/BaseLib/SafeString.c | 25 +++++++++++++++++++++----
> 1 file changed, 21 insertions(+), 4 deletions(-)
>
> diff --git a/MdePkg/Library/BaseLib/SafeString.c
> b/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 = FALSE;
> ASSERT (((UINTN)String & BIT0) == 0);
>
> //
> @@ -892,12 +895,14 @@ StrHexToUintnS (
> //
> // Ignore leading Zeros after the spaces
> //
> +
> + FoundLeadingZero = *String == L'0';
> while (*String == L'0') {
> String++;
> }
>
> if (CharToUpper (*String) == L'X') {
> - if (*(String - 1) != L'0') {
> + if (!FoundLeadingZero) {
> *Data = 0;
> return RETURN_SUCCESS;
> }
> @@ -992,6 +997,9 @@ StrHexToUint64S (
> OUT UINT64 *Data
> )
> {
> + BOOLEAN FoundLeadingZero;
> +
> + FoundLeadingZero = FALSE;
> ASSERT (((UINTN)String & BIT0) == 0);
>
> //
> @@ -1021,12 +1029,13 @@ StrHexToUint64S (
> //
> // Ignore leading Zeros after the spaces
> //
> + FoundLeadingZero = *String == L'0';
> while (*String == L'0') {
> String++;
> }
>
> if (CharToUpper (*String) == L'X') {
> - if (*(String - 1) != L'0') {
> + if (!FoundLeadingZero) {
> *Data = 0;
> return RETURN_SUCCESS;
> }
> @@ -2393,6 +2402,9 @@ AsciiStrHexToUintnS (
> OUT UINTN *Data
> )
> {
> + BOOLEAN FoundLeadingZero;
> +
> + FoundLeadingZero = FALSE;
> //
> // 1. Neither String nor Data shall be a null pointer.
> //
> @@ -2420,12 +2432,13 @@ AsciiStrHexToUintnS (
> //
> // Ignore leading Zeros after the spaces
> //
> + FoundLeadingZero = *String == '0';
> while (*String == '0') {
> String++;
> }
>
> if (AsciiCharToUpper (*String) == 'X') {
> - if (*(String - 1) != '0') {
> + if (!FoundLeadingZero) {
> *Data = 0;
> return RETURN_SUCCESS;
> }
> @@ -2517,6 +2530,9 @@ AsciiStrHexToUint64S (
> OUT UINT64 *Data
> )
> {
> + BOOLEAN FoundLeadingZero;
> +
> + FoundLeadingZero = FALSE;
> //
> // 1. Neither String nor Data shall be a null pointer.
> //
> @@ -2544,12 +2560,13 @@ AsciiStrHexToUint64S (
> //
> // Ignore leading Zeros after the spaces
> //
> + FoundLeadingZero = *String == '0';
> while (*String == '0') {
> 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: 12442 bytes --]
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [edk2-devel] 回复: [PATCH v3 1/1] MdePkg/BaseLib: Fix out-of-bounds reads in SafeString
2022-11-07 1:32 ` 回复: " gaoliming
@ 2022-11-07 8:14 ` Pedro Falcato
2022-11-07 8:57 ` JC
2022-11-07 9:11 ` JC
0 siblings, 2 replies; 7+ messages in thread
From: Pedro Falcato @ 2022-11-07 8:14 UTC (permalink / raw)
To: edk2-devel-groups-io, Liming Gao
Cc: Vitaly Cheptsov, Marvin Häuser, Michael D Kinney,
Zhiguang Liu, Jiewen Yao
[-- Attachment #1: Type: text/plain, Size: 5419 bytes --]
Thanks!
On Mon, 7 Nov 2022, 01:32 gaoliming via groups.io, <gaoliming=
byosoft.com.cn@groups.io> wrote:
> Create https://github.com/tianocore/edk2/pull/3604 to merge this patch.
>
>
>
> Thanks
>
> Liming
>
> *发件人:* devel@edk2.groups.io <devel@edk2.groups.io> *代表 *Pedro Falcato
> *发送时间:* 2022年11月5日 8:25
> *收件人:* devel@edk2.groups.io; gaoliming@byosoft.com.cn
> *抄送:* Vitaly Cheptsov <vit9696@protonmail.com>; Marvin Häuser <
> mhaeuser@posteo.de>; Michael D Kinney <michael.d.kinney@intel.com>;
> Zhiguang Liu <zhiguang.liu@intel.com>; Jiewen Yao <Jiewen.yao@intel.com>
> *主题:* Re: [edk2-devel] 回复: [PATCH v3 1/1] MdePkg/BaseLib: Fix
> out-of-bounds reads in SafeString
>
>
>
> Hi Liming,
>
>
>
> Thank you for the review. Can we please push this in time for the stable
> tag?
>
>
>
> Thanks,
>
> Pedro
>
>
>
> On Fri, Nov 4, 2022 at 1:22 AM gaoliming via groups.io <gaoliming=
> byosoft.com.cn@groups.io> wrote:
>
> Reviewed-by: Liming Gao <gaoliming@byosoft.com.cn>
>
> > -----邮件原件-----
> > 发件人: Pedro Falcato <pedro.falcato@gmail.com>
> > 发送时间: 2022年11月3日 9:12
> > 收件人: devel@edk2.groups.io
> > 抄送: Pedro Falcato <pedro.falcato@gmail.com>; Vitaly Cheptsov
> > <vit9696@protonmail.com>; Marvin Häuser <mhaeuser@posteo.de>;
> > Michael D Kinney <michael.d.kinney@intel.com>; Liming Gao
> > <gaoliming@byosoft.com.cn>; Zhiguang Liu <zhiguang.liu@intel.com>;
> Jiewen
> > Yao <Jiewen.yao@Intel.com>
> > 主题: [PATCH v3 1/1] MdePkg/BaseLib: Fix out-of-bounds reads in SafeString
> >
> > There was a OOB access in *StrHexTo* functions, when passed strings like
> > "XDEADBEEF".
> >
> > 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ä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>
> > 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>
> > ---
> > 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 = FALSE;
> > ASSERT (((UINTN)String & BIT0) == 0);
> >
> > //
> > @@ -892,12 +895,14 @@ StrHexToUintnS (
> > //
> > // Ignore leading Zeros after the spaces
> > //
> > +
> > + FoundLeadingZero = *String == L'0';
> > while (*String == L'0') {
> > String++;
> > }
> >
> > if (CharToUpper (*String) == L'X') {
> > - if (*(String - 1) != L'0') {
> > + if (!FoundLeadingZero) {
> > *Data = 0;
> > return RETURN_SUCCESS;
> > }
> > @@ -992,6 +997,9 @@ StrHexToUint64S (
> > OUT UINT64 *Data
> > )
> > {
> > + BOOLEAN FoundLeadingZero;
> > +
> > + FoundLeadingZero = FALSE;
> > ASSERT (((UINTN)String & BIT0) == 0);
> >
> > //
> > @@ -1021,12 +1029,13 @@ StrHexToUint64S (
> > //
> > // Ignore leading Zeros after the spaces
> > //
> > + FoundLeadingZero = *String == L'0';
> > while (*String == L'0') {
> > String++;
> > }
> >
> > if (CharToUpper (*String) == L'X') {
> > - if (*(String - 1) != L'0') {
> > + if (!FoundLeadingZero) {
> > *Data = 0;
> > return RETURN_SUCCESS;
> > }
> > @@ -2393,6 +2402,9 @@ AsciiStrHexToUintnS (
> > OUT UINTN *Data
> > )
> > {
> > + BOOLEAN FoundLeadingZero;
> > +
> > + FoundLeadingZero = FALSE;
> > //
> > // 1. Neither String nor Data shall be a null pointer.
> > //
> > @@ -2420,12 +2432,13 @@ AsciiStrHexToUintnS (
> > //
> > // Ignore leading Zeros after the spaces
> > //
> > + FoundLeadingZero = *String == '0';
> > while (*String == '0') {
> > String++;
> > }
> >
> > if (AsciiCharToUpper (*String) == 'X') {
> > - if (*(String - 1) != '0') {
> > + if (!FoundLeadingZero) {
> > *Data = 0;
> > return RETURN_SUCCESS;
> > }
> > @@ -2517,6 +2530,9 @@ AsciiStrHexToUint64S (
> > OUT UINT64 *Data
> > )
> > {
> > + BOOLEAN FoundLeadingZero;
> > +
> > + FoundLeadingZero = FALSE;
> > //
> > // 1. Neither String nor Data shall be a null pointer.
> > //
> > @@ -2544,12 +2560,13 @@ AsciiStrHexToUint64S (
> > //
> > // Ignore leading Zeros after the spaces
> > //
> > + FoundLeadingZero = *String == '0';
> > while (*String == '0') {
> > 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: 11772 bytes --]
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [edk2-devel] 回复: [PATCH v3 1/1] MdePkg/BaseLib: Fix out-of-bounds reads in SafeString
2022-11-07 8:14 ` Pedro Falcato
@ 2022-11-07 8:57 ` JC
2022-11-07 9:11 ` JC
1 sibling, 0 replies; 7+ messages in thread
From: JC @ 2022-11-07 8:57 UTC (permalink / raw)
To: devel, pedro.falcato
[-- Attachment #1: Type: text/plain, Size: 5720 bytes --]
test mail
On Mon, Nov 7, 2022 at 9:14 AM Pedro Falcato <pedro.falcato@gmail.com>
wrote:
> Thanks!
>
> On Mon, 7 Nov 2022, 01:32 gaoliming via groups.io, <gaoliming=
> byosoft.com.cn@groups.io> wrote:
>
>> Create https://github.com/tianocore/edk2/pull/3604 to merge this patch.
>>
>>
>>
>> Thanks
>>
>> Liming
>>
>> *发件人:* devel@edk2.groups.io <devel@edk2.groups.io> *代表 *Pedro Falcato
>> *发送时间:* 2022年11月5日 8:25
>> *收件人:* devel@edk2.groups.io; gaoliming@byosoft.com.cn
>> *抄送:* Vitaly Cheptsov <vit9696@protonmail.com>; Marvin Häuser <
>> mhaeuser@posteo.de>; Michael D Kinney <michael.d.kinney@intel.com>;
>> Zhiguang Liu <zhiguang.liu@intel.com>; Jiewen Yao <Jiewen.yao@intel.com>
>> *主题:* Re: [edk2-devel] 回复: [PATCH v3 1/1] MdePkg/BaseLib: Fix
>> out-of-bounds reads in SafeString
>>
>>
>>
>> Hi Liming,
>>
>>
>>
>> Thank you for the review. Can we please push this in time for the stable
>> tag?
>>
>>
>>
>> Thanks,
>>
>> Pedro
>>
>>
>>
>> On Fri, Nov 4, 2022 at 1:22 AM gaoliming via groups.io <gaoliming=
>> byosoft.com.cn@groups.io> wrote:
>>
>> Reviewed-by: Liming Gao <gaoliming@byosoft.com.cn>
>>
>> > -----邮件原件-----
>> > 发件人: Pedro Falcato <pedro.falcato@gmail.com>
>> > 发送时间: 2022年11月3日 9:12
>> > 收件人: devel@edk2.groups.io
>> > 抄送: Pedro Falcato <pedro.falcato@gmail.com>; Vitaly Cheptsov
>> > <vit9696@protonmail.com>; Marvin Häuser <mhaeuser@posteo.de>;
>> > Michael D Kinney <michael.d.kinney@intel.com>; Liming Gao
>> > <gaoliming@byosoft.com.cn>; Zhiguang Liu <zhiguang.liu@intel.com>;
>> Jiewen
>> > Yao <Jiewen.yao@Intel.com>
>> > 主题: [PATCH v3 1/1] MdePkg/BaseLib: Fix out-of-bounds reads in
>> SafeString
>> >
>> > There was a OOB access in *StrHexTo* functions, when passed strings like
>> > "XDEADBEEF".
>> >
>> > 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ä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>
>> > 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>
>> > ---
>> > 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 = FALSE;
>> > ASSERT (((UINTN)String & BIT0) == 0);
>> >
>> > //
>> > @@ -892,12 +895,14 @@ StrHexToUintnS (
>> > //
>> > // Ignore leading Zeros after the spaces
>> > //
>> > +
>> > + FoundLeadingZero = *String == L'0';
>> > while (*String == L'0') {
>> > String++;
>> > }
>> >
>> > if (CharToUpper (*String) == L'X') {
>> > - if (*(String - 1) != L'0') {
>> > + if (!FoundLeadingZero) {
>> > *Data = 0;
>> > return RETURN_SUCCESS;
>> > }
>> > @@ -992,6 +997,9 @@ StrHexToUint64S (
>> > OUT UINT64 *Data
>> > )
>> > {
>> > + BOOLEAN FoundLeadingZero;
>> > +
>> > + FoundLeadingZero = FALSE;
>> > ASSERT (((UINTN)String & BIT0) == 0);
>> >
>> > //
>> > @@ -1021,12 +1029,13 @@ StrHexToUint64S (
>> > //
>> > // Ignore leading Zeros after the spaces
>> > //
>> > + FoundLeadingZero = *String == L'0';
>> > while (*String == L'0') {
>> > String++;
>> > }
>> >
>> > if (CharToUpper (*String) == L'X') {
>> > - if (*(String - 1) != L'0') {
>> > + if (!FoundLeadingZero) {
>> > *Data = 0;
>> > return RETURN_SUCCESS;
>> > }
>> > @@ -2393,6 +2402,9 @@ AsciiStrHexToUintnS (
>> > OUT UINTN *Data
>> > )
>> > {
>> > + BOOLEAN FoundLeadingZero;
>> > +
>> > + FoundLeadingZero = FALSE;
>> > //
>> > // 1. Neither String nor Data shall be a null pointer.
>> > //
>> > @@ -2420,12 +2432,13 @@ AsciiStrHexToUintnS (
>> > //
>> > // Ignore leading Zeros after the spaces
>> > //
>> > + FoundLeadingZero = *String == '0';
>> > while (*String == '0') {
>> > String++;
>> > }
>> >
>> > if (AsciiCharToUpper (*String) == 'X') {
>> > - if (*(String - 1) != '0') {
>> > + if (!FoundLeadingZero) {
>> > *Data = 0;
>> > return RETURN_SUCCESS;
>> > }
>> > @@ -2517,6 +2530,9 @@ AsciiStrHexToUint64S (
>> > OUT UINT64 *Data
>> > )
>> > {
>> > + BOOLEAN FoundLeadingZero;
>> > +
>> > + FoundLeadingZero = FALSE;
>> > //
>> > // 1. Neither String nor Data shall be a null pointer.
>> > //
>> > @@ -2544,12 +2560,13 @@ AsciiStrHexToUint64S (
>> > //
>> > // Ignore leading Zeros after the spaces
>> > //
>> > + FoundLeadingZero = *String == '0';
>> > while (*String == '0') {
>> > 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: 12386 bytes --]
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [edk2-devel] 回复: [PATCH v3 1/1] MdePkg/BaseLib: Fix out-of-bounds reads in SafeString
2022-11-07 8:14 ` Pedro Falcato
2022-11-07 8:57 ` JC
@ 2022-11-07 9:11 ` JC
1 sibling, 0 replies; 7+ messages in thread
From: JC @ 2022-11-07 9:11 UTC (permalink / raw)
To: Pedro Falcato, devel
[-- Attachment #1: Type: text/plain, Size: 52 bytes --]
Hi Pedro, this is just a test email
Thanks,
JC
[-- Attachment #2: Type: text/html, Size: 64 bytes --]
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2022-11-07 9:11 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-11-03 1:11 [PATCH v3 1/1] MdePkg/BaseLib: Fix out-of-bounds reads in SafeString Pedro Falcato
2022-11-04 1:22 ` 回复: " gaoliming
2022-11-05 0:24 ` [edk2-devel] " Pedro Falcato
2022-11-07 1:32 ` 回复: " gaoliming
2022-11-07 8:14 ` Pedro Falcato
2022-11-07 8:57 ` JC
2022-11-07 9:11 ` JC
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox