MdeModulePkg/HiiDatabase: Fix potential integer overflow (CVE-2018-12181)
authorRay Ni <ray.ni@intel.com>
Thu, 7 Mar 2019 10:35:13 +0000 (18:35 +0800)
committerLiming Gao <liming.gao@intel.com>
Fri, 8 Mar 2019 15:44:58 +0000 (23:44 +0800)
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1135

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ray Ni <ray.ni@intel.com>
Cc: Dandan Bi <dandan.bi@intel.com>
Cc: Hao A Wu <hao.a.wu@intel.com>
Reviewed-by: Hao Wu <hao.a.wu@intel.com>
Reviewed-by: Jian J Wang <jian.j.wang@intel.com>
MdeModulePkg/Universal/HiiDatabaseDxe/Image.c

index 71ebc55..80a4ec1 100644 (file)
@@ -16,6 +16,7 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
 \r
 #include "HiiDatabase.h"\r
 \r
+#define MAX_UINT24    0xFFFFFF\r
 \r
 /**\r
   Get the imageid of last image block: EFI_HII_IIBT_END_BLOCK when input\r
@@ -651,8 +652,16 @@ HiiNewImage (
 \r
   EfiAcquireLock (&mHiiDatabaseLock);\r
 \r
-  NewBlockSize = sizeof (EFI_HII_IIBT_IMAGE_24BIT_BLOCK) - sizeof (EFI_HII_RGB_PIXEL) +\r
-                 BITMAP_LEN_24_BIT ((UINT32) Image->Width, Image->Height);\r
+  //\r
+  // Calcuate the size of new image.\r
+  // Make sure the size doesn't overflow UINT32.\r
+  // Note: 24Bit BMP occpuies 3 bytes per pixel.\r
+  //\r
+  NewBlockSize = (UINT32)Image->Width * Image->Height;\r
+  if (NewBlockSize > (MAX_UINT32 - (sizeof (EFI_HII_IIBT_IMAGE_24BIT_BLOCK) - sizeof (EFI_HII_RGB_PIXEL))) / 3) {\r
+    return EFI_OUT_OF_RESOURCES;\r
+  }\r
+  NewBlockSize = NewBlockSize * 3 + (sizeof (EFI_HII_IIBT_IMAGE_24BIT_BLOCK) - sizeof (EFI_HII_RGB_PIXEL));\r
 \r
   //\r
   // Get the image package in the package list,\r
@@ -671,6 +680,18 @@ HiiNewImage (
     //\r
     // Update the package's image block by appending the new block to the end.\r
     //\r
+\r
+    //\r
+    // Make sure the final package length doesn't overflow.\r
+    // Length of the package header is represented using 24 bits. So MAX length is MAX_UINT24.\r
+    //\r
+    if (NewBlockSize > MAX_UINT24 - ImagePackage->ImagePkgHdr.Header.Length) {\r
+      return EFI_OUT_OF_RESOURCES;\r
+    }\r
+    //\r
+    // Because ImagePackage->ImageBlockSize < ImagePackage->ImagePkgHdr.Header.Length,\r
+    // So (ImagePackage->ImageBlockSize + NewBlockSize) <= MAX_UINT24\r
+    //\r
     ImageBlocks = AllocatePool (ImagePackage->ImageBlockSize + NewBlockSize);\r
     if (ImageBlocks == NULL) {\r
       EfiReleaseLock (&mHiiDatabaseLock);\r
@@ -701,6 +722,13 @@ HiiNewImage (
     PackageListNode->PackageListHdr.PackageLength += NewBlockSize;\r
 \r
   } else {\r
+    //\r
+    // Make sure the final package length doesn't overflow.\r
+    // Length of the package header is represented using 24 bits. So MAX length is MAX_UINT24.\r
+    //\r
+    if (NewBlockSize > MAX_UINT24 - (sizeof (EFI_HII_IMAGE_PACKAGE_HDR) + sizeof (EFI_HII_IIBT_END_BLOCK))) {\r
+      return EFI_OUT_OF_RESOURCES;\r
+    }\r
     //\r
     // The specified package list does not contain image package.\r
     // Create one to add this image block.\r
@@ -902,8 +930,11 @@ IGetImage (
     // Use the common block code since the definition of these structures is the same.\r
     //\r
     CopyMem (&Iibt1bit, CurrentImageBlock, sizeof (EFI_HII_IIBT_IMAGE_1BIT_BLOCK));\r
-    ImageLength = sizeof (EFI_GRAPHICS_OUTPUT_BLT_PIXEL) *\r
-                  ((UINT32) Iibt1bit.Bitmap.Width * Iibt1bit.Bitmap.Height);\r
+    ImageLength = (UINTN) Iibt1bit.Bitmap.Width * Iibt1bit.Bitmap.Height;\r
+    if (ImageLength > MAX_UINTN / sizeof (EFI_GRAPHICS_OUTPUT_BLT_PIXEL)) {\r
+      return EFI_OUT_OF_RESOURCES;\r
+    }\r
+    ImageLength  *= sizeof (EFI_GRAPHICS_OUTPUT_BLT_PIXEL);\r
     Image->Bitmap = AllocateZeroPool (ImageLength);\r
     if (Image->Bitmap == NULL) {\r
       return EFI_OUT_OF_RESOURCES;\r
@@ -952,9 +983,13 @@ IGetImage (
     // fall through\r
     //\r
   case EFI_HII_IIBT_IMAGE_24BIT:\r
-    Width = ReadUnaligned16 ((VOID *) &((EFI_HII_IIBT_IMAGE_24BIT_BLOCK *) CurrentImageBlock)->Bitmap.Width);\r
+    Width  = ReadUnaligned16 ((VOID *) &((EFI_HII_IIBT_IMAGE_24BIT_BLOCK *) CurrentImageBlock)->Bitmap.Width);\r
     Height = ReadUnaligned16 ((VOID *) &((EFI_HII_IIBT_IMAGE_24BIT_BLOCK *) CurrentImageBlock)->Bitmap.Height);\r
-    ImageLength = sizeof (EFI_GRAPHICS_OUTPUT_BLT_PIXEL) * ((UINT32) Width * Height);\r
+    ImageLength = (UINTN)Width * Height;\r
+    if (ImageLength > MAX_UINTN / sizeof (EFI_GRAPHICS_OUTPUT_BLT_PIXEL)) {\r
+      return EFI_OUT_OF_RESOURCES;\r
+    }\r
+    ImageLength  *= sizeof (EFI_GRAPHICS_OUTPUT_BLT_PIXEL);\r
     Image->Bitmap = AllocateZeroPool (ImageLength);\r
     if (Image->Bitmap == NULL) {\r
       return EFI_OUT_OF_RESOURCES;\r
@@ -1124,8 +1159,23 @@ HiiSetImage (
   //\r
   // Create the new image block according to input image.\r
   //\r
-  NewBlockSize = sizeof (EFI_HII_IIBT_IMAGE_24BIT_BLOCK) - sizeof (EFI_HII_RGB_PIXEL) +\r
-                 BITMAP_LEN_24_BIT ((UINT32) Image->Width, Image->Height);\r
+\r
+  //\r
+  // Make sure the final package length doesn't overflow.\r
+  // Length of the package header is represented using 24 bits. So MAX length is MAX_UINT24.\r
+  // 24Bit BMP occpuies 3 bytes per pixel.\r
+  //\r
+  NewBlockSize = (UINT32)Image->Width * Image->Height;\r
+  if (NewBlockSize > (MAX_UINT32 - (sizeof (EFI_HII_IIBT_IMAGE_24BIT_BLOCK) - sizeof (EFI_HII_RGB_PIXEL))) / 3) {\r
+    return EFI_OUT_OF_RESOURCES;\r
+  }\r
+  NewBlockSize = NewBlockSize * 3 + (sizeof (EFI_HII_IIBT_IMAGE_24BIT_BLOCK) - sizeof (EFI_HII_RGB_PIXEL));\r
+  if ((NewBlockSize > OldBlockSize) &&\r
+      (NewBlockSize - OldBlockSize > MAX_UINT24 - ImagePackage->ImagePkgHdr.Header.Length)\r
+      ) {\r
+    return EFI_OUT_OF_RESOURCES;\r
+  }\r
+\r
   //\r
   // Adjust the image package to remove the original block firstly then add the new block.\r
   //\r
@@ -1219,8 +1269,8 @@ HiiDrawImage (
   EFI_IMAGE_OUTPUT                    *ImageOut;\r
   EFI_GRAPHICS_OUTPUT_BLT_PIXEL       *BltBuffer;\r
   UINTN                               BufferLen;\r
-  UINT                              Width;\r
-  UINT                              Height;\r
+  UINT16                              Width;\r
+  UINT16                              Height;\r
   UINTN                               Xpos;\r
   UINTN                               Ypos;\r
   UINTN                               OffsetY1;\r
@@ -1280,6 +1330,13 @@ HiiDrawImage (
   // Otherwise a new bitmap will be allocated to hold this image.\r
   //\r
   if (*Blt != NULL) {\r
+    //\r
+    // Make sure the BltX and BltY is inside the Blt area.\r
+    //\r
+    if ((BltX >= (*Blt)->Width) || (BltY >= (*Blt)->Height)) {\r
+      return EFI_INVALID_PARAMETER;\r
+    }\r
+\r
     //\r
     // Clip the image by (Width, Height)\r
     //\r
@@ -1287,15 +1344,23 @@ HiiDrawImage (
     Width  = Image->Width;\r
     Height = Image->Height;\r
 \r
-    if (Width > (*Blt)->Width - BltX) {\r
-      Width = (*Blt)->Width - BltX;\r
+    if (Width > (*Blt)->Width - (UINT16)BltX) {\r
+      Width = (*Blt)->Width - (UINT16)BltX;\r
     }\r
-    if (Height > (*Blt)->Height - BltY) {\r
-      Height = (*Blt)->Height - BltY;\r
+    if (Height > (*Blt)->Height - (UINT16)BltY) {\r
+      Height = (*Blt)->Height - (UINT16)BltY;\r
     }\r
 \r
-    BufferLen = Width * Height * sizeof (EFI_GRAPHICS_OUTPUT_BLT_PIXEL);\r
-    BltBuffer = (EFI_GRAPHICS_OUTPUT_BLT_PIXEL *) AllocateZeroPool (BufferLen);\r
+    //\r
+    // Prepare the buffer for the temporary image.\r
+    // Make sure the buffer size doesn't overflow UINTN.\r
+    //\r
+    BufferLen = Width * Height;\r
+    if (BufferLen > MAX_UINTN / sizeof (EFI_GRAPHICS_OUTPUT_BLT_PIXEL)) {\r
+      return EFI_OUT_OF_RESOURCES;\r
+    }\r
+    BufferLen *= sizeof (EFI_GRAPHICS_OUTPUT_BLT_PIXEL);\r
+    BltBuffer  = AllocateZeroPool (BufferLen);\r
     if (BltBuffer == NULL) {\r
       return EFI_OUT_OF_RESOURCES;\r
     }\r
@@ -1358,11 +1423,26 @@ HiiDrawImage (
     //\r
     // Allocate a new bitmap to hold the incoming image.\r
     //\r
-    Width  = Image->Width  + BltX;\r
-    Height = Image->Height + BltY;\r
 \r
-    BufferLen = Width * Height * sizeof (EFI_GRAPHICS_OUTPUT_BLT_PIXEL);\r
-    BltBuffer = (EFI_GRAPHICS_OUTPUT_BLT_PIXEL *) AllocateZeroPool (BufferLen);\r
+    //\r
+    // Make sure the final width and height doesn't overflow UINT16.\r
+    //\r
+    if ((BltX > (UINTN)MAX_UINT16 - Image->Width) || (BltY > (UINTN)MAX_UINT16 - Image->Height)) {\r
+      return EFI_INVALID_PARAMETER;\r
+    }\r
+\r
+    Width  = Image->Width  + (UINT16)BltX;\r
+    Height = Image->Height + (UINT16)BltY;\r
+\r
+    //\r
+    // Make sure the output image size doesn't overflow UINTN.\r
+    //\r
+    BufferLen = Width * Height;\r
+    if (BufferLen > MAX_UINTN / sizeof (EFI_GRAPHICS_OUTPUT_BLT_PIXEL)) {\r
+      return EFI_OUT_OF_RESOURCES;\r
+    }\r
+    BufferLen *= sizeof (EFI_GRAPHICS_OUTPUT_BLT_PIXEL);\r
+    BltBuffer  = AllocateZeroPool (BufferLen);\r
     if (BltBuffer == NULL) {\r
       return EFI_OUT_OF_RESOURCES;\r
     }\r
@@ -1372,8 +1452,8 @@ HiiDrawImage (
       FreePool (BltBuffer);\r
       return EFI_OUT_OF_RESOURCES;\r
     }\r
-    ImageOut->Width        = (UINT16) Width;\r
-    ImageOut->Height       = (UINT16) Height;\r
+    ImageOut->Width        = Width;\r
+    ImageOut->Height       = Height;\r
     ImageOut->Image.Bitmap = BltBuffer;\r
 \r
     //\r
@@ -1387,7 +1467,7 @@ HiiDrawImage (
       return Status;\r
     }\r
     ASSERT (FontInfo != NULL);\r
-    for (Index = 0; Index < Width * Height; Index++) {\r
+    for (Index = 0; Index < (UINTN)Width * Height; Index++) {\r
       BltBuffer[Index] = FontInfo->BackgroundColor;\r
     }\r
     FreePool (FontInfo);\r