BasePrintLib: Fix Buffer Overflow issue.

BaseMemoryLib: Fix error in CopyMem.S for BaseMemoryLibMmx & BaseMemoryLibRepStr instance.


git-svn-id: https://edk2.svn.sourceforge.net/svnroot/edk2/trunk/edk2@938 6f19259b-4bc3-4df7-8a09-765794883524

6 files changed, 56 insertions, 50 deletions

diff --git a/MdePkg/Include/Library/PrintLib.h b/MdePkg/Include/Library/PrintLib.h
index e1b3cb37bb..004bc9f926 100644
--- a/MdePkg/Include/Library/PrintLib.h
+++ b/MdePkg/Include/Library/PrintLib.h
@@ -394,7 +394,8 @@ AsciiSPrintUnicodeFormat (
                   Unicode string.

   @param  Flags   The bitmask of flags that specify left justification, zero pad, and commas.

   @param  Value   The 64-bit signed value to convert to a string.

-  @param  Width   The maximum number of Unicode characters to place in Buffer.

+  @param  Width   The maximum number of Unicode characters to place in Buffer, not including

+                  the Null-terminator.

   

   @return The number of Unicode characters in Buffer not including the Null-terminator.

 

@@ -438,7 +439,8 @@ UnicodeValueToString (
                   ASCII string.

   @param  Flags   The bitmask of flags that specify left justification, zero pad, and commas.

   @param  Value   The 64-bit signed value to convert to a string.

-  @param  Width   The maximum number of ASCII characters to place in Buffer.

+  @param  Width   The maximum number of ASCII characters to place in Buffer, not including

+                  the Null-terminator.

   

   @return The number of ASCII characters in Buffer not including the Null-terminator.

 

diff --git a/MdePkg/Library/BaseMemoryLibMmx/Ia32/CopyMem.S b/MdePkg/Library/BaseMemoryLibMmx/Ia32/CopyMem.S
index 56788cb981..3c00c2a81e 100644
--- a/MdePkg/Library/BaseMemoryLibMmx/Ia32/CopyMem.S
+++ b/MdePkg/Library/BaseMemoryLibMmx/Ia32/CopyMem.S
@@ -85,6 +85,6 @@ L2:
     movsb
     cld
     movl    12(%esp), %eax
-    push    %esi
-    push    %edi
+    pop    %esi
+    pop    %edi
     ret
diff --git a/MdePkg/Library/BaseMemoryLibRepStr/Ia32/CopyMem.S b/MdePkg/Library/BaseMemoryLibRepStr/Ia32/CopyMem.S
index cce9836833..4215c20393 100644
--- a/MdePkg/Library/BaseMemoryLibRepStr/Ia32/CopyMem.S
+++ b/MdePkg/Library/BaseMemoryLibRepStr/Ia32/CopyMem.S
@@ -53,6 +53,6 @@ L0:
     movsb                               # Copy bytes backward
     cld
     movl    12(%esp),%eax               # eax <- Destination as return value
-    push    %edi
-    push    %esi
+    pop    %edi
+    pop    %esi
     ret
diff --git a/MdePkg/Library/BasePrintLib/PrintLib.c b/MdePkg/Library/BasePrintLib/PrintLib.c
index 33da6cb6b0..6b4f1fad4a 100644
--- a/MdePkg/Library/BasePrintLib/PrintLib.c
+++ b/MdePkg/Library/BasePrintLib/PrintLib.c
@@ -80,6 +80,7 @@ BasePrintLibVSPrint (
   )

 {

   CHAR8           *OriginalBuffer;

+  CHAR8           *EndBuffer;

   CHAR8           ValueBuffer[MAXIMUM_VALUE_CHARACTERS];

   UINTN           BytesPerOutputCharacter;

   UINTN           BytesPerFormatCharacter;

@@ -110,13 +111,22 @@ BasePrintLibVSPrint (
   }

   ASSERT (Buffer != NULL);

 

-  OriginalBuffer = Buffer;

-

   if ((Flags & OUTPUT_UNICODE) != 0) {

     BytesPerOutputCharacter = 2;

   } else {

     BytesPerOutputCharacter = 1;

   }

+

+  //

+  // Reserve space for the Null terminator.

+  //

+  BufferSize--;

+  OriginalBuffer = Buffer;

+  //

+  // Set the tag for the end of the input Buffer.

+  //

+  EndBuffer      = Buffer + BufferSize * BytesPerOutputCharacter;

+

   if ((Flags & FORMAT_UNICODE) != 0) {

     //

     // Make sure format string cannot contain more than PcdMaximumUnicodeStringLength

@@ -135,10 +145,7 @@ BasePrintLibVSPrint (
     FormatMask = 0xff;

   }

 

-  //

-  // Reserve space for the Null terminator.

-  //

-  BufferSize--;

+

 

   //

   // Get the first character from the format string

@@ -148,7 +155,7 @@ BasePrintLibVSPrint (
   //

   // Loop until the end of the format string is reached or the output buffer is full

   //

-  while (FormatCharacter != 0 && BufferSize > 0) {

+  while (FormatCharacter != 0 && Buffer < EndBuffer) {

     //

     // Clear all the flag bits except those that may have been passed in

     //

@@ -245,13 +252,6 @@ BasePrintLibVSPrint (
       } 

 

       //

-      // Limit the maximum field width to the remaining characters in the output buffer

-      //

-      if (Width > BufferSize) {

-        Width = BufferSize;

-      }

-

-      //

       // Handle each argument type

       //

       switch (FormatCharacter) {

@@ -477,12 +477,6 @@ BasePrintLibVSPrint (
       }

     }

 

-    //

-    // Limit the length of the string to append to the remaining characters in the output buffer

-    //

-    if (Count > BufferSize) {

-      Count = BufferSize;

-    }

     if (Precision < Count) {

       Precision = Count;

     }

@@ -491,18 +485,18 @@ BasePrintLibVSPrint (
     // Pad before the string

     //

     if ((Flags & (PAD_TO_WIDTH | LEFT_JUSTIFY)) == (PAD_TO_WIDTH)) {

-      Buffer = BasePrintLibFillBuffer (Buffer, Width - Precision, ' ', BytesPerOutputCharacter);

+      Buffer = BasePrintLibFillBuffer (Buffer, EndBuffer, Width - Precision, ' ', BytesPerOutputCharacter);

     }

 

     if (ZeroPad) {

       if (Prefix != 0) {

-        Buffer = BasePrintLibFillBuffer (Buffer, 1, Prefix, BytesPerOutputCharacter);

+        Buffer = BasePrintLibFillBuffer (Buffer, EndBuffer, 1, Prefix, BytesPerOutputCharacter);

       }

-      Buffer = BasePrintLibFillBuffer (Buffer, Precision - Count, '0', BytesPerOutputCharacter);

+      Buffer = BasePrintLibFillBuffer (Buffer, EndBuffer, Precision - Count, '0', BytesPerOutputCharacter);

     } else {

-      Buffer = BasePrintLibFillBuffer (Buffer, Precision - Count, ' ', BytesPerOutputCharacter);

+      Buffer = BasePrintLibFillBuffer (Buffer, EndBuffer, Precision - Count, ' ', BytesPerOutputCharacter);

       if (Prefix != 0) {

-        Buffer = BasePrintLibFillBuffer (Buffer, 1, Prefix, BytesPerOutputCharacter);

+        Buffer = BasePrintLibFillBuffer (Buffer, EndBuffer, 1, Prefix, BytesPerOutputCharacter);

       }

     }

 

@@ -520,7 +514,7 @@ BasePrintLibVSPrint (
     while (Index < Count) {

       ArgumentCharacter = ((*ArgumentString & 0xff) | (*(ArgumentString + 1) << 8)) & ArgumentMask;

 

-      Buffer = BasePrintLibFillBuffer (Buffer, 1, ArgumentCharacter, BytesPerOutputCharacter);

+      Buffer = BasePrintLibFillBuffer (Buffer, EndBuffer, 1, ArgumentCharacter, BytesPerOutputCharacter);

       ArgumentString    += BytesPerArgumentCharacter;

       Index++;

       if (Comma) {

@@ -529,7 +523,7 @@ BasePrintLibVSPrint (
           Digits = 0;

           Index++;

           if (Index < Count) {

-            Buffer = BasePrintLibFillBuffer (Buffer, 1, ',', BytesPerOutputCharacter);

+            Buffer = BasePrintLibFillBuffer (Buffer, EndBuffer, 1, ',', BytesPerOutputCharacter);

           }

         }

       }

@@ -539,15 +533,10 @@ BasePrintLibVSPrint (
     // Pad after the string

     //

     if ((Flags & (PAD_TO_WIDTH | LEFT_JUSTIFY)) == (PAD_TO_WIDTH | LEFT_JUSTIFY)) {

-      Buffer = BasePrintLibFillBuffer (Buffer, Width - Precision, ' ', BytesPerOutputCharacter);

+      Buffer = BasePrintLibFillBuffer (Buffer, EndBuffer, Width - Precision, ' ', BytesPerOutputCharacter);

     }

 

     //

-    // Reduce the number of characters

-    //

-    BufferSize -= Count;

-

-    //

     // Get the next character from the format string

     //

     Format += BytesPerFormatCharacter;

@@ -561,7 +550,7 @@ BasePrintLibVSPrint (
   //

   // Null terminate the Unicode or ASCII string

   //

-  BasePrintLibFillBuffer (Buffer, 1, 0, BytesPerOutputCharacter);

+  BasePrintLibFillBuffer (Buffer, EndBuffer, 1, 0, BytesPerOutputCharacter);

   //

   // Make sure output buffer cannot contain more than PcdMaximumUnicodeStringLength

   // Unicode characters if PcdMaximumUnicodeStringLength is not zero. 

@@ -999,7 +988,8 @@ AsciiSPrintUnicodeFormat (
                   Unicode string.

   @param  Flags   The bitmask of flags that specify left justification, zero pad, and commas.

   @param  Value   The 64-bit signed value to convert to a string.

-  @param  Width   The maximum number of Unicode characters to place in Buffer.

+  @param  Width   The maximum number of Unicode characters to place in Buffer, not including

+                  the Null-terminator.

   

   @return The number of Unicode characters in Buffer not including the Null-terminator.

 

@@ -1046,7 +1036,8 @@ UnicodeValueToString (
                   ASCII string.

   @param  Flags   The bitmask of flags that specify left justification, zero pad, and commas.

   @param  Value   The 64-bit signed value to convert to a string.

-  @param  Width   The maximum number of ASCII characters to place in Buffer.

+  @param  Width   The maximum number of ASCII characters to place in Buffer, not including

+                  the Null-terminator.

   

   @return The number of ASCII characters in Buffer not including the Null-terminator.

 

diff --git a/MdePkg/Library/BasePrintLib/PrintLibInternal.c b/MdePkg/Library/BasePrintLib/PrintLibInternal.c
index 8f417fbf11..0a75a3c581 100644
--- a/MdePkg/Library/BasePrintLib/PrintLibInternal.c
+++ b/MdePkg/Library/BasePrintLib/PrintLibInternal.c
@@ -25,6 +25,8 @@ static CONST CHAR8 mHexStr[] = {'0','1','2','3','4','5','6','7','8','9','A','B',
   Internal function that places ASCII or Unicode character into the Buffer.

 

   @param  Buffer      Buffer to place the Unicode or ASCII string.

+  @param  EndBuffer   The end of the input Buffer. No characters will be

+                      placed after that. 

   @param  Length      Count of character to be placed into Buffer.

   @param  Character   Character to be placed into Buffer.

   @param  Increment   Character increment in Buffer.

@@ -35,6 +37,7 @@ static CONST CHAR8 mHexStr[] = {'0','1','2','3','4','5','6','7','8','9','A','B',
 CHAR8 *

 BasePrintLibFillBuffer (

   CHAR8   *Buffer,

+  CHAR8   *EndBuffer,

   INTN    Length,

   UINTN   Character,

   INTN    Increment

@@ -42,7 +45,7 @@ BasePrintLibFillBuffer (
 {

   INTN  Index;

 

-  for (Index = 0; Index < Length; Index++) {

+  for (Index = 0; Index < Length && Buffer < EndBuffer; Index++) {

     *Buffer       =  (CHAR8) Character;

     *(Buffer + 1) =  (CHAR8) (Character >> 8);

     Buffer        += Increment;

@@ -117,7 +120,8 @@ BasePrintLibValueToString (
   @param  Flags     The bitmask of flags that specify left justification, zero pad,

                     and commas.

   @param  Value     The 64-bit signed value to convert to a string.

-  @param  Width      The maximum number of characters to place in Buffer.

+  @param  Width     The maximum number of characters to place in Buffer, not including

+                    the Null-terminator.

   @param  Increment Character increment in Buffer.

   

   @return The number of characters in Buffer not including the Null-terminator.

@@ -133,6 +137,7 @@ BasePrintLibConvertValueToString (
   )

 {

   CHAR8  *OriginalBuffer;

+  CHAR8  *EndBuffer;

   CHAR8  ValueBuffer[MAXIMUM_VALUE_CHARACTERS];

   UINTN  Count;

   UINTN  Digits;

@@ -154,17 +159,21 @@ BasePrintLibConvertValueToString (
   if (Width == 0) {

     Width = MAXIMUM_VALUE_CHARACTERS - 1;

   }

+  //

+  // Set the tag for the end of the input Buffer.

+  //

+  EndBuffer = Buffer + Width * Increment;

 

   if (Value < 0) {

     Value = -Value;

-    Buffer = BasePrintLibFillBuffer (Buffer, 1, '-', Increment);

+    Buffer = BasePrintLibFillBuffer (Buffer, EndBuffer, 1, '-', Increment);

     Width--;

   }

 

   Count = BasePrintLibValueToString (ValueBuffer, Value, 10);

 

   if ((Flags & PREFIX_ZERO) != 0) {

-    Buffer = BasePrintLibFillBuffer (Buffer, Width - Count, '0', Increment);

+    Buffer = BasePrintLibFillBuffer (Buffer, EndBuffer, Width - Count, '0', Increment);

   }

 

   Digits = Count % 3;

@@ -172,19 +181,19 @@ BasePrintLibConvertValueToString (
     Digits = 3 - Digits;

   }

   for (Index = 0; Index < Count; Index++) {

-    Buffer = BasePrintLibFillBuffer (Buffer, 1, ValueBuffer[Count - Index], Increment);

+    Buffer = BasePrintLibFillBuffer (Buffer, EndBuffer, 1, ValueBuffer[Count - Index], Increment);

     if ((Flags & COMMA_TYPE) != 0) {

       Digits++;

       if (Digits == 3) {

         Digits = 0;

         if ((Index + 1) < Count) {

-          Buffer = BasePrintLibFillBuffer (Buffer, 1, ',', Increment);

+          Buffer = BasePrintLibFillBuffer (Buffer, EndBuffer, 1, ',', Increment);

         }

       }

     }

   }

 

-  BasePrintLibFillBuffer (Buffer, 1, 0, Increment);

+  BasePrintLibFillBuffer (Buffer, EndBuffer, 1, 0, Increment);

 

   return ((Buffer - OriginalBuffer) / Increment);

 }

diff --git a/MdePkg/Library/BasePrintLib/PrintLibInternal.h b/MdePkg/Library/BasePrintLib/PrintLibInternal.h
index b7c95a8e43..e0928b8c80 100644
--- a/MdePkg/Library/BasePrintLib/PrintLibInternal.h
+++ b/MdePkg/Library/BasePrintLib/PrintLibInternal.h
@@ -85,6 +85,8 @@ BasePrintLibSPrint (
   Internal function that places ASCII or Unicode character into the Buffer.

 

   @param  Buffer      Buffer to place the Unicode or ASCII string.

+  @param  EndBuffer   The end of the input Buffer. No characters will be

+                      placed after that. 

   @param  Length      Count of character to be placed into Buffer.

   @param  Character   Character to be placed into Buffer.

   @param  Increment   Character increment in Buffer.

@@ -95,6 +97,7 @@ BasePrintLibSPrint (
 CHAR8 *

 BasePrintLibFillBuffer (

   CHAR8   *Buffer,

+  CHAR8   *EndBuffer,

   INTN    Length,

   UINTN   Character,

   INTN    Increment

@@ -151,7 +154,8 @@ BasePrintLibValueToString (
   @param  Flags     The bitmask of flags that specify left justification, zero pad,

                     and commas.

   @param  Value     The 64-bit signed value to convert to a string.

-  @param  Width     The maximum number of characters to place in Buffer.

+  @param  Width     The maximum number of characters to place in Buffer, not including

+                    the Null-terminator.

   @param  Increment Character increment in Buffer.

   

   @return Total number of characters required to perform the conversion.