* [patch 1/2] MdeModulePkg/String.c: Zero memory before free (CVE-2019-14558)
2020-02-13 4:03 [patch] MdeModulePkg/HiiDB: Remove configuration table when it's freed (CVE-2019-14586) Dandan Bi
@ 2020-02-13 4:03 ` Dandan Bi
2020-02-13 4:06 ` Wang, Jian J
2020-02-14 6:44 ` Dong, Eric
2020-02-13 4:03 ` [patch 2/2] MdeModulePkg/DisplayEngine: " Dandan Bi
2020-02-13 4:05 ` [patch] MdeModulePkg/HiiDB: Remove configuration table when it's freed (CVE-2019-14586) Wang, Jian J
2 siblings, 2 replies; 8+ messages in thread
From: Dandan Bi @ 2020-02-13 4:03 UTC (permalink / raw)
To: devel; +Cc: Liming Gao, Eric Dong, Jian J Wang
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1611
Cc: Liming Gao <liming.gao@intel.com>
Cc: Eric Dong <eric.dong@intel.com>
Cc: Jian J Wang <jian.j.wang@intel.com>
Signed-off-by: Dandan Bi <dandan.bi@intel.com>
---
MdeModulePkg/Universal/HiiDatabaseDxe/String.c | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/MdeModulePkg/Universal/HiiDatabaseDxe/String.c b/MdeModulePkg/Universal/HiiDatabaseDxe/String.c
index 505e063d49..10a1e691a3 100644
--- a/MdeModulePkg/Universal/HiiDatabaseDxe/String.c
+++ b/MdeModulePkg/Universal/HiiDatabaseDxe/String.c
@@ -1004,10 +1004,11 @@ SetStringWorker (
BlockPtr,
StringTextPtr + AsciiStrSize ((CHAR8 *)StringTextPtr),
TmpSize
);
+ ZeroMem (StringPackage->StringBlock, OldBlockSize);
FreePool (StringPackage->StringBlock);
StringPackage->StringBlock = Block;
StringPackage->StringPkgHdr->Header.Length += (UINT32) (BlockSize - OldBlockSize);
break;
@@ -1037,10 +1038,11 @@ SetStringWorker (
BlockPtr,
StringTextPtr + StringSize,
OldBlockSize - (StringTextPtr - StringPackage->StringBlock) - StringSize
);
+ ZeroMem (StringPackage->StringBlock, OldBlockSize);
FreePool (StringPackage->StringBlock);
StringPackage->StringBlock = Block;
StringPackage->StringPkgHdr->Header.Length += (UINT32) (BlockSize - OldBlockSize);
break;
@@ -1088,10 +1090,11 @@ SetStringWorker (
);
BlockPtr += StrSize (GlobalFont->FontInfo->FontName);
CopyMem (BlockPtr, StringPackage->StringBlock, OldBlockSize);
+ ZeroMem (StringPackage->StringBlock, OldBlockSize);
FreePool (StringPackage->StringBlock);
StringPackage->StringBlock = Block;
StringPackage->StringPkgHdr->Header.Length += Ext2.Length;
return EFI_SUCCESS;
@@ -1273,10 +1276,11 @@ HiiNewString (
//
// Append a EFI_HII_SIBT_END block to the end.
//
*BlockPtr = EFI_HII_SIBT_END;
+ ZeroMem (StringPackage->StringBlock, OldBlockSize);
FreePool (StringPackage->StringBlock);
StringPackage->StringBlock = StringBlock;
StringPackage->StringPkgHdr->Header.Length += Ucs2BlockSize;
PackageListNode->PackageListHdr.PackageLength += Ucs2BlockSize;
}
@@ -1404,10 +1408,11 @@ HiiNewString (
//
// Append a EFI_HII_SIBT_END block to the end.
//
*BlockPtr = EFI_HII_SIBT_END;
+ ZeroMem (StringPackage->StringBlock, OldBlockSize);
FreePool (StringPackage->StringBlock);
StringPackage->StringBlock = StringBlock;
StringPackage->StringPkgHdr->Header.Length += Ucs2BlockSize;
PackageListNode->PackageListHdr.PackageLength += Ucs2BlockSize;
@@ -1446,10 +1451,11 @@ HiiNewString (
//
// Append a EFI_HII_SIBT_END block to the end.
//
*BlockPtr = EFI_HII_SIBT_END;
+ ZeroMem (StringPackage->StringBlock, OldBlockSize);
FreePool (StringPackage->StringBlock);
StringPackage->StringBlock = StringBlock;
StringPackage->StringPkgHdr->Header.Length += Ucs2FontBlockSize;
PackageListNode->PackageListHdr.PackageLength += Ucs2FontBlockSize;
@@ -1507,10 +1513,11 @@ HiiNewString (
//
// Append a EFI_HII_SIBT_END block to the end.
//
*BlockPtr = EFI_HII_SIBT_END;
+ ZeroMem (StringPackage->StringBlock, OldBlockSize);
FreePool (StringPackage->StringBlock);
StringPackage->StringBlock = StringBlock;
StringPackage->StringPkgHdr->Header.Length += FontBlockSize + Ucs2FontBlockSize;
PackageListNode->PackageListHdr.PackageLength += FontBlockSize + Ucs2FontBlockSize;
--
2.18.0.windows.1
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [patch 1/2] MdeModulePkg/String.c: Zero memory before free (CVE-2019-14558)
2020-02-13 4:03 ` [patch 1/2] MdeModulePkg/String.c: Zero memory before free (CVE-2019-14558) Dandan Bi
@ 2020-02-13 4:06 ` Wang, Jian J
2020-02-14 6:44 ` Dong, Eric
1 sibling, 0 replies; 8+ messages in thread
From: Wang, Jian J @ 2020-02-13 4:06 UTC (permalink / raw)
To: Bi, Dandan, devel@edk2.groups.io; +Cc: Gao, Liming, Dong, Eric
Reviewed-by: Jian J Wang <jian.j.wang@intel.com>
Regards,
Jian
> -----Original Message-----
> From: Bi, Dandan <dandan.bi@intel.com>
> Sent: Thursday, February 13, 2020 12:03 PM
> To: devel@edk2.groups.io
> Cc: Gao, Liming <liming.gao@intel.com>; Dong, Eric <eric.dong@intel.com>;
> Wang, Jian J <jian.j.wang@intel.com>
> Subject: [patch 1/2] MdeModulePkg/String.c: Zero memory before free (CVE-
> 2019-14558)
>
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1611
>
> Cc: Liming Gao <liming.gao@intel.com>
> Cc: Eric Dong <eric.dong@intel.com>
> Cc: Jian J Wang <jian.j.wang@intel.com>
> Signed-off-by: Dandan Bi <dandan.bi@intel.com>
> ---
> MdeModulePkg/Universal/HiiDatabaseDxe/String.c | 7 +++++++
> 1 file changed, 7 insertions(+)
>
> diff --git a/MdeModulePkg/Universal/HiiDatabaseDxe/String.c
> b/MdeModulePkg/Universal/HiiDatabaseDxe/String.c
> index 505e063d49..10a1e691a3 100644
> --- a/MdeModulePkg/Universal/HiiDatabaseDxe/String.c
> +++ b/MdeModulePkg/Universal/HiiDatabaseDxe/String.c
> @@ -1004,10 +1004,11 @@ SetStringWorker (
> BlockPtr,
> StringTextPtr + AsciiStrSize ((CHAR8 *)StringTextPtr),
> TmpSize
> );
>
> + ZeroMem (StringPackage->StringBlock, OldBlockSize);
> FreePool (StringPackage->StringBlock);
> StringPackage->StringBlock = Block;
> StringPackage->StringPkgHdr->Header.Length += (UINT32) (BlockSize -
> OldBlockSize);
> break;
>
> @@ -1037,10 +1038,11 @@ SetStringWorker (
> BlockPtr,
> StringTextPtr + StringSize,
> OldBlockSize - (StringTextPtr - StringPackage->StringBlock) - StringSize
> );
>
> + ZeroMem (StringPackage->StringBlock, OldBlockSize);
> FreePool (StringPackage->StringBlock);
> StringPackage->StringBlock = Block;
> StringPackage->StringPkgHdr->Header.Length += (UINT32) (BlockSize -
> OldBlockSize);
> break;
>
> @@ -1088,10 +1090,11 @@ SetStringWorker (
> );
> BlockPtr += StrSize (GlobalFont->FontInfo->FontName);
>
> CopyMem (BlockPtr, StringPackage->StringBlock, OldBlockSize);
>
> + ZeroMem (StringPackage->StringBlock, OldBlockSize);
> FreePool (StringPackage->StringBlock);
> StringPackage->StringBlock = Block;
> StringPackage->StringPkgHdr->Header.Length += Ext2.Length;
>
> return EFI_SUCCESS;
> @@ -1273,10 +1276,11 @@ HiiNewString (
>
> //
> // Append a EFI_HII_SIBT_END block to the end.
> //
> *BlockPtr = EFI_HII_SIBT_END;
> + ZeroMem (StringPackage->StringBlock, OldBlockSize);
> FreePool (StringPackage->StringBlock);
> StringPackage->StringBlock = StringBlock;
> StringPackage->StringPkgHdr->Header.Length += Ucs2BlockSize;
> PackageListNode->PackageListHdr.PackageLength += Ucs2BlockSize;
> }
> @@ -1404,10 +1408,11 @@ HiiNewString (
>
> //
> // Append a EFI_HII_SIBT_END block to the end.
> //
> *BlockPtr = EFI_HII_SIBT_END;
> + ZeroMem (StringPackage->StringBlock, OldBlockSize);
> FreePool (StringPackage->StringBlock);
> StringPackage->StringBlock = StringBlock;
> StringPackage->StringPkgHdr->Header.Length += Ucs2BlockSize;
> PackageListNode->PackageListHdr.PackageLength += Ucs2BlockSize;
>
> @@ -1446,10 +1451,11 @@ HiiNewString (
>
> //
> // Append a EFI_HII_SIBT_END block to the end.
> //
> *BlockPtr = EFI_HII_SIBT_END;
> + ZeroMem (StringPackage->StringBlock, OldBlockSize);
> FreePool (StringPackage->StringBlock);
> StringPackage->StringBlock = StringBlock;
> StringPackage->StringPkgHdr->Header.Length += Ucs2FontBlockSize;
> PackageListNode->PackageListHdr.PackageLength += Ucs2FontBlockSize;
>
> @@ -1507,10 +1513,11 @@ HiiNewString (
>
> //
> // Append a EFI_HII_SIBT_END block to the end.
> //
> *BlockPtr = EFI_HII_SIBT_END;
> + ZeroMem (StringPackage->StringBlock, OldBlockSize);
> FreePool (StringPackage->StringBlock);
> StringPackage->StringBlock = StringBlock;
> StringPackage->StringPkgHdr->Header.Length += FontBlockSize +
> Ucs2FontBlockSize;
> PackageListNode->PackageListHdr.PackageLength += FontBlockSize +
> Ucs2FontBlockSize;
>
> --
> 2.18.0.windows.1
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [patch 1/2] MdeModulePkg/String.c: Zero memory before free (CVE-2019-14558)
2020-02-13 4:03 ` [patch 1/2] MdeModulePkg/String.c: Zero memory before free (CVE-2019-14558) Dandan Bi
2020-02-13 4:06 ` Wang, Jian J
@ 2020-02-14 6:44 ` Dong, Eric
1 sibling, 0 replies; 8+ messages in thread
From: Dong, Eric @ 2020-02-14 6:44 UTC (permalink / raw)
To: Bi, Dandan, devel@edk2.groups.io; +Cc: Gao, Liming, Wang, Jian J
Reviewed-by: Eric Dong <eric.dong@intel.com>
-----Original Message-----
From: Bi, Dandan <dandan.bi@intel.com>
Sent: Thursday, February 13, 2020 12:03 PM
To: devel@edk2.groups.io
Cc: Gao, Liming <liming.gao@intel.com>; Dong, Eric <eric.dong@intel.com>; Wang, Jian J <jian.j.wang@intel.com>
Subject: [patch 1/2] MdeModulePkg/String.c: Zero memory before free (CVE-2019-14558)
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1611
Cc: Liming Gao <liming.gao@intel.com>
Cc: Eric Dong <eric.dong@intel.com>
Cc: Jian J Wang <jian.j.wang@intel.com>
Signed-off-by: Dandan Bi <dandan.bi@intel.com>
---
MdeModulePkg/Universal/HiiDatabaseDxe/String.c | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/MdeModulePkg/Universal/HiiDatabaseDxe/String.c b/MdeModulePkg/Universal/HiiDatabaseDxe/String.c
index 505e063d49..10a1e691a3 100644
--- a/MdeModulePkg/Universal/HiiDatabaseDxe/String.c
+++ b/MdeModulePkg/Universal/HiiDatabaseDxe/String.c
@@ -1004,10 +1004,11 @@ SetStringWorker (
BlockPtr,
StringTextPtr + AsciiStrSize ((CHAR8 *)StringTextPtr),
TmpSize
);
+ ZeroMem (StringPackage->StringBlock, OldBlockSize);
FreePool (StringPackage->StringBlock);
StringPackage->StringBlock = Block;
StringPackage->StringPkgHdr->Header.Length += (UINT32) (BlockSize - OldBlockSize);
break;
@@ -1037,10 +1038,11 @@ SetStringWorker (
BlockPtr,
StringTextPtr + StringSize,
OldBlockSize - (StringTextPtr - StringPackage->StringBlock) - StringSize
);
+ ZeroMem (StringPackage->StringBlock, OldBlockSize);
FreePool (StringPackage->StringBlock);
StringPackage->StringBlock = Block;
StringPackage->StringPkgHdr->Header.Length += (UINT32) (BlockSize - OldBlockSize);
break;
@@ -1088,10 +1090,11 @@ SetStringWorker (
);
BlockPtr += StrSize (GlobalFont->FontInfo->FontName);
CopyMem (BlockPtr, StringPackage->StringBlock, OldBlockSize);
+ ZeroMem (StringPackage->StringBlock, OldBlockSize);
FreePool (StringPackage->StringBlock);
StringPackage->StringBlock = Block;
StringPackage->StringPkgHdr->Header.Length += Ext2.Length;
return EFI_SUCCESS;
@@ -1273,10 +1276,11 @@ HiiNewString (
//
// Append a EFI_HII_SIBT_END block to the end.
//
*BlockPtr = EFI_HII_SIBT_END;
+ ZeroMem (StringPackage->StringBlock, OldBlockSize);
FreePool (StringPackage->StringBlock);
StringPackage->StringBlock = StringBlock;
StringPackage->StringPkgHdr->Header.Length += Ucs2BlockSize;
PackageListNode->PackageListHdr.PackageLength += Ucs2BlockSize;
}
@@ -1404,10 +1408,11 @@ HiiNewString (
//
// Append a EFI_HII_SIBT_END block to the end.
//
*BlockPtr = EFI_HII_SIBT_END;
+ ZeroMem (StringPackage->StringBlock, OldBlockSize);
FreePool (StringPackage->StringBlock);
StringPackage->StringBlock = StringBlock;
StringPackage->StringPkgHdr->Header.Length += Ucs2BlockSize;
PackageListNode->PackageListHdr.PackageLength += Ucs2BlockSize;
@@ -1446,10 +1451,11 @@ HiiNewString (
//
// Append a EFI_HII_SIBT_END block to the end.
//
*BlockPtr = EFI_HII_SIBT_END;
+ ZeroMem (StringPackage->StringBlock, OldBlockSize);
FreePool (StringPackage->StringBlock);
StringPackage->StringBlock = StringBlock;
StringPackage->StringPkgHdr->Header.Length += Ucs2FontBlockSize;
PackageListNode->PackageListHdr.PackageLength += Ucs2FontBlockSize;
@@ -1507,10 +1513,11 @@ HiiNewString (
//
// Append a EFI_HII_SIBT_END block to the end.
//
*BlockPtr = EFI_HII_SIBT_END;
+ ZeroMem (StringPackage->StringBlock, OldBlockSize);
FreePool (StringPackage->StringBlock);
StringPackage->StringBlock = StringBlock;
StringPackage->StringPkgHdr->Header.Length += FontBlockSize + Ucs2FontBlockSize;
PackageListNode->PackageListHdr.PackageLength += FontBlockSize + Ucs2FontBlockSize;
--
2.18.0.windows.1
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [patch 2/2] MdeModulePkg/DisplayEngine: Zero memory before free (CVE-2019-14558)
2020-02-13 4:03 [patch] MdeModulePkg/HiiDB: Remove configuration table when it's freed (CVE-2019-14586) Dandan Bi
2020-02-13 4:03 ` [patch 1/2] MdeModulePkg/String.c: Zero memory before free (CVE-2019-14558) Dandan Bi
@ 2020-02-13 4:03 ` Dandan Bi
2020-02-13 4:11 ` Wang, Jian J
2020-02-14 6:45 ` Dong, Eric
2020-02-13 4:05 ` [patch] MdeModulePkg/HiiDB: Remove configuration table when it's freed (CVE-2019-14586) Wang, Jian J
2 siblings, 2 replies; 8+ messages in thread
From: Dandan Bi @ 2020-02-13 4:03 UTC (permalink / raw)
To: devel; +Cc: Liming Gao, Eric Dong, Jian J Wang
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1611
Cc: Liming Gao <liming.gao@intel.com>
Cc: Eric Dong <eric.dong@intel.com>
Cc: Jian J Wang <jian.j.wang@intel.com>
Signed-off-by: Dandan Bi <dandan.bi@intel.com>
---
MdeModulePkg/Universal/DisplayEngineDxe/ProcessOptions.c | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/MdeModulePkg/Universal/DisplayEngineDxe/ProcessOptions.c b/MdeModulePkg/Universal/DisplayEngineDxe/ProcessOptions.c
index 7d9486112b..1087004939 100644
--- a/MdeModulePkg/Universal/DisplayEngineDxe/ProcessOptions.c
+++ b/MdeModulePkg/Universal/DisplayEngineDxe/ProcessOptions.c
@@ -821,10 +821,11 @@ PasswordProcess (
//
// Old password exist, ask user for the old password
//
Status = ReadString (MenuOption, gPromptForPassword, StringPtr);
if (EFI_ERROR (Status)) {
+ ZeroMem (StringPtr, (Maximum + 1) * sizeof (CHAR16));
FreePool (StringPtr);
return Status;
}
//
@@ -838,11 +839,11 @@ PasswordProcess (
//
PasswordInvalid ();
} else {
Status = EFI_SUCCESS;
}
-
+ ZeroMem (StringPtr, (Maximum + 1) * sizeof (CHAR16));
FreePool (StringPtr);
return Status;
}
}
@@ -854,10 +855,11 @@ PasswordProcess (
if (EFI_ERROR (Status)) {
//
// Reset state machine for password
//
Question->PasswordCheck (gFormData, Question, NULL);
+ ZeroMem (StringPtr, (Maximum + 1) * sizeof (CHAR16));
FreePool (StringPtr);
return Status;
}
//
@@ -869,10 +871,12 @@ PasswordProcess (
if (EFI_ERROR (Status)) {
//
// Reset state machine for password
//
Question->PasswordCheck (gFormData, Question, NULL);
+ ZeroMem (StringPtr, (Maximum + 1) * sizeof (CHAR16));
+ ZeroMem (TempString, (Maximum + 1) * sizeof (CHAR16));
FreePool (StringPtr);
FreePool (TempString);
return Status;
}
--
2.18.0.windows.1
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [patch 2/2] MdeModulePkg/DisplayEngine: Zero memory before free (CVE-2019-14558)
2020-02-13 4:03 ` [patch 2/2] MdeModulePkg/DisplayEngine: " Dandan Bi
@ 2020-02-13 4:11 ` Wang, Jian J
2020-02-14 6:45 ` Dong, Eric
1 sibling, 0 replies; 8+ messages in thread
From: Wang, Jian J @ 2020-02-13 4:11 UTC (permalink / raw)
To: Bi, Dandan, devel@edk2.groups.io; +Cc: Gao, Liming, Dong, Eric
Please update copyright year for patch 1 and 2. With it addressed,
Reviewed-by: Jian J Wang <jian.j.wang@intel.com>
Regards,
Jian
> -----Original Message-----
> From: Bi, Dandan <dandan.bi@intel.com>
> Sent: Thursday, February 13, 2020 12:03 PM
> To: devel@edk2.groups.io
> Cc: Gao, Liming <liming.gao@intel.com>; Dong, Eric <eric.dong@intel.com>;
> Wang, Jian J <jian.j.wang@intel.com>
> Subject: [patch 2/2] MdeModulePkg/DisplayEngine: Zero memory before free
> (CVE-2019-14558)
>
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1611
>
> Cc: Liming Gao <liming.gao@intel.com>
> Cc: Eric Dong <eric.dong@intel.com>
> Cc: Jian J Wang <jian.j.wang@intel.com>
> Signed-off-by: Dandan Bi <dandan.bi@intel.com>
> ---
> MdeModulePkg/Universal/DisplayEngineDxe/ProcessOptions.c | 6 +++++-
> 1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/MdeModulePkg/Universal/DisplayEngineDxe/ProcessOptions.c
> b/MdeModulePkg/Universal/DisplayEngineDxe/ProcessOptions.c
> index 7d9486112b..1087004939 100644
> --- a/MdeModulePkg/Universal/DisplayEngineDxe/ProcessOptions.c
> +++ b/MdeModulePkg/Universal/DisplayEngineDxe/ProcessOptions.c
> @@ -821,10 +821,11 @@ PasswordProcess (
> //
> // Old password exist, ask user for the old password
> //
> Status = ReadString (MenuOption, gPromptForPassword, StringPtr);
> if (EFI_ERROR (Status)) {
> + ZeroMem (StringPtr, (Maximum + 1) * sizeof (CHAR16));
> FreePool (StringPtr);
> return Status;
> }
>
> //
> @@ -838,11 +839,11 @@ PasswordProcess (
> //
> PasswordInvalid ();
> } else {
> Status = EFI_SUCCESS;
> }
> -
> + ZeroMem (StringPtr, (Maximum + 1) * sizeof (CHAR16));
> FreePool (StringPtr);
> return Status;
> }
> }
>
> @@ -854,10 +855,11 @@ PasswordProcess (
> if (EFI_ERROR (Status)) {
> //
> // Reset state machine for password
> //
> Question->PasswordCheck (gFormData, Question, NULL);
> + ZeroMem (StringPtr, (Maximum + 1) * sizeof (CHAR16));
> FreePool (StringPtr);
> return Status;
> }
>
> //
> @@ -869,10 +871,12 @@ PasswordProcess (
> if (EFI_ERROR (Status)) {
> //
> // Reset state machine for password
> //
> Question->PasswordCheck (gFormData, Question, NULL);
> + ZeroMem (StringPtr, (Maximum + 1) * sizeof (CHAR16));
> + ZeroMem (TempString, (Maximum + 1) * sizeof (CHAR16));
> FreePool (StringPtr);
> FreePool (TempString);
> return Status;
> }
>
> --
> 2.18.0.windows.1
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [patch 2/2] MdeModulePkg/DisplayEngine: Zero memory before free (CVE-2019-14558)
2020-02-13 4:03 ` [patch 2/2] MdeModulePkg/DisplayEngine: " Dandan Bi
2020-02-13 4:11 ` Wang, Jian J
@ 2020-02-14 6:45 ` Dong, Eric
1 sibling, 0 replies; 8+ messages in thread
From: Dong, Eric @ 2020-02-14 6:45 UTC (permalink / raw)
To: Bi, Dandan, devel@edk2.groups.io; +Cc: Gao, Liming, Wang, Jian J
Reviewed-by: Eric Dong <eric.dong@intel.com>
-----Original Message-----
From: Bi, Dandan <dandan.bi@intel.com>
Sent: Thursday, February 13, 2020 12:03 PM
To: devel@edk2.groups.io
Cc: Gao, Liming <liming.gao@intel.com>; Dong, Eric <eric.dong@intel.com>; Wang, Jian J <jian.j.wang@intel.com>
Subject: [patch 2/2] MdeModulePkg/DisplayEngine: Zero memory before free (CVE-2019-14558)
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1611
Cc: Liming Gao <liming.gao@intel.com>
Cc: Eric Dong <eric.dong@intel.com>
Cc: Jian J Wang <jian.j.wang@intel.com>
Signed-off-by: Dandan Bi <dandan.bi@intel.com>
---
MdeModulePkg/Universal/DisplayEngineDxe/ProcessOptions.c | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/MdeModulePkg/Universal/DisplayEngineDxe/ProcessOptions.c b/MdeModulePkg/Universal/DisplayEngineDxe/ProcessOptions.c
index 7d9486112b..1087004939 100644
--- a/MdeModulePkg/Universal/DisplayEngineDxe/ProcessOptions.c
+++ b/MdeModulePkg/Universal/DisplayEngineDxe/ProcessOptions.c
@@ -821,10 +821,11 @@ PasswordProcess (
//
// Old password exist, ask user for the old password
//
Status = ReadString (MenuOption, gPromptForPassword, StringPtr);
if (EFI_ERROR (Status)) {
+ ZeroMem (StringPtr, (Maximum + 1) * sizeof (CHAR16));
FreePool (StringPtr);
return Status;
}
//
@@ -838,11 +839,11 @@ PasswordProcess (
//
PasswordInvalid ();
} else {
Status = EFI_SUCCESS;
}
-
+ ZeroMem (StringPtr, (Maximum + 1) * sizeof (CHAR16));
FreePool (StringPtr);
return Status;
}
}
@@ -854,10 +855,11 @@ PasswordProcess (
if (EFI_ERROR (Status)) {
//
// Reset state machine for password
//
Question->PasswordCheck (gFormData, Question, NULL);
+ ZeroMem (StringPtr, (Maximum + 1) * sizeof (CHAR16));
FreePool (StringPtr);
return Status;
}
//
@@ -869,10 +871,12 @@ PasswordProcess (
if (EFI_ERROR (Status)) {
//
// Reset state machine for password
//
Question->PasswordCheck (gFormData, Question, NULL);
+ ZeroMem (StringPtr, (Maximum + 1) * sizeof (CHAR16));
+ ZeroMem (TempString, (Maximum + 1) * sizeof (CHAR16));
FreePool (StringPtr);
FreePool (TempString);
return Status;
}
--
2.18.0.windows.1
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [patch] MdeModulePkg/HiiDB: Remove configuration table when it's freed (CVE-2019-14586)
2020-02-13 4:03 [patch] MdeModulePkg/HiiDB: Remove configuration table when it's freed (CVE-2019-14586) Dandan Bi
2020-02-13 4:03 ` [patch 1/2] MdeModulePkg/String.c: Zero memory before free (CVE-2019-14558) Dandan Bi
2020-02-13 4:03 ` [patch 2/2] MdeModulePkg/DisplayEngine: " Dandan Bi
@ 2020-02-13 4:05 ` Wang, Jian J
2 siblings, 0 replies; 8+ messages in thread
From: Wang, Jian J @ 2020-02-13 4:05 UTC (permalink / raw)
To: Bi, Dandan, devel@edk2.groups.io; +Cc: Gao, Liming, Dong, Eric
Reviewed-by: Jian J Wang <jian.j.wang@intel.com>
Regards,
Jian
> -----Original Message-----
> From: Bi, Dandan <dandan.bi@intel.com>
> Sent: Thursday, February 13, 2020 12:03 PM
> To: devel@edk2.groups.io
> Cc: Gao, Liming <liming.gao@intel.com>; Dong, Eric <eric.dong@intel.com>;
> Wang, Jian J <jian.j.wang@intel.com>
> Subject: [patch] MdeModulePkg/HiiDB: Remove configuration table when it's
> freed (CVE-2019-14586)
>
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1995
>
> Fix the corner case issue that the original configuration runtime
> memory is freed, but it is still exposed to the OS runtime.
> So this patch is to remove the configuration table to avoid being
> used in OS runtime when the configuration runtime memory is freed.
>
> Cc: Liming Gao <liming.gao@intel.com>
> Cc: Eric Dong <eric.dong@intel.com>
> Cc: Jian J Wang <jian.j.wang@intel.com>
> Signed-off-by: Dandan Bi <dandan.bi@intel.com>
> Reviewed-by: Eric Dong <eric.dong@intel.com>
> ---
> MdeModulePkg/Universal/HiiDatabaseDxe/Database.c | 8 ++++++++
> 1 file changed, 8 insertions(+)
>
> diff --git a/MdeModulePkg/Universal/HiiDatabaseDxe/Database.c
> b/MdeModulePkg/Universal/HiiDatabaseDxe/Database.c
> index d3791ca68b..36265b8ff9 100644
> --- a/MdeModulePkg/Universal/HiiDatabaseDxe/Database.c
> +++ b/MdeModulePkg/Universal/HiiDatabaseDxe/Database.c
> @@ -3374,10 +3374,14 @@ HiiGetConfigRespInfo(
> }
> gRTConfigRespBuffer = (EFI_STRING) AllocateRuntimeZeroPool
> (gConfigRespSize);
> if (gRTConfigRespBuffer == NULL){
> FreePool(ConfigAltResp);
> DEBUG ((DEBUG_ERROR, "[HiiDatabase]: No enough memory resource to
> store the ConfigResp string.\n"));
> + //
> + // Remove from the System Table when the configuration runtime buffer is
> freed.
> + //
> + gBS->InstallConfigurationTable (&gEfiHiiConfigRoutingProtocolGuid,
> NULL);
> return EFI_OUT_OF_RESOURCES;
> }
> } else {
> ZeroMem(gRTConfigRespBuffer,gConfigRespSize);
> }
> @@ -3429,10 +3433,14 @@ HiiGetDatabaseInfo(
> DEBUG ((DEBUG_WARN, "[HiiDatabase]: Memory allocation is required
> after ReadyToBoot, which may change memory map and cause S4 resume
> issue.\n"));
> }
> gRTDatabaseInfoBuffer = AllocateRuntimeZeroPool (gDatabaseInfoSize);
> if (gRTDatabaseInfoBuffer == NULL){
> DEBUG ((DEBUG_ERROR, "[HiiDatabase]: No enough memory resource to
> store the HiiDatabase info.\n"));
> + //
> + // Remove from the System Table when the configuration runtime buffer is
> freed.
> + //
> + gBS->InstallConfigurationTable (&gEfiHiiDatabaseProtocolGuid, NULL);
> return EFI_OUT_OF_RESOURCES;
> }
> } else {
> ZeroMem(gRTDatabaseInfoBuffer,gDatabaseInfoSize);
> }
> --
> 2.18.0.windows.1
^ permalink raw reply [flat|nested] 8+ messages in thread