[edk2] [PATCH v2 2/2] MdeModulePkg/DxePrintLibPrint2Protocol: Fix potential string over read

Jian J Wang posted 2 patches 6 years, 12 months ago
[edk2] [PATCH v2 2/2] MdeModulePkg/DxePrintLibPrint2Protocol: Fix potential string over read
Posted by Jian J Wang 6 years, 12 months ago
> v2:
>    Add in v2

Due to a potential hole in the stop condition of loop, the two continuous
access to ArgumentString (index, index+1) inside the loop might cause the
string ending character ('\0') and the byte after it to be read.

Cc: Michael D Kinney <michael.d.kinney@intel.com>
Cc: Liming Gao <liming.gao@intel.com>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Star Zeng <star.zeng@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Jian J Wang <jian.j.wang@intel.com>
---
 MdeModulePkg/Library/DxePrintLibPrint2Protocol/PrintLib.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/MdeModulePkg/Library/DxePrintLibPrint2Protocol/PrintLib.c b/MdeModulePkg/Library/DxePrintLibPrint2Protocol/PrintLib.c
index 56534e56c3..570d06d82e 100644
--- a/MdeModulePkg/Library/DxePrintLibPrint2Protocol/PrintLib.c
+++ b/MdeModulePkg/Library/DxePrintLibPrint2Protocol/PrintLib.c
@@ -2050,7 +2050,10 @@ InternalPrintLibSPrintMarker (
       // Compute the number of characters in ArgumentString and store it in Count
       // ArgumentString is either null-terminated, or it contains Precision characters
       //
-      for (Count = 0; Count < Precision || ((Flags & PRECISION) == 0); Count++) {
+      for (Count = 0;
+            ArgumentString[Count * BytesPerArgumentCharacter] != '\0' &&
+            Count < Precision || ((Flags & PRECISION) == 0);
+            Count++) {
         ArgumentCharacter = ((ArgumentString[Count * BytesPerArgumentCharacter] & 0xff) | ((ArgumentString[Count * BytesPerArgumentCharacter + 1]) << 8)) & ArgumentMask;
         if (ArgumentCharacter == 0) {
           break;
@@ -2107,7 +2110,7 @@ InternalPrintLibSPrintMarker (
     //
     // Copy the string into the output buffer performing the required type conversions
     //
-    while (Index < Count) {
+    while (Index < Count && (*ArgumentString) != '\0') {
       ArgumentCharacter = ((*ArgumentString & 0xff) | (((UINT8)*(ArgumentString + 1)) << 8)) & ArgumentMask;
 
       LengthToReturn += (1 * BytesPerOutputCharacter);
-- 
2.15.1.windows.2

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel