From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from linux.microsoft.com (linux.microsoft.com [13.77.154.182]) by mx.groups.io with SMTP id smtpd.web10.13597.1669254378139384525 for ; Wed, 23 Nov 2022 17:46:18 -0800 Authentication-Results: mx.groups.io; dkim=pass header.i=@linux.microsoft.com header.s=default header.b=kWer4u3e; spf=pass (domain: linux.microsoft.com, ip: 13.77.154.182, mailfrom: mikuback@linux.microsoft.com) Received: from [192.168.4.22] (unknown [47.201.8.94]) by linux.microsoft.com (Postfix) with ESMTPSA id 82D0420B6C40; Wed, 23 Nov 2022 17:46:16 -0800 (PST) DKIM-Filter: OpenDKIM Filter v2.11.0 linux.microsoft.com 82D0420B6C40 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linux.microsoft.com; s=default; t=1669254377; bh=DQ62de0Q02UIZIMbU7g/4z+bs7QvYmSQ+M7xGRBeXcU=; h=Date:Subject:To:Cc:References:From:In-Reply-To:From; b=kWer4u3eOsXAgb3gY5OPGpzEa2/XmFWwrzkZxkiB0J22dBVV9ki4r0JD5HLLv1xQm J+wm5wUbYQ2McwWpnQFIV7t9ryLfIjlE5j6JlLbGOg6xnK6qRpQA8nx9zNhZ6eXAcR a1DwHHFdM+kUzYIgMK/PF6Pn514wO4DQLsORTPco= Message-ID: Date: Wed, 23 Nov 2022 20:46:15 -0500 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:91.0) Gecko/20100101 Thunderbird/91.13.1 Subject: Re: [edk2-devel] [PATCH v1 01/12] MdeModulePkg/SmbiosDxe: Fix pointer and buffer overflow CodeQL alerts To: devel@edk2.groups.io, michael.d.kinney@intel.com Cc: "Bi, Dandan" , Erich McMillan , "Wang, Jian J" , "Gao, Liming" , "Zeng, Star" , "Gao, Zhichao" , "Liu, Zhiguang" , "Kubacki, Michael" References: <20221109173246.174-1-mikuback@linux.microsoft.com> <20221109173246.174-2-mikuback@linux.microsoft.com> From: "Michael Kubacki" In-Reply-To: Content-Language: en-US Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit Thanks Mike. Erich, I'll include the update for this in the v2 series. On 11/23/2022 8:28 PM, Michael D Kinney wrote: > Hi Erich, > > One comment below. > > Mike > >> -----Original Message----- >> From: devel@edk2.groups.io On Behalf Of Michael Kubacki >> Sent: Wednesday, November 9, 2022 9:33 AM >> To: devel@edk2.groups.io >> Cc: Bi, Dandan ; Erich McMillan ; Wang, Jian J ; Gao, >> Liming ; Michael Kubacki ; Zeng, Star ; Gao, >> Zhichao ; Liu, Zhiguang ; Kubacki, Michael >> Subject: [edk2-devel] [PATCH v1 01/12] MdeModulePkg/SmbiosDxe: Fix pointer and buffer overflow CodeQL alerts >> >> From: Erich McMillan >> >> 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 >> Cc: Erich McMillan >> Cc: Jian J Wang >> Cc: Liming Gao >> Cc: Michael Kubacki >> Cc: Star Zeng >> Cc: Zhichao Gao >> Cc: Zhiguang Liu >> Co-authored-by: Michael Kubacki >> Signed-off-by: Erich McMillan >> --- >> 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 >> **/ >> >> #include "SmbiosDxe.h" >> +#include >> >> // >> // Module Global: >> @@ -1594,6 +1595,7 @@ ParseAndAddExistingSmbiosTable ( >> CHAR8 *String; >> EFI_SMBIOS_HANDLE SmbiosHandle; >> SMBIOS_STRUCTURE_POINTER SmbiosEnd; >> + UINTN SafeIntResult; >> >> mPrivateData.Smbios.MajorVersion = MajorVersion; >> mPrivateData.Smbios.MinorVersion = MinorVersion; >> @@ -1609,7 +1611,7 @@ ParseAndAddExistingSmbiosTable ( >> // Make sure not to access memory beyond SmbiosEnd >> // >> if ((Smbios.Raw + sizeof (SMBIOS_STRUCTURE) > SmbiosEnd.Raw) || > > 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)) >> + EFI_ERROR (SafeUintnAdd ((UINTN)Smbios.Raw, sizeof (SMBIOS_STRUCTURE), &SafeIntResult))) >> { >> return EFI_INVALID_PARAMETER; >> } >> diff --git a/MdeModulePkg/Universal/SmbiosDxe/SmbiosDxe.inf b/MdeModulePkg/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 >> >> [Protocols] >> gEfiSmbiosProtocolGuid ## PRODUCES >> -- >> 2.28.0.windows.1 >> >> >> >> -=-=-=-=-=-= >> Groups.io Links: You receive all messages sent to this group. >> View/Reply Online (#96147): https://edk2.groups.io/g/devel/message/96147 >> Mute This Topic: https://groups.io/mt/94918085/1643496 >> Group Owner: devel+owner@edk2.groups.io >> Unsubscribe: https://edk2.groups.io/g/devel/unsub [michael.d.kinney@intel.com] >> -=-=-=-=-=-= >> > > > > > >