MdeModulePkg/HiiDB: Remove configuration table when it's freed (CVE-2019-14586)

[edk2-devel] [patch] MdeModulePkg/HiiDB: Remove configuration table when it's freed (CVE-2019-14586)

Posted by Dandan Bi 4 years, 2 months ago

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


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#54332): https://edk2.groups.io/g/devel/message/54332
Mute This Topic: https://groups.io/mt/71232485/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [patch] MdeModulePkg/HiiDB: Remove configuration table when it's freed (CVE-2019-14586)

Posted by Wang, Jian J 4 years, 2 months ago

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


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#54335): https://edk2.groups.io/g/devel/message/54335
Mute This Topic: https://groups.io/mt/71232485/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

[edk2-devel] [patch 1/2] MdeModulePkg/String.c: Zero memory before free (CVE-2019-14558)

Posted by Dandan Bi 4 years, 2 months ago

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


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#54333): https://edk2.groups.io/g/devel/message/54333
Mute This Topic: https://groups.io/mt/71232488/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [patch 1/2] MdeModulePkg/String.c: Zero memory before free (CVE-2019-14558)

Posted by Dong, Eric 4 years, 2 months ago

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


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#54413): https://edk2.groups.io/g/devel/message/54413
Mute This Topic: https://groups.io/mt/71232488/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [patch 1/2] MdeModulePkg/String.c: Zero memory before free (CVE-2019-14558)

Posted by Wang, Jian J 4 years, 2 months ago

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


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#54336): https://edk2.groups.io/g/devel/message/54336
Mute This Topic: https://groups.io/mt/71232488/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

[edk2-devel] [patch 2/2] MdeModulePkg/DisplayEngine: Zero memory before free (CVE-2019-14558)

Posted by Dandan Bi 4 years, 2 months ago

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


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#54334): https://edk2.groups.io/g/devel/message/54334
Mute This Topic: https://groups.io/mt/71232490/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [patch 2/2] MdeModulePkg/DisplayEngine: Zero memory before free (CVE-2019-14558)

Posted by Dong, Eric 4 years, 2 months ago

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


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#54415): https://edk2.groups.io/g/devel/message/54415
Mute This Topic: https://groups.io/mt/71232490/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [patch 2/2] MdeModulePkg/DisplayEngine: Zero memory before free (CVE-2019-14558)

Posted by Wang, Jian J 4 years, 2 months ago

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


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#54337): https://edk2.groups.io/g/devel/message/54337
Mute This Topic: https://groups.io/mt/71232490/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-