From 461a59c408ea925656e1b951acbebb41f8e89633 Mon Sep 17 00:00:00 2001 From: Taylor Beebe <31827475+TaylorBeebe@users.noreply.github.com> Date: Mon, 23 Oct 2023 13:14:25 -0700 Subject: [PATCH] EsrtFmp CodeQL Fix (#598) ## Description Adds a check that PcdGetPtr returned a non-NULL value before calling into CompareGuid() - [x] Impacts functionality? - **Functionality** - Does the change ultimately impact how firmware functions? - Examples: Add a new library, publish a new PPI, update an algorithm, ... - [x] Impacts security? - **Security** - Does the change have a direct security impact on an application, flow, or firmware? - Examples: Crypto algorithm change, buffer overflow fix, parameter validation improvement, ... - [ ] Breaking change? - **Breaking change** - Will anyone consuming this change experience a break in build or boot behavior? - Examples: Add a new library class, move a module to a different repo, call a function in a new library class in a pre-existing module, ... - [ ] Includes tests? - **Tests** - Does the change include any explicit test code? - Examples: Unit tests, integration tests, robot tests, ... - [ ] Includes documentation? - **Documentation** - Does the change contain explicit documentation additions outside direct code modifications (and comments)? - Examples: Update readme file, add feature readme file, link to documentation on an a separate Web page, ... ## How This Was Tested Tested on Q35 ## Integration Instructions N/A --- MdeModulePkg/Universal/EsrtFmpDxe/EsrtFmp.c | 42 ++++++++++++--------- 1 file changed, 24 insertions(+), 18 deletions(-) diff --git a/MdeModulePkg/Universal/EsrtFmpDxe/EsrtFmp.c b/MdeModulePkg/Universal/EsrtFmpDxe/EsrtFmp.c index 62b72378db..1714d91bbf 100644 --- a/MdeModulePkg/Universal/EsrtFmpDxe/EsrtFmp.c +++ b/MdeModulePkg/Universal/EsrtFmpDxe/EsrtFmp.c @@ -103,6 +103,12 @@ IsSystemFmp ( Guid = PcdGetPtr (PcdSystemFmpCapsuleImageTypeIdGuid); Count = PcdGetSize (PcdSystemFmpCapsuleImageTypeIdGuid) / sizeof (GUID); + // MU_CHANGE [BEGIN] - CodeQL change + if (Guid == NULL) { + return FALSE; + } + + // MU_CHANGE [END] - CodeQL change for (Index = 0; Index < Count; Index++, Guid++) { if (CompareGuid (&FmpImageInfo->ImageTypeId, Guid)) { @@ -314,14 +320,14 @@ FmpGetFirmwareImageDescriptor ( ImageInfoSize = 0; Status = Fmp->GetImageInfo ( - Fmp, // FMP Pointer - &ImageInfoSize, // Buffer Size (in this case 0) - NULL, // NULL so we can get size - FmpImageInfoDescriptorVer, // DescriptorVersion - FmpImageInfoCount, // DescriptorCount - DescriptorSize, // DescriptorSize - &PackageVersion, // PackageVersion - &PackageVersionName // PackageVersionName + Fmp, // FMP Pointer + &ImageInfoSize, // Buffer Size (in this case 0) + NULL, // NULL so we can get size + FmpImageInfoDescriptorVer, // DescriptorVersion + FmpImageInfoCount, // DescriptorCount + DescriptorSize, // DescriptorSize + &PackageVersion, // PackageVersion + &PackageVersionName // PackageVersionName ); if (Status != EFI_BUFFER_TOO_SMALL) { DEBUG ((DEBUG_ERROR, "EsrtFmpDxe: Unexpected Failure in GetImageInfo. Status = %r\n", Status)); @@ -336,14 +342,14 @@ FmpGetFirmwareImageDescriptor ( PackageVersionName = NULL; Status = Fmp->GetImageInfo ( - Fmp, // FMP Pointer - &ImageInfoSize, // ImageInfoSize - FmpImageInfoBuf, // ImageInfo - FmpImageInfoDescriptorVer, // DescriptorVersion - FmpImageInfoCount, // DescriptorCount - DescriptorSize, // DescriptorSize - &PackageVersion, // PackageVersion - &PackageVersionName // PackageVersionName + Fmp, // FMP Pointer + &ImageInfoSize, // ImageInfoSize + FmpImageInfoBuf, // ImageInfo + FmpImageInfoDescriptorVer, // DescriptorVersion + FmpImageInfoCount, // DescriptorCount + DescriptorSize, // DescriptorSize + &PackageVersion, // PackageVersion + &PackageVersionName // PackageVersionName ); if (PackageVersionName != NULL) { FreePool (PackageVersionName); @@ -505,7 +511,7 @@ EsrtReadyToBootEventNotify ( EFI_STATUS Status; EFI_SYSTEM_RESOURCE_TABLE *Table; - PERF_CALLBACK_BEGIN (&gEfiEventReadyToBootGuid); // MU_CHANGE + PERF_CALLBACK_BEGIN (&gEfiEventReadyToBootGuid); // MU_CHANGE Table = CreateFmpBasedEsrt (); if (Table != NULL) { @@ -529,7 +535,7 @@ EsrtReadyToBootEventNotify ( // gBS->CloseEvent (Event); - PERF_CALLBACK_END (&gEfiEventReadyToBootGuid); // MU_CHANGE + PERF_CALLBACK_END (&gEfiEventReadyToBootGuid); // MU_CHANGE } /**