[edk2-devel] [PATCH v2 1/1] MdePkg/BaseLib: Fix out-of-bounds reads in SafeString

[Pedro Falcato]

OpenCore folks established an ASAN-equipped project to fuzz Ext4Dxe,
which was able to catch these (mostly harmless) issues.

Signed-off-by: Pedro Falcato <pedro.falc...@gmail.com>
Cc: Vitaly Cheptsov <vit9...@protonmail.com>
Cc: Marvin Häuser <mhaeu...@posteo.de>
Cc: Michael D Kinney <michael.d.kin...@intel.com>
Cc: Liming Gao <gaolim...@byosoft.com.cn>
Cc: Zhiguang Liu <zhiguang....@intel.com>
---
 MdePkg/Library/BaseLib/SafeString.c | 24 ++++++++++++++++++++----
 1 file changed, 20 insertions(+), 4 deletions(-)

diff --git a/MdePkg/Library/BaseLib/SafeString.c 
b/MdePkg/Library/BaseLib/SafeString.c
index f338a32a3a41..77a2585ad56d 100644
--- a/MdePkg/Library/BaseLib/SafeString.c
+++ b/MdePkg/Library/BaseLib/SafeString.c
@@ -863,6 +863,9 @@ StrHexToUintnS (
   OUT       UINTN   *Data
   )
 {
+  BOOLEAN  FoundLeadingZero;
+
+  FoundLeadingZero = FALSE;
   ASSERT (((UINTN)String & BIT0) == 0);
 
   //
@@ -893,11 +896,12 @@ StrHexToUintnS (
   // Ignore leading Zeros after the spaces
   //
   while (*String == L'0') {
+    FoundLeadingZero = TRUE;
     String++;
   }
 
   if (CharToUpper (*String) == L'X') {
-    if (*(String - 1) != L'0') {
+    if (!FoundLeadingZero) {
       *Data = 0;
       return RETURN_SUCCESS;
     }
@@ -992,6 +996,9 @@ StrHexToUint64S (
   OUT       UINT64  *Data
   )
 {
+  BOOLEAN  FoundLeadingZero;
+
+  FoundLeadingZero = FALSE;
   ASSERT (((UINTN)String & BIT0) == 0);
 
   //
@@ -1022,11 +1029,12 @@ StrHexToUint64S (
   // Ignore leading Zeros after the spaces
   //
   while (*String == L'0') {
+    FoundLeadingZero = TRUE;
     String++;
   }
 
   if (CharToUpper (*String) == L'X') {
-    if (*(String - 1) != L'0') {
+    if (!FoundLeadingZero) {
       *Data = 0;
       return RETURN_SUCCESS;
     }
@@ -2393,6 +2401,9 @@ AsciiStrHexToUintnS (
   OUT       UINTN  *Data
   )
 {
+  BOOLEAN  FoundLeadingZero;
+
+  FoundLeadingZero = FALSE;
   //
   // 1. Neither String nor Data shall be a null pointer.
   //
@@ -2421,11 +2432,12 @@ AsciiStrHexToUintnS (
   // Ignore leading Zeros after the spaces
   //
   while (*String == '0') {
+    FoundLeadingZero = TRUE;
     String++;
   }
 
   if (AsciiCharToUpper (*String) == 'X') {
-    if (*(String - 1) != '0') {
+    if (!FoundLeadingZero) {
       *Data = 0;
       return RETURN_SUCCESS;
     }
@@ -2517,6 +2529,9 @@ AsciiStrHexToUint64S (
   OUT       UINT64  *Data
   )
 {
+  BOOLEAN  FoundLeadingZero;
+
+  FoundLeadingZero = FALSE;
   //
   // 1. Neither String nor Data shall be a null pointer.
   //
@@ -2545,11 +2560,12 @@ AsciiStrHexToUint64S (
   // Ignore leading Zeros after the spaces
   //
   while (*String == '0') {
+    FoundLeadingZero = TRUE;
     String++;
   }
 
   if (AsciiCharToUpper (*String) == 'X') {
-    if (*(String - 1) != '0') {
+    if (!FoundLeadingZero) {
       *Data = 0;
       return RETURN_SUCCESS;
     }
-- 
2.38.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#95535): https://edk2.groups.io/g/devel/message/95535
Mute This Topic: https://groups.io/mt/94546879/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-