回复: [edk2-devel] [PATCH v2 2/2] ShellPkg/UefiShellDebug1CommandsLib: Replace hardcoded SMBIOS strings.

["gaoliming" <gaoliming@byosoft.com.cn>]

Reviewed-by: Liming Gao <gaoliming@byosoft.com.cn>

> -----邮件原件-----
> 发件人: devel@edk2.groups.io <devel@edk2.groups.io> 代表 Giri
> Mudusuru via groups.io
> 发送时间: 2023年5月6日 18:28
> 收件人: devel@edk2.groups.io
> 抄送: Giri Mudusuru <girim@apple.com>; Michael D Kinney
> <michael.d.kinney@intel.com>; Liming Gao <gaoliming@byosoft.com.cn>;
> Zhiguang Liu <zhiguang.liu@intel.com>; Andrew Fish <afish@apple.com>
> 主题: [edk2-devel] [PATCH v2 2/2] ShellPkg/UefiShellDebug1CommandsLib:
> Replace hardcoded SMBIOS strings.
> 
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3805
> 
> Replace hardcoded SMBIOS Anchor string and size with defines.
> 
> Fix buffer overflow as described below.
> 
> Smbios64BitPrintEPSInfo () is coded like:
> UINT8  Anchor[5];
> 
> MemToString (Anchor, SmbiosTable->AnchorString, 5);
> 
> But the definition of MemToString()
>   Copy Length of Src buffer to Dest buffer,
>   add a NULL termination to Dest buffer.
> 
> So Anchor needs to be +1 the size of the SMBIOS Anchor string `_SM3_`.
> 
> Changes from v1 to v2:
>  - Replace doxygen style inline comments
> 
> Cc: Michael D Kinney <michael.d.kinney@intel.com>
> Cc: Liming Gao <gaoliming@byosoft.com.cn>
> Cc: Zhiguang Liu <zhiguang.liu@intel.com>
> Cc: Andrew Fish <afish@apple.com>
> Signed-off-by: Giri Mudusuru <girim@apple.com>
> ---
>  .../UefiShellDebug1CommandsLib/SmbiosView/PrintInfo.c    | 9 +++++----
>  .../UefiShellDebug1CommandsLib/SmbiosView/SmbiosView.c   | 9
> +++++----
>  2 files changed, 10 insertions(+), 8 deletions(-)
> 
> diff --git
> a/ShellPkg/Library/UefiShellDebug1CommandsLib/SmbiosView/PrintInfo.c
> b/ShellPkg/Library/UefiShellDebug1CommandsLib/SmbiosView/PrintInfo.c
> index 1811cf0c44..a14b79904d 100644
> --- a/ShellPkg/Library/UefiShellDebug1CommandsLib/SmbiosView/PrintInfo.c
> +++
> b/ShellPkg/Library/UefiShellDebug1CommandsLib/SmbiosView/PrintInfo.c
> @@ -5,6 +5,7 @@
>    Copyright (c) 1985 - 2022, American Megatrends International LLC.<BR>
> 
>    (C) Copyright 2014 Hewlett-Packard Development Company, L.P.<BR>
> 
>    (C) Copyright 2015-2019 Hewlett Packard Enterprise Development
> LP<BR>
> 
> +  Copyright (c) 2023 Apple Inc. All rights reserved.<BR>
> 
>    SPDX-License-Identifier: BSD-2-Clause-Patent
> 
> 
> 
>  **/
> 
> @@ -135,7 +136,7 @@ SmbiosPrintEPSInfo (
>    IN  UINT8                     Option
> 
>    )
> 
>  {
> 
> -  UINT8  Anchor[5];
> 
> +  UINT8  Anchor[SMBIOS_ANCHOR_STRING_LENGTH + 1]; // Including
> terminating NULL character
> 
>    UINT8  InAnchor[6];
> 
> 
> 
>    if (SmbiosTable == NULL) {
> 
> @@ -149,7 +150,7 @@ SmbiosPrintEPSInfo (
> 
> 
>    if (Option >= SHOW_NORMAL) {
> 
>      ShellPrintHiiEx (-1, -1, NULL, STRING_TOKEN
> (STR_SMBIOSVIEW_PRINTINFO_ENTRY_POINT_SIGN),
> gShellDebug1HiiHandle);
> 
> -    MemToString (Anchor, SmbiosTable->AnchorString, 4);
> 
> +    MemToString (Anchor, SmbiosTable->AnchorString,
> SMBIOS_ANCHOR_STRING_LENGTH);
> 
>      ShellPrintHiiEx (-1, -1, NULL, STRING_TOKEN
> (STR_SMBIOSVIEW_PRINTINFO_ANCHOR_STR), gShellDebug1HiiHandle,
> Anchor);
> 
>      ShellPrintHiiEx (
> 
>        -1,
> 
> @@ -220,7 +221,7 @@ Smbios64BitPrintEPSInfo (
>    IN  UINT8                         Option
> 
>    )
> 
>  {
> 
> -  UINT8  Anchor[5];
> 
> +  UINT8  Anchor[SMBIOS_3_0_ANCHOR_STRING_LENGTH + 1]; //
> Including terminating NULL character
> 
> 
> 
>    if (SmbiosTable == NULL) {
> 
>      ShellPrintHiiEx (-1, -1, NULL, STRING_TOKEN
> (STR_SMBIOSVIEW_PRINTINFO_SMBIOSTABLE_NULL),
> gShellDebug1HiiHandle);
> 
> @@ -234,7 +235,7 @@ Smbios64BitPrintEPSInfo (
>    if (Option >= SHOW_NORMAL) {
> 
>      ShellPrintHiiEx (-1, -1, NULL, STRING_TOKEN
> (STR_SMBIOSVIEW_PRINTINFO_64_BIT_ENTRY_POINT_SIGN),
> gShellDebug1HiiHandle);
> 
> 
> 
> -    MemToString (Anchor, SmbiosTable->AnchorString, 5);
> 
> +    MemToString (Anchor, SmbiosTable->AnchorString,
> SMBIOS_3_0_ANCHOR_STRING_LENGTH);
> 
>      ShellPrintHiiEx (-1, -1, NULL, STRING_TOKEN
> (STR_SMBIOSVIEW_PRINTINFO_ANCHOR_STR), gShellDebug1HiiHandle,
> Anchor);
> 
> 
> 
>      ShellPrintHiiEx (
> 
> diff --git
> a/ShellPkg/Library/UefiShellDebug1CommandsLib/SmbiosView/SmbiosView.c
> b/ShellPkg/Library/UefiShellDebug1CommandsLib/SmbiosView/SmbiosView.c
> index e9360beb23..7e7eef3fd8 100644
> ---
> a/ShellPkg/Library/UefiShellDebug1CommandsLib/SmbiosView/SmbiosView.c
> +++
> b/ShellPkg/Library/UefiShellDebug1CommandsLib/SmbiosView/SmbiosView.c
> @@ -3,6 +3,7 @@
> 
> 
>    (C) Copyright 2015 Hewlett-Packard Development Company, L.P.<BR>
> 
>    Copyright (c) 2005 - 2018, Intel Corporation. All rights reserved.<BR>
> 
> +  Copyright (c) 2023 Apple Inc. All rights reserved.<BR>
> 
>    SPDX-License-Identifier: BSD-2-Clause-Patent
> 
> 
> 
>  **/
> 
> @@ -263,7 +264,7 @@ SMBiosView (
>      return EFI_BAD_BUFFER_SIZE;
> 
>    }
> 
> 
> 
> -  if (CompareMem (SMBiosTable->AnchorString, "_SM_", 4) == 0) {
> 
> +  if (CompareMem (SMBiosTable->AnchorString, SMBIOS_ANCHOR_STRING,
> SMBIOS_ANCHOR_STRING_LENGTH) == 0) {
> 
>      //
> 
>      // Have got SMBIOS table
> 
>      //
> 
> @@ -441,7 +442,7 @@ SMBios64View (
>      return EFI_BAD_BUFFER_SIZE;
> 
>    }
> 
> 
> 
> -  if (CompareMem (SMBiosTable->AnchorString, "_SM3_", 5) == 0) {
> 
> +  if (CompareMem (SMBiosTable->AnchorString,
> SMBIOS_3_0_ANCHOR_STRING, SMBIOS_3_0_ANCHOR_STRING_LENGTH)
> == 0) {
> 
>      //
> 
>      // Have got SMBIOS table
> 
>      //
> 
> @@ -612,7 +613,7 @@ InitSmbiosTableStatistics (
>      return EFI_NOT_FOUND;
> 
>    }
> 
> 
> 
> -  if (CompareMem (SMBiosTable->AnchorString, "_SM_", 4) != 0) {
> 
> +  if (CompareMem (SMBiosTable->AnchorString, SMBIOS_ANCHOR_STRING,
> SMBIOS_ANCHOR_STRING_LENGTH) != 0) {
> 
>      ShellPrintHiiEx (-1, -1, NULL, STRING_TOKEN
> (STR_SMBIOSVIEW_SMBIOSVIEW_SMBIOS_TABLE), gShellDebug1HiiHandle);
> 
>      return EFI_INVALID_PARAMETER;
> 
>    }
> 
> @@ -753,7 +754,7 @@ InitSmbios64BitTableStatistics (
>      return EFI_NOT_FOUND;
> 
>    }
> 
> 
> 
> -  if (CompareMem (SMBiosTable->AnchorString, "_SM3_", 5) != 0) {
> 
> +  if (CompareMem (SMBiosTable->AnchorString,
> SMBIOS_3_0_ANCHOR_STRING,
> SMBIOS_3_0_ANCHOR_STRING_LENGTH) != 0) {
> 
>      ShellPrintHiiEx (-1, -1, NULL, STRING_TOKEN
> (STR_SMBIOSVIEW_SMBIOSVIEW_SMBIOS_TABLE), gShellDebug1HiiHandle);
> 
>      return EFI_INVALID_PARAMETER;
> 
>    }
> 
> --
> 2.39.2 (Apple Git-144)
> 
> 
> 
> -=-=-=-=-=-=
> Groups.io Links: You receive all messages sent to this group.
> View/Reply Online (#104189):
> https://edk2.groups.io/g/devel/message/104189
> Mute This Topic: https://groups.io/mt/98722745/4905953
> Group Owner: devel+owner@edk2.groups.io
> Unsubscribe: https://edk2.groups.io/g/devel/unsub
> [gaoliming@byosoft.com.cn]
> -=-=-=-=-=-=
>