From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from a7-18.smtp-out.eu-west-1.amazonses.com (a7-18.smtp-out.eu-west-1.amazonses.com [54.240.7.18]) by mx.groups.io with SMTP id smtpd.web11.18815.1676304932481357436 for ; Mon, 13 Feb 2023 08:15:32 -0800 Authentication-Results: mx.groups.io; dkim=pass header.i=@ipxe.org header.s=cphpx6z2rfcgehlykjjh3gknqe3hsoe2 header.b=hBpxpJuE; spf=pass (domain: eu-west-1.amazonses.com, ip: 54.240.7.18, mailfrom: 010201864b8f56cb-c9b052f6-c9e6-4c22-9d99-c87c947a7169-000000@eu-west-1.amazonses.com) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/simple; s=cphpx6z2rfcgehlykjjh3gknqe3hsoe2; d=ipxe.org; t=1676304930; h=Message-ID:Date:MIME-Version:To:Cc:References:From:Subject:In-Reply-To:Content-Type:Content-Transfer-Encoding; bh=WWuQxQa12i+9iL1lhqf5pQiN1YaYybn3zFml9ErYTlo=; b=hBpxpJuEl0wvC8CXXpDWqdX99Q5Jqv7+dMWrdFGBzLw0WYphztpLHPemwJmRsIQ3 dKsZIQDC/vzbKy0V+RJFG4kwg+pxWBTGGvNHBvOYPJVwbnIw4x6Dkz7P4Grqi9QcvOe N6TwnuXpV0ujXAqZSdBvHuEDz7ucAadSxxbT+07kUbLpSqn7UXj2nnXb+6JjUTp7pV5 9GgDVG36zFt3E1xc3E/rzF4M+Sky0O4Eecj5zuURWiD5bpXUxI0Q6yO4df59MDHO0Ua RorCcNIdDGr7HrcR3GLDPm04z5VHoPMHl0P7awAaILBtteqlzOOjKuasEx+MpjX/rC+ rYxiA5pP0Q== DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/simple; s=ihchhvubuqgjsxyuhssfvqohv7z3u4hn; d=amazonses.com; t=1676304930; h=Message-ID:Date:MIME-Version:To:Cc:References:From:Subject:In-Reply-To:Content-Type:Content-Transfer-Encoding:Feedback-ID; bh=WWuQxQa12i+9iL1lhqf5pQiN1YaYybn3zFml9ErYTlo=; b=C0RRgquLnOqfdWJpQeFvvkEKNm70pyoQKZbQzpOFdbYvotYy1BaJCdT3zKLy0MAC RYOrOf2ALXEPgmqIRRGiqOqS1E07xovcp+g7zFD6V5f0rGgvtomm8FIMYmq3vt0fUNW 2x2RuCOlS90PbRBY3TLhL5T71gF9nzsT2TmVixq0= Message-ID: <010201864b8f56cb-c9b052f6-c9e6-4c22-9d99-c87c947a7169-000000@eu-west-1.amazonses.com> Date: Mon, 13 Feb 2023 16:15:30 +0000 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.6.0 To: devel@edk2.groups.io, mikuback@linux.microsoft.com Cc: Dandan Bi , Erich McMillan , Jian J Wang , Liming Gao , Star Zeng , Zhichao Gao , Zhiguang Liu , Michael Kubacki References: <20230213154908.1993-1-mikuback@linux.microsoft.com> <20230213154908.1993-2-mikuback@linux.microsoft.com> From: "Michael Brown" Subject: Re: [edk2-devel] [PATCH v3 01/12] MdeModulePkg/SmbiosDxe: Fix pointer and buffer overflow CodeQL alerts In-Reply-To: <20230213154908.1993-2-mikuback@linux.microsoft.com> X-Spam-Status: No, score=-2.9 required=5.0 tests=ALL_TRUSTED,BAYES_00 autolearn=ham autolearn_force=no version=3.4.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on blyat.fensystems.co.uk Feedback-ID: 1.eu-west-1.fspj4M/5bzJ9NLRzJP0PaxRwxrpZqiDQJ1IF94CF2TA=:AmazonSES X-SES-Outgoing: 2023.02.13-54.240.7.18 Content-Language: en-US Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit On 13/02/2023 15:48, Michael Kubacki wrote: > @@ -1608,9 +1610,12 @@ ParseAndAddExistingSmbiosTable ( > // > // Make sure not to access memory beyond SmbiosEnd > // > - if ((Smbios.Raw + sizeof (SMBIOS_STRUCTURE) > SmbiosEnd.Raw) || > - (Smbios.Raw + sizeof (SMBIOS_STRUCTURE) < Smbios.Raw)) > - { > + Status = SafeUintnAdd ((UINTN)Smbios.Raw, sizeof (SMBIOS_STRUCTURE), &SafeIntResult); > + if (EFI_ERROR (Status)) { > + return EFI_INVALID_PARAMETER; > + } > + > + if ((SafeIntResult > (UINTN)SmbiosEnd.Raw) || (SafeIntResult < (UINTN)Smbios.Raw)) { > return EFI_INVALID_PARAMETER; > } Could this not be expressed much more cleanly as just: if ((UINTN)(SmbiosEnd.Raw - Smbios.Raw) < sizeof (SMBIOS_STRUCTURE)) { return EFI_INVALID_PARAMETER; } without the need to hide a basic arithmetic operation behind an ugly wrapper function and drag in an external library? The almost-identical check performed in the code immediately below (that takes Smbios.Hdr->Length into account) should also be updated to e.g.: if ((UINTN)(SmbiosEnd.Raw - Smbios.Raw) < (Smbios.Hdr->Length + 2)) { ... } Thanks, Michael