Re: [PATCH v2 1/6] MdePkg/PrintLib: Fix possible negative value left shift

["Gao, Liming" <liming.gao@intel.com>]

Reviewed-by: Liming Gao <liming.gao@intel.com>

>-----Original Message-----
>From: Wu, Hao A
>Sent: Thursday, September 21, 2017 2:46 PM
>To: edk2-devel@lists.01.org
>Cc: Wu, Hao A <hao.a.wu@intel.com>; Shi, Steven <steven.shi@intel.com>;
>Kinney, Michael D <michael.d.kinney@intel.com>; Gao, Liming
><liming.gao@intel.com>
>Subject: [PATCH v2 1/6] MdePkg/PrintLib: Fix possible negative value left shift
>
>REF: https://bugzilla.tianocore.org/show_bug.cgi?id=702
>
>Within function InternalPrintLibSPrintMarker(), possible left shift of a
>negative value is found in:
>"(*(ArgumentString + 1) << 8)"
>
>which involves undefined behavior.
>
>Since '*(ArgumentString + 1)' is of type CONST CHAR8 (signed), it will be
>promoted to type int (signed) during the left shift operation. If
>'*(ArgumentString + 1)' is a negative value, the behavior will be
>undefined.
>
>According to the C11 spec, Section 6.5.7:
>> 4 The result of E1 << E2 is E1 left-shifted E2 bit positions; vacated
>>   bits are filled with zeros. If E1 has an unsigned type, the value
>>   of the result is E1 * 2^E2 , reduced modulo one more than the
>>   maximum value representable in the result type. If E1 has a signed
>>   type and nonnegative value, and E1 * 2^E2 is representable in the
>>   result type, then that is the resulting value; otherwise, the
>>   behavior is undefined.
>
>This commit explicitly cast '*(ArgumentString + 1)' with UINT8 to resolve
>this issue.
>
>Cc: Steven Shi <steven.shi@intel.com>
>Cc: Michael Kinney <michael.d.kinney@intel.com>
>Cc: Liming Gao <liming.gao@intel.com>
>Contributed-under: TianoCore Contribution Agreement 1.1
>Signed-off-by: Hao Wu <hao.a.wu@intel.com>
>---
> MdePkg/Library/BasePrintLib/PrintLibInternal.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
>diff --git a/MdePkg/Library/BasePrintLib/PrintLibInternal.c
>b/MdePkg/Library/BasePrintLib/PrintLibInternal.c
>index cec5b3bc99..28d946472f 100644
>--- a/MdePkg/Library/BasePrintLib/PrintLibInternal.c
>+++ b/MdePkg/Library/BasePrintLib/PrintLibInternal.c
>@@ -1165,7 +1165,7 @@ BasePrintLibSPrintMarker (
>     // Copy the string into the output buffer performing the required type
>conversions
>     //
>     while (Index < Count) {
>-      ArgumentCharacter = ((*ArgumentString & 0xff) | (*(ArgumentString + 1)
><< 8)) & ArgumentMask;
>+      ArgumentCharacter = ((*ArgumentString & 0xff) |
>(((UINT8)*(ArgumentString + 1)) << 8)) & ArgumentMask;
>
>       LengthToReturn += (1 * BytesPerOutputCharacter);
>       if ((Flags & COUNT_ONLY_NO_PRINT) == 0 && Buffer != NULL) {
>--
>2.12.0.windows.1