[edk2] [PATCH v2 2/5] IntelFrameworkModulePkg BdsDxe: Fix ASSERT in BdsMemoryTest

[Hao Wu]

This commit will resolve the issue brought by r17735.

StrCatS (StrPercent, sizeof (StrPercent) / sizeof (CHAR16), TmpStr);

The above using of StrCatS will cause ASSERT if TmpStr is longer than
StrPercent. Therefore, StrnCatS is used here to resolve the issue.

Similar scenario is for:
StrCatS (StrTotalMemory, StrTotalMemorySize / sizeof (CHAR16), TmpStr);

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Hao Wu <***@intel.com>
Reviewed-by: Qiu Shumin <***@intel.com>
Reviewed-by: Jeff Fan <***@intel.com>
---
IntelFrameworkModulePkg/Universal/BdsDxe/MemoryTest.c | 14 ++++++++++++--
1 file changed, 12 insertions(+), 2 deletions(-)

diff --git a/IntelFrameworkModulePkg/Universal/BdsDxe/MemoryTest.c b/IntelFrameworkModulePkg/Universal/BdsDxe/MemoryTest.c
index eef840b..fedb151 100644
--- a/IntelFrameworkModulePkg/Universal/BdsDxe/MemoryTest.c
+++ b/IntelFrameworkModulePkg/Universal/BdsDxe/MemoryTest.c
@@ -324,7 +324,12 @@ BdsMemoryTest (
//
// TmpStr size is 64, StrPercent is reserved to 16.
//
- StrCatS (StrPercent, sizeof (StrPercent) / sizeof (CHAR16), TmpStr);
+ StrnCatS (
+ StrPercent,
+ sizeof (StrPercent) / sizeof (CHAR16),
+ TmpStr,
+ sizeof (StrPercent) / sizeof (CHAR16) - StrLen (StrPercent) - 1
+ );
PrintXY (10, 10, NULL, NULL, StrPercent);
FreePool (TmpStr);
}
@@ -389,7 +394,12 @@ Done:

TmpStr = GetStringById (STRING_TOKEN (STR_MEM_TEST_COMPLETED));
if (TmpStr != NULL) {
- StrCatS (StrTotalMemory, StrTotalMemorySize / sizeof (CHAR16), TmpStr);
+ StrnCatS (
+ StrTotalMemory,
+ StrTotalMemorySize / sizeof (CHAR16),
+ TmpStr,
+ StrTotalMemorySize / sizeof (CHAR16) - StrLen (StrTotalMemory) - 1
+ );
FreePool (TmpStr);
}

[Hao Wu]

This commit will resolve the issue brought by r17736.

Str = AllocateCopyPool (MaxLen * sizeof (CHAR16), Str1);

