MdePkg/BaseLib: Fix out-of-bounds reads in SafeString

There was a OOB access in *StrHexTo* functions, when passed strings like
"XDEADBEEF".

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

Cc: Vitaly Cheptsov <vit9696@protonmail.com>
Cc: Marvin H?user <mhaeuser@posteo.de>
Cc: Michael D Kinney <michael.d.kinney@intel.com>
Cc: Liming Gao <gaoliming@byosoft.com.cn>
Cc: Zhiguang Liu <zhiguang.liu@intel.com>
Signed-off-by: Pedro Falcato <pedro.falcato@gmail.com>
Acked-by: Michael D Kinney <michael.d.kinney@intel.com>
Reviewed-by: Jiewen Yao <Jiewen.yao@Intel.com>
Reviewed-by: Liming Gao <gaoliming@byosoft.com.cn>

diff --git a/MdePkg/Library/BaseLib/SafeString.c b/MdePkg/Library/BaseLib/SafeString.c
index f338a32a3a415ece743fd6ee00c3152980d65919..b75b33381732a3c6b7998218bd896d2e8dcf9194 100644 (file)
--- a/MdePkg/Library/BaseLib/SafeString.c
+++ b/MdePkg/Library/BaseLib/SafeString.c
@@ -863,6 +863,9 @@ StrHexToUintnS (
   OUT       UINTN   *Data\r
   )\r
 {\r
+  BOOLEAN  FoundLeadingZero;\r
+\r
+  FoundLeadingZero = FALSE;\r
   ASSERT (((UINTN)String & BIT0) == 0);\r
 \r
   //\r
@@ -892,12 +895,14 @@ StrHexToUintnS (
   //\r
   // Ignore leading Zeros after the spaces\r
   //\r
+\r
+  FoundLeadingZero = *String == L'0';\r
   while (*String == L'0') {\r
     String++;\r
   }\r
 \r
   if (CharToUpper (*String) == L'X') {\r
-    if (*(String - 1) != L'0') {\r
+    if (!FoundLeadingZero) {\r
       *Data = 0;\r
       return RETURN_SUCCESS;\r
     }\r
@@ -992,6 +997,9 @@ StrHexToUint64S (
   OUT       UINT64  *Data\r
   )\r
 {\r
+  BOOLEAN  FoundLeadingZero;\r
+\r
+  FoundLeadingZero = FALSE;\r
   ASSERT (((UINTN)String & BIT0) == 0);\r
 \r
   //\r
@@ -1021,12 +1029,13 @@ StrHexToUint64S (
   //\r
   // Ignore leading Zeros after the spaces\r
   //\r
+  FoundLeadingZero = *String == L'0';\r
   while (*String == L'0') {\r
     String++;\r
   }\r
 \r
   if (CharToUpper (*String) == L'X') {\r
-    if (*(String - 1) != L'0') {\r
+    if (!FoundLeadingZero) {\r
       *Data = 0;\r
       return RETURN_SUCCESS;\r
     }\r
@@ -2393,6 +2402,9 @@ AsciiStrHexToUintnS (
   OUT       UINTN  *Data\r
   )\r
 {\r
+  BOOLEAN  FoundLeadingZero;\r
+\r
+  FoundLeadingZero = FALSE;\r
   //\r
   // 1. Neither String nor Data shall be a null pointer.\r
   //\r
@@ -2420,12 +2432,13 @@ AsciiStrHexToUintnS (
   //\r
   // Ignore leading Zeros after the spaces\r
   //\r
+  FoundLeadingZero = *String == '0';\r
   while (*String == '0') {\r
     String++;\r
   }\r
 \r
   if (AsciiCharToUpper (*String) == 'X') {\r
-    if (*(String - 1) != '0') {\r
+    if (!FoundLeadingZero) {\r
       *Data = 0;\r
       return RETURN_SUCCESS;\r
     }\r
@@ -2517,6 +2530,9 @@ AsciiStrHexToUint64S (
   OUT       UINT64  *Data\r
   )\r
 {\r
+  BOOLEAN  FoundLeadingZero;\r
+\r
+  FoundLeadingZero = FALSE;\r
   //\r
   // 1. Neither String nor Data shall be a null pointer.\r
   //\r
@@ -2544,12 +2560,13 @@ AsciiStrHexToUint64S (
   //\r
   // Ignore leading Zeros after the spaces\r
   //\r
+  FoundLeadingZero = *String == '0';\r
   while (*String == '0') {\r
     String++;\r
   }\r
 \r
   if (AsciiCharToUpper (*String) == 'X') {\r
-    if (*(String - 1) != '0') {\r
+    if (!FoundLeadingZero) {\r
       *Data = 0;\r
       return RETURN_SUCCESS;\r
     }\r