From 476b78bbad7e2c126002a7dde332dbf67ae7b172 Mon Sep 17 00:00:00 2001 From: Aaron Pop Date: Thu, 23 Oct 2025 15:23:51 -0700 Subject: [PATCH] MdeModulePkg: Fix Comparison overflow https://github.com/github/codeql/blob/codeql-cli-2.7.3/cpp/ql/src/Security/CWE/CWE-190/ComparisonWithWiderType.qhelp Switch to using SafeUint16Add for calculating offsets into block data. The data being used in the calculation comes from config block strings, and there is no validation of the values before the calculation occurs. Signed-off-by: Aaron Pop --- MdeModulePkg/Library/UefiHiiLib/HiiLib.c | 24 +++++++++++++------ .../Library/UefiHiiLib/UefiHiiLib.inf | 1 + 2 files changed, 18 insertions(+), 7 deletions(-) diff --git a/MdeModulePkg/Library/UefiHiiLib/HiiLib.c b/MdeModulePkg/Library/UefiHiiLib/HiiLib.c index 7625fe8e3d..476b69eec3 100644 --- a/MdeModulePkg/Library/UefiHiiLib/HiiLib.c +++ b/MdeModulePkg/Library/UefiHiiLib/HiiLib.c @@ -8,6 +8,8 @@ #include "InternalHiiLib.h" +#include + #define GUID_CONFIG_STRING_TYPE 0x00 #define NAME_CONFIG_STRING_TYPE 0x01 #define PATH_CONFIG_STRING_TYPE 0x02 @@ -1948,6 +1950,8 @@ GetBlockDataInfo ( EFI_STATUS Status; IFR_BLOCK_DATA *BlockArray; UINT8 *DataBuffer; + UINT16 Sum1; + UINT16 Sum2; // // Initialize the local variables. @@ -2144,14 +2148,20 @@ GetBlockDataInfo ( while ((Link != &BlockArray->Entry) && (Link->ForwardLink != &BlockArray->Entry)) { BlockData = BASE_CR (Link, IFR_BLOCK_DATA, Entry); NewBlockData = BASE_CR (Link->ForwardLink, IFR_BLOCK_DATA, Entry); - if ((NewBlockData->Offset >= BlockData->Offset) && (NewBlockData->Offset <= (BlockData->Offset + BlockData->Width))) { - if ((NewBlockData->Offset + NewBlockData->Width) > (BlockData->Offset + BlockData->Width)) { - BlockData->Width = (UINT16)(NewBlockData->Offset + NewBlockData->Width - BlockData->Offset); + if ((!EFI_ERROR (SafeUint16Add (BlockData->Offset, BlockData->Width, &Sum1))) && + (!EFI_ERROR (SafeUint16Add (NewBlockData->Offset, NewBlockData->Width, &Sum2))) && + (NewBlockData->Offset >= BlockData->Offset) && + (NewBlockData->Offset <= Sum1) && + (Sum2 > Sum1)) + { + Sum1 = BlockData->Width; + if (!EFI_ERROR (SafeUint16Sub (Sum2, BlockData->Offset, &BlockData->Width))) { + RemoveEntryList (Link->ForwardLink); + FreePool (NewBlockData); + continue; + } else { + BlockData->Width = Sum1; } - - RemoveEntryList (Link->ForwardLink); - FreePool (NewBlockData); - continue; } Link = Link->ForwardLink; diff --git a/MdeModulePkg/Library/UefiHiiLib/UefiHiiLib.inf b/MdeModulePkg/Library/UefiHiiLib/UefiHiiLib.inf index d432b439bc..be0c417578 100644 --- a/MdeModulePkg/Library/UefiHiiLib/UefiHiiLib.inf +++ b/MdeModulePkg/Library/UefiHiiLib/UefiHiiLib.inf @@ -41,6 +41,7 @@ UefiLib UefiHiiServicesLib PrintLib + SafeIntLib [Protocols] gEfiFormBrowser2ProtocolGuid ## SOMETIMES_CONSUMES