Sign Up
Log In
Log In
or
Sign Up
Places
All Projects
Status Monitor
Collapse sidebar
SUSE:SLE-15-SP2:GA
ovmf.10547
ovmf-bsc1128503-fix-stack-overflow-in-HiiImage-...
Overview
Repositories
Revisions
Requests
Users
Attributes
Meta
File ovmf-bsc1128503-fix-stack-overflow-in-HiiImage-and-HiiDatabase.patch of Package ovmf.10547
From d3743fad8d9e3d3877849fa4d099a7d20a0ca256 Mon Sep 17 00:00:00 2001 From: Ray Ni <ray.ni@intel.com> Date: Thu, 7 Mar 2019 18:35:13 +0800 Subject: [PATCH 1/2] MdeModulePkg/HiiDatabase: Fix potential integer overflow (CVE-2018-12181) 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> (cherry picked from commit ffe5f7a6b4e978dffbe1df228963adc914451106) --- MdeModulePkg/Universal/HiiDatabaseDxe/Image.c | 126 ++++++++++++++---- 1 file changed, 103 insertions(+), 23 deletions(-) diff --git a/MdeModulePkg/Universal/HiiDatabaseDxe/Image.c b/MdeModulePkg/Universal/HiiDatabaseDxe/Image.c index 431a5b84543b..dc9566b736cb 100644 --- a/MdeModulePkg/Universal/HiiDatabaseDxe/Image.c +++ b/MdeModulePkg/Universal/HiiDatabaseDxe/Image.c @@ -16,6 +16,7 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED. #include "HiiDatabase.h" +#define MAX_UINT24 0xFFFFFF /** Get the imageid of last image block: EFI_HII_IIBT_END_BLOCK when input @@ -649,8 +650,16 @@ HiiNewImage ( return EFI_NOT_FOUND; } - NewBlockSize = sizeof (EFI_HII_IIBT_IMAGE_24BIT_BLOCK) - sizeof (EFI_HII_RGB_PIXEL) + - BITMAP_LEN_24_BIT ((UINT32) Image->Width, Image->Height); + // + // Calcuate the size of new image. + // Make sure the size doesn't overflow UINT32. + // Note: 24Bit BMP occpuies 3 bytes per pixel. + // + NewBlockSize = (UINT32)Image->Width * Image->Height; + if (NewBlockSize > (MAX_UINT32 - (sizeof (EFI_HII_IIBT_IMAGE_24BIT_BLOCK) - sizeof (EFI_HII_RGB_PIXEL))) / 3) { + return EFI_OUT_OF_RESOURCES; + } + NewBlockSize = NewBlockSize * 3 + (sizeof (EFI_HII_IIBT_IMAGE_24BIT_BLOCK) - sizeof (EFI_HII_RGB_PIXEL)); // // Get the image package in the package list, @@ -669,6 +678,18 @@ HiiNewImage ( // // Update the package's image block by appending the new block to the end. // + + // + // Make sure the final package length doesn't overflow. + // Length of the package header is represented using 24 bits. So MAX length is MAX_UINT24. + // + if (NewBlockSize > MAX_UINT24 - ImagePackage->ImagePkgHdr.Header.Length) { + return EFI_OUT_OF_RESOURCES; + } + // + // Because ImagePackage->ImageBlockSize < ImagePackage->ImagePkgHdr.Header.Length, + // So (ImagePackage->ImageBlockSize + NewBlockSize) <= MAX_UINT24 + // ImageBlocks = AllocatePool (ImagePackage->ImageBlockSize + NewBlockSize); if (ImageBlocks == NULL) { return EFI_OUT_OF_RESOURCES; @@ -698,6 +719,13 @@ HiiNewImage ( PackageListNode->PackageListHdr.PackageLength += NewBlockSize; } else { + // + // Make sure the final package length doesn't overflow. + // Length of the package header is represented using 24 bits. So MAX length is MAX_UINT24. + // + if (NewBlockSize > MAX_UINT24 - (sizeof (EFI_HII_IMAGE_PACKAGE_HDR) + sizeof (EFI_HII_IIBT_END_BLOCK))) { + return EFI_OUT_OF_RESOURCES; + } // // The specified package list does not contain image package. // Create one to add this image block. @@ -895,8 +923,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) * - ((UINT32) Iibt1bit.Bitmap.Width * Iibt1bit.Bitmap.Height); + ImageLength = (UINTN) Iibt1bit.Bitmap.Width * Iibt1bit.Bitmap.Height; + if (ImageLength > MAX_UINTN / sizeof (EFI_GRAPHICS_OUTPUT_BLT_PIXEL)) { + return EFI_OUT_OF_RESOURCES; + } + ImageLength *= sizeof (EFI_GRAPHICS_OUTPUT_BLT_PIXEL); Image->Bitmap = AllocateZeroPool (ImageLength); if (Image->Bitmap == NULL) { return EFI_OUT_OF_RESOURCES; @@ -945,9 +976,13 @@ IGetImage ( // fall through // case EFI_HII_IIBT_IMAGE_24BIT: - Width = ReadUnaligned16 ((VOID *) &((EFI_HII_IIBT_IMAGE_24BIT_BLOCK *) CurrentImageBlock)->Bitmap.Width); + 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) * ((UINT32) Width * Height); + ImageLength = (UINTN)Width * Height; + if (ImageLength > MAX_UINTN / sizeof (EFI_GRAPHICS_OUTPUT_BLT_PIXEL)) { + return EFI_OUT_OF_RESOURCES; + } + ImageLength *= sizeof (EFI_GRAPHICS_OUTPUT_BLT_PIXEL); Image->Bitmap = AllocateZeroPool (ImageLength); if (Image->Bitmap == NULL) { return EFI_OUT_OF_RESOURCES; @@ -1114,8 +1149,23 @@ 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 ((UINT32) Image->Width, Image->Height); + + // + // Make sure the final package length doesn't overflow. + // Length of the package header is represented using 24 bits. So MAX length is MAX_UINT24. + // 24Bit BMP occpuies 3 bytes per pixel. + // + NewBlockSize = (UINT32)Image->Width * Image->Height; + if (NewBlockSize > (MAX_UINT32 - (sizeof (EFI_HII_IIBT_IMAGE_24BIT_BLOCK) - sizeof (EFI_HII_RGB_PIXEL))) / 3) { + return EFI_OUT_OF_RESOURCES; + } + NewBlockSize = NewBlockSize * 3 + (sizeof (EFI_HII_IIBT_IMAGE_24BIT_BLOCK) - sizeof (EFI_HII_RGB_PIXEL)); + if ((NewBlockSize > OldBlockSize) && + (NewBlockSize - OldBlockSize > MAX_UINT24 - ImagePackage->ImagePkgHdr.Header.Length) + ) { + return EFI_OUT_OF_RESOURCES; + } + // // Adjust the image package to remove the original block firstly then add the new block. // @@ -1207,8 +1257,8 @@ HiiDrawImage ( EFI_IMAGE_OUTPUT *ImageOut; EFI_GRAPHICS_OUTPUT_BLT_PIXEL *BltBuffer; UINTN BufferLen; - UINTN Width; - UINTN Height; + UINT16 Width; + UINT16 Height; UINTN Xpos; UINTN Ypos; UINTN OffsetY1; @@ -1268,6 +1318,13 @@ HiiDrawImage ( // Otherwise a new bitmap will be allocated to hold this image. // if (*Blt != NULL) { + // + // Make sure the BltX and BltY is inside the Blt area. + // + if ((BltX >= (*Blt)->Width) || (BltY >= (*Blt)->Height)) { + return EFI_INVALID_PARAMETER; + } + // // Clip the image by (Width, Height) // @@ -1275,15 +1332,23 @@ HiiDrawImage ( Width = Image->Width; Height = Image->Height; - if (Width > (*Blt)->Width - BltX) { - Width = (*Blt)->Width - BltX; + if (Width > (*Blt)->Width - (UINT16)BltX) { + Width = (*Blt)->Width - (UINT16)BltX; } - if (Height > (*Blt)->Height - BltY) { - Height = (*Blt)->Height - BltY; + if (Height > (*Blt)->Height - (UINT16)BltY) { + Height = (*Blt)->Height - (UINT16)BltY; } - BufferLen = Width * Height * sizeof (EFI_GRAPHICS_OUTPUT_BLT_PIXEL); - BltBuffer = (EFI_GRAPHICS_OUTPUT_BLT_PIXEL *) AllocateZeroPool (BufferLen); + // + // Prepare the buffer for the temporary image. + // Make sure the buffer size doesn't overflow UINTN. + // + BufferLen = Width * Height; + if (BufferLen > MAX_UINTN / sizeof (EFI_GRAPHICS_OUTPUT_BLT_PIXEL)) { + return EFI_OUT_OF_RESOURCES; + } + BufferLen *= sizeof (EFI_GRAPHICS_OUTPUT_BLT_PIXEL); + BltBuffer = AllocateZeroPool (BufferLen); if (BltBuffer == NULL) { return EFI_OUT_OF_RESOURCES; } @@ -1346,11 +1411,26 @@ HiiDrawImage ( // // Allocate a new bitmap to hold the incoming image. // - Width = Image->Width + BltX; - Height = Image->Height + BltY; - BufferLen = Width * Height * sizeof (EFI_GRAPHICS_OUTPUT_BLT_PIXEL); - BltBuffer = (EFI_GRAPHICS_OUTPUT_BLT_PIXEL *) AllocateZeroPool (BufferLen); + // + // Make sure the final width and height doesn't overflow UINT16. + // + if ((BltX > (UINTN)MAX_UINT16 - Image->Width) || (BltY > (UINTN)MAX_UINT16 - Image->Height)) { + return EFI_INVALID_PARAMETER; + } + + Width = Image->Width + (UINT16)BltX; + Height = Image->Height + (UINT16)BltY; + + // + // Make sure the output image size doesn't overflow UINTN. + // + BufferLen = Width * Height; + if (BufferLen > MAX_UINTN / sizeof (EFI_GRAPHICS_OUTPUT_BLT_PIXEL)) { + return EFI_OUT_OF_RESOURCES; + } + BufferLen *= sizeof (EFI_GRAPHICS_OUTPUT_BLT_PIXEL); + BltBuffer = AllocateZeroPool (BufferLen); if (BltBuffer == NULL) { return EFI_OUT_OF_RESOURCES; } @@ -1360,8 +1440,8 @@ HiiDrawImage ( FreePool (BltBuffer); return EFI_OUT_OF_RESOURCES; } - ImageOut->Width = (UINT16) Width; - ImageOut->Height = (UINT16) Height; + ImageOut->Width = Width; + ImageOut->Height = Height; ImageOut->Image.Bitmap = BltBuffer; // @@ -1375,7 +1455,7 @@ HiiDrawImage ( return Status; } ASSERT (FontInfo != NULL); - for (Index = 0; Index < Width * Height; Index++) { + for (Index = 0; Index < (UINTN)Width * Height; Index++) { BltBuffer[Index] = FontInfo->BackgroundColor; } FreePool (FontInfo); -- 2.20.1 From 00fe2260ceb106e2bfe69312c9a17b7ab1658fe5 Mon Sep 17 00:00:00 2001 From: Ray Ni <ray.ni@intel.com> Date: Thu, 7 Mar 2019 18:35:14 +0800 Subject: [PATCH 2/2] MdeModulePkg/HiiImage: Fix stack overflow when corrupted BMP is parsed (CVE-2018-12181) REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1135 For 4bit BMP, there are only 2^4 = 16 colors in the palette. But when a corrupted BMP contains more than 16 colors in the palette, today's implementation wrongly copies all colors to the local PaletteValue[16] array which causes stack overflow. The similar issue also exists in the logic to handle 8bit BMP. The patch fixes the issue by only copies the first 16 or 256 colors in the palette depending on the BMP type. Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Ray Ni <ray.ni@intel.com> Cc: Liming Gao <liming.gao@intel.com> Cc: Jiewen Yao <jiewen.yao@intel.com> Reviewed-by: Jian J Wang <jian.j.wang@intel.com> (cherry picked from commit 89910a39dcfd788057caa5d88b7e76e112d187b5) --- MdeModulePkg/Universal/HiiDatabaseDxe/Image.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/MdeModulePkg/Universal/HiiDatabaseDxe/Image.c b/MdeModulePkg/Universal/HiiDatabaseDxe/Image.c index dc9566b736cb..9829bddaa804 100644 --- a/MdeModulePkg/Universal/HiiDatabaseDxe/Image.c +++ b/MdeModulePkg/Universal/HiiDatabaseDxe/Image.c @@ -370,7 +370,7 @@ Output4bitPixel ( PaletteNum = (UINT16)(Palette->PaletteSize / sizeof (EFI_HII_RGB_PIXEL)); ZeroMem (PaletteValue, sizeof (PaletteValue)); - CopyRgbToGopPixel (PaletteValue, Palette->PaletteValue, PaletteNum); + CopyRgbToGopPixel (PaletteValue, Palette->PaletteValue, MIN (PaletteNum, ARRAY_SIZE (PaletteValue))); FreePool (Palette); // @@ -447,7 +447,7 @@ Output8bitPixel ( CopyMem (Palette, PaletteInfo, PaletteSize); PaletteNum = (UINT16)(Palette->PaletteSize / sizeof (EFI_HII_RGB_PIXEL)); ZeroMem (PaletteValue, sizeof (PaletteValue)); - CopyRgbToGopPixel (PaletteValue, Palette->PaletteValue, PaletteNum); + CopyRgbToGopPixel (PaletteValue, Palette->PaletteValue, MIN (PaletteNum, ARRAY_SIZE (PaletteValue))); FreePool (Palette); // -- 2.20.1
Locations
Projects
Search
Status Monitor
Help
OpenBuildService.org
Documentation
API Documentation
Code of Conduct
Contact
Support
@OBShq
Terms
openSUSE Build Service is sponsored by
The Open Build Service is an
openSUSE project
.
Sign Up
Log In
Places
Places
All Projects
Status Monitor
