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

[Giri Mudusuru via groups.io]

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_`.

Cc: Michael D Kinney <michael.d.kinney at intel.com>
Cc: Liming Gao <gaoliming at byosoft.com.cn>
Cc: Zhiguang Liu <zhiguang.liu at intel.com>
Cc: Andrew Fish <afish at apple.com>
Signed-off-by: Giri Mudusuru <girim at 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..dd190b006f 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 (#103785): https://edk2.groups.io/g/devel/message/103785
Mute This Topic: https://groups.io/mt/98566051/1813853
Group Owner: devel+owner at edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [edk2-devel-archive at redhat.com]
-=-=-=-=-=-=-=-=-=-=-=-