MdeModulePkg: More CodeQL fixes (#319)

## Description Various fixes - [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 Build and boot changes on QemuQ35Pkg to EFI shell. ## Integration Instructions N/A
microsoft · May 9, 2023 · bf3dfbe · bf3dfbe
1 parent d859369
commit bf3dfbe
Show file tree

Hide file tree

Showing 30 changed files with 938 additions and 118 deletions.
diff --git a/MdeModulePkg/Bus/Pci/PciBusDxe/PciResourceSupport.c b/MdeModulePkg/Bus/Pci/PciBusDxe/PciResourceSupport.c
@@ -835,6 +835,35 @@ CreateResourceMap (
                        PciResUsageTypical
                        );
 
+      // MU_CHANGE [BEGIN] - CodeQL change
+      if ((IoBridge == NULL) || (Mem32Bridge == NULL) || (PMem32Bridge == NULL) ||
+          (Mem64Bridge == NULL) || (PMem64Bridge == NULL))
+      {
+        if (IoBridge != NULL) {
+          FreePool (IoBridge);
+        }
+
+        if (Mem32Bridge != NULL) {
+          FreePool (Mem32Bridge);
+        }
+
+        if (PMem32Bridge != NULL) {
+          FreePool (PMem32Bridge);
+        }
+
+        if (Mem64Bridge != NULL) {
+          FreePool (Mem64Bridge);
+        }
+
+        if (PMem64Bridge != NULL) {
+          FreePool (PMem64Bridge);
+        }
+
+        return;
+      }
+
+      // MU_CHANGE [END] - CodeQL change
+
       //
       // Recursively create resource map on this bridge
       //

diff --git a/MdeModulePkg/Bus/Usb/UsbMouseAbsolutePointerDxe/UsbMouseAbsolutePointer.c b/MdeModulePkg/Bus/Usb/UsbMouseAbsolutePointerDxe/UsbMouseAbsolutePointer.c
@@ -160,7 +160,14 @@ USBMouseAbsolutePointerDriverBindingStart (
   }
 
   UsbMouseAbsolutePointerDevice = AllocateZeroPool (sizeof (USB_MOUSE_ABSOLUTE_POINTER_DEV));
-  ASSERT (UsbMouseAbsolutePointerDevice != NULL);
+  // MU_CHANGE [BEGIN] - CodeQL change
+  if (UsbMouseAbsolutePointerDevice == NULL) {
+    ASSERT (UsbMouseAbsolutePointerDevice != NULL);
+    Status = EFI_OUT_OF_RESOURCES;
+    goto ErrorExit;
+  }
+
+  // MU_CHANGE [END] - CodeQL change
 
   UsbMouseAbsolutePointerDevice->UsbIo     = UsbIo;
   UsbMouseAbsolutePointerDevice->Signature = USB_MOUSE_ABSOLUTE_POINTER_DEV_SIGNATURE;
@@ -631,7 +638,14 @@ InitializeUsbMouseDevice (
   }
 
   ReportDesc = AllocateZeroPool (MouseHidDesc->HidClassDesc[0].DescriptorLength);
-  ASSERT (ReportDesc != NULL);
+  // MU_CHANGE [BEGIN] - CodeQL change
+  if (ReportDesc == NULL) {
+    ASSERT (ReportDesc != NULL);
+    FreePool (Buf);
+    return EFI_OUT_OF_RESOURCES;
+  }
+
+  // MU_CHANGE [END] - CodeQL change
 
   Status = UsbGetReportDescriptor (
              UsbIo,

diff --git a/MdeModulePkg/Core/Dxe/Dispatcher/Dispatcher.c b/MdeModulePkg/Core/Dxe/Dispatcher/Dispatcher.c
@@ -775,7 +775,13 @@ FvIsBeingProcessed (
   }
 
   KnownHandle = AllocateZeroPool (sizeof (KNOWN_HANDLE));
-  ASSERT (KnownHandle != NULL);
+  // MU_CHANGE [BEGIN] - CodeQL change
+  if (KnownHandle == NULL) {
+    ASSERT (KnownHandle != NULL);
+    return NULL;
+  }
+
+  // MU_CHANGE [END] - CodeQL change
 
   KnownHandle->Signature = KNOWN_HANDLE_SIGNATURE;
   KnownHandle->Handle    = FvHandle;
@@ -852,6 +858,7 @@ CoreFvToDevicePath (
   @retval EFI_ALREADY_STARTED   The driver has already been started. Only one
                                 DriverName may be active in the system at any one
                                 time.
+  @retval EFI_OUT_OF_RESOURCES  If memory could not be allocated for the DriverEntry. // MU_CHANGE - CodeQL change
 
 **/
 EFI_STATUS
@@ -869,7 +876,13 @@ CoreAddToDriverList (
   // NULL or FALSE.
   //
   DriverEntry = AllocateZeroPool (sizeof (EFI_CORE_DRIVER_ENTRY));
-  ASSERT (DriverEntry != NULL);
+  // MU_CHANGE [BEGIN] - CodeQL change
+  if (DriverEntry == NULL) {
+    ASSERT (DriverEntry != NULL);
+    return EFI_OUT_OF_RESOURCES;
+  }
+
+  // MU_CHANGE [END] - CodeQL change
   if (Type == EFI_FV_FILETYPE_FIRMWARE_VOLUME_IMAGE) {
     DriverEntry->IsFvImage = TRUE;
   }

diff --git a/MdeModulePkg/Core/Dxe/Image/Image.c b/MdeModulePkg/Core/Dxe/Image/Image.c
@@ -160,9 +160,14 @@ PeCoffEmuProtocolNotify (
       continue;
     }
 
+    // MU_CHANGE [BEGIN] - CodeQL change
     Entry = AllocateZeroPool (sizeof (*Entry));
-    ASSERT (Entry != NULL);
+    if (Entry == NULL) {
+      ASSERT (Entry != NULL);
+      break;
+    }
 
+    // MU_CHANGE [END] - CodeQL change
     Entry->Emulator    = Emulator;
     Entry->MachineType = Entry->Emulator->MachineType;
 

diff --git a/MdeModulePkg/Core/Dxe/Misc/MemoryAttributesTable.c b/MdeModulePkg/Core/Dxe/Misc/MemoryAttributesTable.c
@@ -147,7 +147,13 @@ InstallMemoryAttributesTable (
 
   do {
     MemoryMap = AllocatePool (MemoryMapSize);
-    ASSERT (MemoryMap != NULL);
+    // MU_CHANGE [BEGIN] - CodeQL change
+    if (MemoryMap == NULL) {
+      ASSERT (MemoryMap != NULL);
+      return;
+    }
+
+    // MU_CHANGE [END] - CodeQL change
 
     Status = CoreGetMemoryMapWithSeparatedImageSection (
                &MemoryMapSize,
@@ -178,7 +184,14 @@ InstallMemoryAttributesTable (
   // Allocate MemoryAttributesTable
   //
   MemoryAttributesTable = AllocatePool (sizeof (EFI_MEMORY_ATTRIBUTES_TABLE) + DescriptorSize * RuntimeEntryCount);
-  ASSERT (MemoryAttributesTable != NULL);
+  // MU_CHANGE [BEGIN] - CodeQL change
+  if (MemoryAttributesTable == NULL) {
+    ASSERT (MemoryAttributesTable != NULL);
+    FreePool (MemoryMapStart);
+    return;
+  }
+
+  // MU_CHANGE [END] - CodeQL change
   MemoryAttributesTable->Version         = EFI_MEMORY_ATTRIBUTES_TABLE_VERSION;
   MemoryAttributesTable->NumberOfEntries = RuntimeEntryCount;
   MemoryAttributesTable->DescriptorSize  = (UINT32)DescriptorSize;

diff --git a/MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c b/MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c
@@ -880,7 +880,13 @@ InitializeDxeNxMemoryProtectionPolicy (
   ASSERT (Status == EFI_BUFFER_TOO_SMALL);
   do {
     MemoryMap = (EFI_MEMORY_DESCRIPTOR *)AllocatePool (MemoryMapSize);
-    ASSERT (MemoryMap != NULL);
+    // MU_CHANGE [BEGIN] - CodeQL change
+    if (MemoryMap == NULL) {
+      ASSERT (MemoryMap != NULL);
+      return;
+    }
+
+    // MU_CHANGE [END] - CodeQL change
     Status = gBS->GetMemoryMap (
                     &MemoryMapSize,
                     MemoryMap,

diff --git a/MdeModulePkg/Core/Dxe/SectionExtraction/CoreSectionExtraction.c b/MdeModulePkg/Core/Dxe/SectionExtraction/CoreSectionExtraction.c
@@ -626,7 +626,13 @@ CreateGuidedExtractionRpnEvent (
   // Allocate new event structure and context
   //
   Context = AllocatePool (sizeof (RPN_EVENT_CONTEXT));
-  ASSERT (Context != NULL);
+  // MU_CHANGE [BEGIN] - CodeQL change
+  if (Context == NULL) {
+    ASSERT (Context != NULL);
+    return;
+  }
+
+  // MU_CHANGE [END] - CodeQL change
 
   Context->ChildNode    = ChildNode;
   Context->ParentStream = ParentStream;

diff --git a/MdeModulePkg/Library/DxeCapsuleLibFmp/CapsuleOnDisk.c b/MdeModulePkg/Library/DxeCapsuleLibFmp/CapsuleOnDisk.c
@@ -301,7 +301,8 @@ GetBootOptionInOrder (
   // Second get BootOption from "BootOrder"
   //
   BootOrderOptionBuf = EfiBootManagerGetLoadOptions (&BootOrderCount, LoadOptionTypeBoot);
-  if ((BootNextCount == 0) && (BootOrderCount == 0)) {
+  // MU_CHANGE - CodeQL change
+  if (((BootNextCount == 0) && (BootOrderCount == 0)) || (BootOrderOptionBuf == NULL)) {
     return EFI_NOT_FOUND;
   }
 

diff --git a/MdeModulePkg/Library/UefiBootManagerLib/BmBootDescription.c b/MdeModulePkg/Library/UefiBootManagerLib/BmBootDescription.c
@@ -175,7 +175,13 @@ BmGetDescriptionFromDiskInfo (
                              );
     if (!EFI_ERROR (Status)) {
       Description = AllocateZeroPool ((ModelNameLength + SerialNumberLength + 2) * sizeof (CHAR16));
-      ASSERT (Description != NULL);
+      // MU_CHANGE [BEGIN] - CodeQL change
+      if (Description == NULL) {
+        ASSERT (Description != NULL);
+        return NULL;
+      }
+
+      // MU_CHANGE [END] - CodeQL change
       for (Index = 0; Index + 1 < ModelNameLength; Index += 2) {
         Description[Index]     = (CHAR16)IdentifyData.ModelName[Index + 1];
         Description[Index + 1] = (CHAR16)IdentifyData.ModelName[Index];
@@ -206,7 +212,13 @@ BmGetDescriptionFromDiskInfo (
                              );
     if (!EFI_ERROR (Status)) {
       Description = AllocateZeroPool ((VENDOR_IDENTIFICATION_LENGTH + PRODUCT_IDENTIFICATION_LENGTH + 2) * sizeof (CHAR16));
-      ASSERT (Description != NULL);
+      // MU_CHANGE [BEGIN] - CodeQL change
+      if (Description == NULL) {
+        ASSERT (Description != NULL);
+        return NULL;
+      }
+
+      // MU_CHANGE [END] - CodeQL change
 
       //
       // Per SCSI spec, EFI_SCSI_INQUIRY_DATA.Reserved_5_95[3 - 10] save the Verdor identification
@@ -336,7 +348,13 @@ BmGetUsbDescription (
 
   DescMaxSize = StrSize (Manufacturer) + StrSize (Product) + StrSize (SerialNumber);
   Description = AllocateZeroPool (DescMaxSize);
-  ASSERT (Description != NULL);
+  // MU_CHANGE [BEGIN] - CodeQL change
+  if (Description == NULL) {
+    ASSERT (Description != NULL);
+    return NULL;
+  }
+
+  // MU_CHANGE [END] - CodeQL change
   StrCatS (Description, DescMaxSize/sizeof (CHAR16), Manufacturer);
   StrCatS (Description, DescMaxSize/sizeof (CHAR16), L" ");
 
@@ -890,7 +908,13 @@ BmMakeBootOptionDescriptionUnique (
   }
 
   Visited = AllocateZeroPool (sizeof (BOOLEAN) * BootOptionCount);
-  ASSERT (Visited != NULL);
+  // MU_CHANGE [BEGIN] - CodeQL change
+  if (Visited == NULL) {
+    ASSERT (Visited != NULL);
+    return;
+  }
+
+  // MU_CHANGE [END] - CodeQL change
 
   for (Base = 0; Base < BootOptionCount; Base++) {
     if (!Visited[Base]) {

diff --git a/MdeModulePkg/Library/UefiBootManagerLib/BmDriverHealth.c b/MdeModulePkg/Library/UefiBootManagerLib/BmDriverHealth.c
@@ -130,7 +130,12 @@ BmDisplayMessages (
                      DriverHealthInfo->ControllerHandle,
                      DriverHealthInfo->ChildHandle
                      );
+  // MU_CHANGE [BEGIN] - CodeQL change
+  if (ControllerName == NULL) {
+    return;
+  }
 
+  // MU_CHANGE [END] - CodeQL change
   DEBUG ((DEBUG_INFO, "Controller: %s\n", ControllerName));
   Print (L"Controller: %s\n", ControllerName);
   for (Index = 0; DriverHealthInfo->MessageList[Index].HiiHandle != NULL; Index++) {

diff --git a/MdeModulePkg/Library/UefiBootManagerLib/BmHotkey.c b/MdeModulePkg/Library/UefiBootManagerLib/BmHotkey.c
@@ -689,7 +689,19 @@ BmProcessKeyOption (
 
   for (Index = 0; Index < KeyShiftStateCount; Index++) {
     Hotkey = AllocateZeroPool (sizeof (BM_HOTKEY));
-    ASSERT (Hotkey != NULL);
+    // MU_CHANGE [BEGIN] - CodeQL change
+    if (Hotkey == NULL) {
+      ASSERT (Hotkey != NULL);
+      if (Handles != NULL) {
+        FreePool (Handles);
+      }
+
+      EfiReleaseLock (&mBmHotkeyLock);
+
+      return EFI_OUT_OF_RESOURCES;
+    }
+
+    // MU_CHANGE [END] - CodeQL change
 
     Hotkey->Signature  = BM_HOTKEY_SIGNATURE;
     Hotkey->BootOption = KeyOption->BootOption;

diff --git a/MdeModulePkg/Library/UefiBootManagerLib/BmLoadOption.c b/MdeModulePkg/Library/UefiBootManagerLib/BmLoadOption.c
@@ -47,7 +47,13 @@ BmForEachVariable (
 
   NameSize = sizeof (CHAR16);
   Name     = AllocateZeroPool (NameSize);
-  ASSERT (Name != NULL);
+  // MU_CHANGE [BEGIN] - CodeQL change
+  if (Name == NULL) {
+    ASSERT (Name != NULL);
+    return;
+  }
+
+  // MU_CHANGE [END] - CodeQL change
   while (TRUE) {
     NewNameSize = NameSize;
     Status      = gRT->GetNextVariableName (&NewNameSize, Name, &Guid);