Date
1 - 3 of 3
[PATCH v1 01/12] MdeModulePkg/SmbiosDxe: Fix pointer and buffer overflow CodeQL alerts
|
Michael Kubacki
From: Erich McMillan <emcmillan@...>
Details for these CodeQL alerts can be found here: - Pointer overflow check (cpp/pointer-overflow-check): - https://cwe.mitre.org/data/definitions/758.html - Potential buffer overflow check (cpp/potential-buffer-overflow): - https://cwe.mitre.org/data/definitions/676.html CodeQL alert: - Line 1612 in MdeModulePkg/Universal/SmbiosDxe/SmbiosDxe.c - Type: Pointer overflow check - Severity: Low - Problem: Range check relying on pointer overflow Cc: Dandan Bi <dandan.bi@...> Cc: Erich McMillan <emcmillan@...> Cc: Jian J Wang <jian.j.wang@...> Cc: Liming Gao <gaoliming@...> Cc: Michael Kubacki <mikuback@...> Cc: Star Zeng <star.zeng@...> Cc: Zhichao Gao <zhichao.gao@...> Cc: Zhiguang Liu <zhiguang.liu@...> Co-authored-by: Michael Kubacki <michael.kubacki@...> Signed-off-by: Erich McMillan <emcmillan@...> --- MdeModulePkg/Universal/SmbiosDxe/SmbiosDxe.c | 4 +++- MdeModulePkg/Universal/SmbiosDxe/SmbiosDxe.inf | 1 + 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/MdeModulePkg/Universal/SmbiosDxe/SmbiosDxe.c b/MdeModulePkg/= Universal/SmbiosDxe/SmbiosDxe.c index 1d43adc7662c..03eca4e6b103 100644 --- a/MdeModulePkg/Universal/SmbiosDxe/SmbiosDxe.c +++ b/MdeModulePkg/Universal/SmbiosDxe/SmbiosDxe.c @@ -8,6 +8,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent **/ =20 #include "SmbiosDxe.h" +#include <Library/SafeIntLib.h> =20 // // Module Global: @@ -1594,6 +1595,7 @@ ParseAndAddExistingSmbiosTable ( CHAR8 *String; EFI_SMBIOS_HANDLE SmbiosHandle; SMBIOS_STRUCTURE_POINTER SmbiosEnd; + UINTN SafeIntResult; =20 mPrivateData.Smbios.MajorVersion =3D MajorVersion; mPrivateData.Smbios.MinorVersion =3D MinorVersion; @@ -1609,7 +1611,7 @@ ParseAndAddExistingSmbiosTable ( // Make sure not to access memory beyond SmbiosEnd // if ((Smbios.Raw + sizeof (SMBIOS_STRUCTURE) > SmbiosEnd.Raw) || - (Smbios.Raw + sizeof (SMBIOS_STRUCTURE) < Smbios.Raw)) + EFI_ERROR (SafeUintnAdd ((UINTN)Smbios.Raw, sizeof (SMBIOS_STRUC= TURE), &SafeIntResult))) { return EFI_INVALID_PARAMETER; } diff --git a/MdeModulePkg/Universal/SmbiosDxe/SmbiosDxe.inf b/MdeModulePk= g/Universal/SmbiosDxe/SmbiosDxe.inf index c03915a6921f..8b7c74694775 100644 --- a/MdeModulePkg/Universal/SmbiosDxe/SmbiosDxe.inf +++ b/MdeModulePkg/Universal/SmbiosDxe/SmbiosDxe.inf @@ -42,6 +42,7 @@ [LibraryClasses] DebugLib PcdLib HobLib + SafeIntLib =20 [Protocols] gEfiSmbiosProtocolGuid ## PRODUCES --=20 2.28.0.windows.1 |
|
|
|
Michael D Kinney
Hi Erich,
toggle quoted message
Show quoted text
One comment below. Mike -----Original Message-----The line above does the same unsafe add operation. The use of SafeUintnAdd()should be moved before the if statement and SafeIntResult should be used twice in the if statement. The check for EFI_ERROR from SafeUintnAdd() can be performed before the if statement. - (Smbios.Raw + sizeof (SMBIOS_STRUCTURE) < Smbios.Raw)) |
|
|
|
Michael Kubacki
Thanks Mike. Erich, I'll include the update for this in the v2 series.
toggle quoted message
Show quoted text
On 11/23/2022 8:28 PM, Michael D Kinney wrote:
Hi Erich, |
|
|