Re: [PATCH 1/2] MdePkg/UefiLib: Avoid mis-calculate of graphic console size

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

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

>-----Original Message-----
>From: Wu, Hao A
>Sent: Tuesday, April 11, 2017 10:17 AM
>To: edk2-devel@lists.01.org
>Cc: Wu, Hao A <hao.a.wu@intel.com>; Gao, Liming <liming.gao@intel.com>
>Subject: [PATCH 1/2] MdePkg/UefiLib: Avoid mis-calculate of graphic console
>size
>
>The commit adds check in function InternalPrintGraphic() to ensure that
>the expression:
>
>Blt->Width * Blt->Height * sizeof (EFI_GRAPHICS_OUTPUT_BLT_PIXEL)
>
>will not overflow in the UINTN range.
>
>The commit also adds an explicit UINT32 type cast for 'Blt->Width' to
>avoid possible overflow in the int range for:
>
>Blt->Width * Blt->Height
>
>Since both Blt->Width and Blt->Height are of type UINT16. They will be
>promoted to int (signed) first, and then perform the multiplication
>operation. If the result of multiplication between Blt->Width and
>Blt->Height exceeds the range of type int, a potential incorrect size will
>be passed into funciton AllocateZeroPool().
>
>Cc: Liming Gao <liming.gao@intel.com>
>Contributed-under: TianoCore Contribution Agreement 1.0
>Signed-off-by: Hao Wu <hao.a.wu@intel.com>
>---
> MdePkg/Library/UefiLib/UefiLibPrint.c | 11 +++++++++--
> 1 file changed, 9 insertions(+), 2 deletions(-)
>
>diff --git a/MdePkg/Library/UefiLib/UefiLibPrint.c
>b/MdePkg/Library/UefiLib/UefiLibPrint.c
>index 9f52e7d0ce..5527f8e7a8 100644
>--- a/MdePkg/Library/UefiLib/UefiLibPrint.c
>+++ b/MdePkg/Library/UefiLib/UefiLibPrint.c
>@@ -2,7 +2,7 @@
>   Mde UEFI library API implementation.
>   Print to StdErr or ConOut defined in EFI_SYSTEM_TABLE
>
>-  Copyright (c) 2007 - 2015, Intel Corporation. All rights reserved.<BR>
>+  Copyright (c) 2007 - 2017, Intel Corporation. All rights reserved.<BR>
>   This program and the accompanying materials
>   are licensed and made available under the terms and conditions of the BSD
>License
>   which accompanies this distribution.  The full text of the license may be
>found at
>@@ -474,7 +474,14 @@ InternalPrintGraphic (
>   } else if (FeaturePcdGet (PcdUgaConsumeSupport)) {
>     ASSERT (UgaDraw!= NULL);
>
>-    Blt->Image.Bitmap = AllocateZeroPool (Blt->Width * Blt->Height * sizeof
>(EFI_GRAPHICS_OUTPUT_BLT_PIXEL));
>+    //
>+    // Ensure Width * Height * sizeof (EFI_GRAPHICS_OUTPUT_BLT_PIXEL)
>doesn't overflow.
>+    //
>+    if (Blt->Width > DivU64x32 (MAX_UINTN, Blt->Height * sizeof
>(EFI_GRAPHICS_OUTPUT_BLT_PIXEL))) {
>+      goto Error;
>+    }
>+
>+    Blt->Image.Bitmap = AllocateZeroPool ((UINT32) Blt->Width * Blt->Height
>* sizeof (EFI_GRAPHICS_OUTPUT_BLT_PIXEL));
>     ASSERT (Blt->Image.Bitmap != NULL);
>
>     //
>--
>2.12.0.windows.1