The above using of AllocateCopyPool() will read contents out of the scope
of Str1. Potential risk for Str1 allocated at the boundary of memory
region.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Hao Wu <***@intel.com>
Reviewed-by: Qiu Shumin <***@intel.com>
Reviewed-by: Jeff Fan <***@intel.com>
---
IntelFrameworkModulePkg/Universal/BdsDxe/BootMaint/BootOption.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/IntelFrameworkModulePkg/Universal/BdsDxe/BootMaint/BootOption.c b/IntelFrameworkModulePkg/Universal/BdsDxe/BootMaint/BootOption.c
index 1519315..56bcfab 100644
--- a/IntelFrameworkModulePkg/Universal/BdsDxe/BootMaint/BootOption.c
+++ b/IntelFrameworkModulePkg/Universal/BdsDxe/BootMaint/BootOption.c
@@ -1096,12 +1096,13 @@ BOpt_AppendFileName (
Size1 = StrSize (Str1);
Size2 = StrSize (Str2);
MaxLen = (Size1 + Size2 + sizeof (CHAR16)) / sizeof (CHAR16);
- Str = AllocateCopyPool (MaxLen * sizeof (CHAR16), Str1);
+ Str = AllocateZeroPool (MaxLen * sizeof (CHAR16));
ASSERT (Str != NULL);

TmpStr = AllocateZeroPool (MaxLen * sizeof (CHAR16));
ASSERT (TmpStr != NULL);

+ StrCatS (Str, MaxLen, Str1);
if (!((*Str == '\\') && (*(Str + 1) == 0))) {
StrCatS (Str, MaxLen, L"\\");
}

[Hao Wu]

This commit will resolve the issue brought by r17733.

StringBuffer1 = AllocateCopyPool (
MAX_STRING_LEN * sizeof (CHAR16),
L"Configuration changed. Reset to apply it Now."
);

The above using of AllocateCopyPool() will read contents out of the scope
of the constant string. Potential risk for the constant string allocated
at the boundary of memory region.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Hao Wu <***@intel.com>
Reviewed-by: Qiu Shumin <***@intel.com>
Reviewed-by: Jeff Fan <***@intel.com>
---
.../Library/GenericBdsLib/BdsMisc.c | 20 ++++++++++++--------
1 file changed, 12 insertions(+), 8 deletions(-)

diff --git a/IntelFrameworkModulePkg/Library/GenericBdsLib/BdsMisc.c b/IntelFrameworkModulePkg/Library/GenericBdsLib/BdsMisc.c
index b5be631..24c1998 100644
--- a/IntelFrameworkModulePkg/Library/GenericBdsLib/BdsMisc.c
+++ b/IntelFrameworkModulePkg/Library/GenericBdsLib/BdsMisc.c
@@ -1127,16 +1127,20 @@ SetupResetReminder (
if (IsResetReminderFeatureEnable ()) {
if (IsResetRequired ()) {

- StringBuffer1 = AllocateCopyPool (
- MAX_STRING_LEN * sizeof (CHAR16),
- L"Configuration changed. Reset to apply it Now."
- );
+ StringBuffer1 = AllocateZeroPool (MAX_STRING_LEN * sizeof (CHAR16));
ASSERT (StringBuffer1 != NULL);
- StringBuffer2 = AllocateCopyPool (
- MAX_STRING_LEN * sizeof (CHAR16),
- L"Press ENTER to reset"
- );
+ StringBuffer2 = AllocateZeroPool (MAX_STRING_LEN * sizeof (CHAR16));
ASSERT (StringBuffer2 != NULL);
+ StrCpyS (
+ StringBuffer1,
+ MAX_STRING_LEN,
+ L"Configuration changed. Reset to apply it Now."
+ );
+ StrCpyS (
+ StringBuffer2,
+ MAX_STRING_LEN,
+ L"Press ENTER to reset"
+ );
//
// Popup a menu to notice user
//

[Hao Wu]

This commit will resolve the issue brought by r17737.

HelpString = AllocateCopyPool (HelpSize, L"Device Path : ");

The above using of AllocateCopyPool() will read contents out of the scope
of the constant string. Potential risk for the constant string allocated
at the boundary of memory region.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Hao Wu <***@intel.com>
Reviewed-by: Qiu Shumin <***@intel.com>
Reviewed-by: Jeff Fan <***@intel.com>
---
IntelFrameworkModulePkg/Universal/BdsDxe/BootMngr/BootManager.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/IntelFrameworkModulePkg/Universal/BdsDxe/BootMngr/BootManager.c b/IntelFrameworkModulePkg/Universal/BdsDxe/BootMngr/BootManager.c
index 978959d..6efd783 100644
--- a/IntelFrameworkModulePkg/Universal/BdsDxe/BootMngr/BootManager.c
+++ b/IntelFrameworkModulePkg/Universal/BdsDxe/BootMngr/BootManager.c
@@ -319,8 +319,9 @@ CallBootManager (

TempStr = DevicePathToStr (Option->DevicePath);
HelpSize = StrSize (TempStr) + StrSize (L"Device Path : ");
- HelpString = AllocateCopyPool (HelpSize, L"Device Path : ");
+ HelpString = AllocateZeroPool (HelpSize);
ASSERT (HelpString != NULL);
+ StrCatS (HelpString, HelpSize / sizeof (CHAR16), L"Device Path : ");
StrCatS (HelpString, HelpSize / sizeof (CHAR16), TempStr);

HelpToken = HiiSetString (HiiHandle, 0, HelpString, NULL);

[Hao Wu]

This post might be inappropriate. Click to display it.