From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga01.intel.com (mga01.intel.com [192.55.52.88]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id 6D407207E57E0 for ; Wed, 12 Apr 2017 00:09:18 -0700 (PDT) Received: from fmsmga001.fm.intel.com ([10.253.24.23]) by fmsmga101.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 12 Apr 2017 00:09:18 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.37,189,1488873600"; d="scan'208";a="1134311164" Received: from shwdeopenpsi114.ccr.corp.intel.com ([10.239.157.135]) by fmsmga001.fm.intel.com with ESMTP; 12 Apr 2017 00:09:16 -0700 From: Dandan Bi To: edk2-devel@lists.01.org Cc: Eric Dong , Liming Gao , Hao Wu Date: Wed, 12 Apr 2017 15:08:51 +0800 Message-Id: <1491980931-115060-1-git-send-email-dandan.bi@intel.com> X-Mailer: git-send-email 1.9.5.msysgit.1 Subject: [patch] MdeModulePkg/HiiDB: Avoid incorrect results of multiplication X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.22 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 12 Apr 2017 07:09:18 -0000 An example: The codes in function Output8bitPixel in Image.c: OffsetY = BITMAP_LEN_8_BIT ((UINT32) Image->Width, Ypos); Both Image->Width and Ypos are of type UINT16. They will be promoted to int (signed) first, and then perform the multiplication defined by macro BITMAP_LEN_8_BIT. If the result of multiplication between Image->Width and Ypos exceeds the range of type int, a potential incorrect results will be assigned to OffsetY. This commit adds explicit UINT32 type cast for 'Image->Width' to avoid possible overflow in the int range. And also fix similar issues in HiiDatabase. Cc: Eric Dong Cc: Liming Gao Cc: Hao Wu Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: Dandan Bi --- MdeModulePkg/Universal/HiiDatabaseDxe/Image.c | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/MdeModulePkg/Universal/HiiDatabaseDxe/Image.c b/MdeModulePkg/Universal/HiiDatabaseDxe/Image.c index e2fa16e..431a5b8 100644 --- a/MdeModulePkg/Universal/HiiDatabaseDxe/Image.c +++ b/MdeModulePkg/Universal/HiiDatabaseDxe/Image.c @@ -103,21 +103,21 @@ GetImageIdOrAddress ( case EFI_HII_IIBT_IMAGE_8BIT: case EFI_HII_IIBT_IMAGE_8BIT_TRANS: Length = sizeof (EFI_HII_IIBT_IMAGE_8BIT_BLOCK) - sizeof (UINT8) + BITMAP_LEN_8_BIT ( - ReadUnaligned16 (&((EFI_HII_IIBT_IMAGE_8BIT_BLOCK *) CurrentImageBlock)->Bitmap.Width), + (UINT32) ReadUnaligned16 (&((EFI_HII_IIBT_IMAGE_8BIT_BLOCK *) CurrentImageBlock)->Bitmap.Width), ReadUnaligned16 (&((EFI_HII_IIBT_IMAGE_8BIT_BLOCK *) CurrentImageBlock)->Bitmap.Height) ); ImageIdCurrent++; break; case EFI_HII_IIBT_IMAGE_24BIT: case EFI_HII_IIBT_IMAGE_24BIT_TRANS: Length = sizeof (EFI_HII_IIBT_IMAGE_24BIT_BLOCK) - sizeof (EFI_HII_RGB_PIXEL) + BITMAP_LEN_24_BIT ( - ReadUnaligned16 ((VOID *) &((EFI_HII_IIBT_IMAGE_24BIT_BLOCK *) CurrentImageBlock)->Bitmap.Width), + (UINT32) ReadUnaligned16 ((VOID *) &((EFI_HII_IIBT_IMAGE_24BIT_BLOCK *) CurrentImageBlock)->Bitmap.Width), ReadUnaligned16 ((VOID *) &((EFI_HII_IIBT_IMAGE_24BIT_BLOCK *) CurrentImageBlock)->Bitmap.Height) ); ImageIdCurrent++; break; @@ -451,11 +451,11 @@ Output8bitPixel ( // // Convert the pixel from 8 bits to corresponding color. // for (Ypos = 0; Ypos < Image->Height; Ypos++) { - OffsetY = BITMAP_LEN_8_BIT (Image->Width, Ypos); + OffsetY = BITMAP_LEN_8_BIT ((UINT32) Image->Width, Ypos); // // All bits are meaningful since the bitmap is 8 bits per pixel. // for (Xpos = 0; Xpos < Image->Width; Xpos++) { Byte = *(Data + OffsetY + Xpos); @@ -491,11 +491,11 @@ Output24bitPixel ( ASSERT (Image != NULL && Data != NULL); BitMapPtr = Image->Bitmap; for (Ypos = 0; Ypos < Image->Height; Ypos++) { - OffsetY = BITMAP_LEN_8_BIT (Image->Width, Ypos); + OffsetY = BITMAP_LEN_8_BIT ((UINT32) Image->Width, Ypos); CopyRgbToGopPixel (&BitMapPtr[OffsetY], &Data[OffsetY], Image->Width); } } @@ -648,11 +648,11 @@ HiiNewImage ( if (PackageListNode == NULL) { return EFI_NOT_FOUND; } NewBlockSize = sizeof (EFI_HII_IIBT_IMAGE_24BIT_BLOCK) - sizeof (EFI_HII_RGB_PIXEL) + - BITMAP_LEN_24_BIT (Image->Width, Image->Height); + BITMAP_LEN_24_BIT ((UINT32) Image->Width, Image->Height); // // Get the image package in the package list, // or create a new image package if image package does not exist. // @@ -751,11 +751,11 @@ HiiNewImage ( } else { ImageBlocks->BlockType = EFI_HII_IIBT_IMAGE_24BIT; } WriteUnaligned16 ((VOID *) &((EFI_HII_IIBT_IMAGE_24BIT_BLOCK *) ImageBlocks)->Bitmap.Width, Image->Width); WriteUnaligned16 ((VOID *) &((EFI_HII_IIBT_IMAGE_24BIT_BLOCK *) ImageBlocks)->Bitmap.Height, Image->Height); - CopyGopToRgbPixel (((EFI_HII_IIBT_IMAGE_24BIT_BLOCK *) ImageBlocks)->Bitmap.Bitmap, Image->Bitmap, Image->Width * Image->Height); + CopyGopToRgbPixel (((EFI_HII_IIBT_IMAGE_24BIT_BLOCK *) ImageBlocks)->Bitmap.Bitmap, Image->Bitmap, (UINT32) Image->Width * Image->Height); // // Append the block end // ImageBlocks = (EFI_HII_IMAGE_BLOCK *) ((UINT8 *) ImageBlocks + NewBlockSize); @@ -894,11 +894,11 @@ IGetImage ( // // Use the common block code since the definition of these structures is the same. // CopyMem (&Iibt1bit, CurrentImageBlock, sizeof (EFI_HII_IIBT_IMAGE_1BIT_BLOCK)); ImageLength = sizeof (EFI_GRAPHICS_OUTPUT_BLT_PIXEL) * - (Iibt1bit.Bitmap.Width * Iibt1bit.Bitmap.Height); + ((UINT32) Iibt1bit.Bitmap.Width * Iibt1bit.Bitmap.Height); Image->Bitmap = AllocateZeroPool (ImageLength); if (Image->Bitmap == NULL) { return EFI_OUT_OF_RESOURCES; } @@ -945,11 +945,11 @@ IGetImage ( // fall through // case EFI_HII_IIBT_IMAGE_24BIT: Width = ReadUnaligned16 ((VOID *) &((EFI_HII_IIBT_IMAGE_24BIT_BLOCK *) CurrentImageBlock)->Bitmap.Width); Height = ReadUnaligned16 ((VOID *) &((EFI_HII_IIBT_IMAGE_24BIT_BLOCK *) CurrentImageBlock)->Bitmap.Height); - ImageLength = sizeof (EFI_GRAPHICS_OUTPUT_BLT_PIXEL) * (Width * Height); + ImageLength = sizeof (EFI_GRAPHICS_OUTPUT_BLT_PIXEL) * ((UINT32) Width * Height); Image->Bitmap = AllocateZeroPool (ImageLength); if (Image->Bitmap == NULL) { return EFI_OUT_OF_RESOURCES; } @@ -1093,19 +1093,19 @@ HiiSetImage ( break; case EFI_HII_IIBT_IMAGE_8BIT: case EFI_HII_IIBT_IMAGE_8BIT_TRANS: OldBlockSize = sizeof (EFI_HII_IIBT_IMAGE_8BIT_BLOCK) - sizeof (UINT8) + BITMAP_LEN_8_BIT ( - ReadUnaligned16 (&((EFI_HII_IIBT_IMAGE_8BIT_BLOCK *) CurrentImageBlock)->Bitmap.Width), + (UINT32) ReadUnaligned16 (&((EFI_HII_IIBT_IMAGE_8BIT_BLOCK *) CurrentImageBlock)->Bitmap.Width), ReadUnaligned16 (&((EFI_HII_IIBT_IMAGE_8BIT_BLOCK *) CurrentImageBlock)->Bitmap.Height) ); break; case EFI_HII_IIBT_IMAGE_24BIT: case EFI_HII_IIBT_IMAGE_24BIT_TRANS: OldBlockSize = sizeof (EFI_HII_IIBT_IMAGE_24BIT_BLOCK) - sizeof (EFI_HII_RGB_PIXEL) + BITMAP_LEN_24_BIT ( - ReadUnaligned16 ((VOID *) &((EFI_HII_IIBT_IMAGE_24BIT_BLOCK *) CurrentImageBlock)->Bitmap.Width), + (UINT32) ReadUnaligned16 ((VOID *) &((EFI_HII_IIBT_IMAGE_24BIT_BLOCK *) CurrentImageBlock)->Bitmap.Width), ReadUnaligned16 ((VOID *) &((EFI_HII_IIBT_IMAGE_24BIT_BLOCK *) CurrentImageBlock)->Bitmap.Height) ); break; default: return EFI_NOT_FOUND; @@ -1113,11 +1113,11 @@ HiiSetImage ( // // Create the new image block according to input image. // NewBlockSize = sizeof (EFI_HII_IIBT_IMAGE_24BIT_BLOCK) - sizeof (EFI_HII_RGB_PIXEL) + - BITMAP_LEN_24_BIT (Image->Width, Image->Height); + BITMAP_LEN_24_BIT ((UINT32) Image->Width, Image->Height); // // Adjust the image package to remove the original block firstly then add the new block. // ImageBlocks = AllocateZeroPool (ImagePackage->ImageBlockSize + NewBlockSize - OldBlockSize); if (ImageBlocks == NULL) { @@ -1138,11 +1138,11 @@ HiiSetImage ( NewImageBlock->BlockType = EFI_HII_IIBT_IMAGE_24BIT; } WriteUnaligned16 ((VOID *) &((EFI_HII_IIBT_IMAGE_24BIT_BLOCK *) NewImageBlock)->Bitmap.Width, Image->Width); WriteUnaligned16 ((VOID *) &((EFI_HII_IIBT_IMAGE_24BIT_BLOCK *) NewImageBlock)->Bitmap.Height, Image->Height); CopyGopToRgbPixel (((EFI_HII_IIBT_IMAGE_24BIT_BLOCK *) NewImageBlock)->Bitmap.Bitmap, - Image->Bitmap, Image->Width * Image->Height); + Image->Bitmap, (UINT32) Image->Width * Image->Height); CopyMem ((UINT8 *) NewImageBlock + NewBlockSize, (UINT8 *) CurrentImageBlock + OldBlockSize, Part2Size); FreePool (ImagePackage->ImageBlock); ImagePackage->ImageBlock = ImageBlocks; -- 1.9.5.msysgit.1