From 4f7f1f5fa8bdd3f4446ba8e8eb6fb662c4eb99d9 Mon Sep 17 00:00:00 2001 From: qhuang8 Date: Thu, 13 Jul 2006 01:53:27 +0000 Subject: [PATCH] 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 | 6 +- .../Library/BaseMemoryLibMmx/Ia32/CopyMem.S | 4 +- .../BaseMemoryLibRepStr/Ia32/CopyMem.S | 4 +- MdePkg/Library/BasePrintLib/PrintLib.c | 63 ++++++++----------- .../Library/BasePrintLib/PrintLibInternal.c | 23 ++++--- .../Library/BasePrintLib/PrintLibInternal.h | 6 +- 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 // @@ -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. 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.