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

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.
 

MdePkg/Library/BaseMemoryLibMmx/Ia32/CopyMem.S

@@ -85,6 +85,6 @@ L2:
     movsb
     cld
     movl    12(%esp), %eax
-    push    %esi
+    pop    %esi
-    push    %edi
+    pop    %edi
     ret

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
+    pop    %edi
-    push    %esi
+    pop    %esi
     ret

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
     //
@@ -244,13 +251,6 @@ BasePrintLibVSPrint (
         }
       } 
 
-      //
-      // Limit the maximum field width to the remaining characters in the output buffer
-      //
-      if (Width > BufferSize) {
-        Width = BufferSize;
-      }
-
       //
       // Handle each argument type
       //
@@ -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,14 +533,9 @@ 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
     //
@@ -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.
 

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);
 }

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.