Pei: Critical buffer overflow when loading TE Images

Dong, Eric Mon, 18 Feb 2013 23:57:58 -0800

Oliver,

In red mark code is used to fix the alignment issue raised in IPF platform, we 
can't remove it. For TE image, it also needs to make sure the section data is 
section alignment. So in my patch, i just expand the memory size to the PE 
image size for TE image. Attach my patch; please help to verify this patch.


Thanks,
Eric
From: Olivier Martin [mailto:olivier.mar...@arm.com]
Sent: Tuesday, February 19, 2013 12:57 AM
To: Tian, Feng; Kinney, Michael D
Cc: edk2-devel@lists.sourceforge.net; Harry Liebel
Subject: [edk2] [PATCH] MdeModulePkg/Core/Pei: Critical buffer overflow when 
loading TE Images

Dear MdePkg and MdeModulePkg maintainers,

We found a buffer overflow in the TE image loading. This issue is architecture 
independent and could potentially crash the platform.
The issue has been introduced by this commit: "Fix alignment requirement when 
Load IPF TeImage into memory" 
(https://github.com/tianocore/edk2/commit/4e844595f27ba8031b1b72fb3e3ab16bcf246ebc)

# TE images are loaded by PEI Core during the PI phase with the function 
LoadAndRelocatePeCoffImage() (in MdeModulePkg/Core/Pei/Image/Image.c).

LoadAndRelocatePeCoffImage() {
// Get the Image information (including the total size of the TE image)
PeCoffLoaderGetImageInfo (&ImageContext);

// Allocate the memory for the image:
ImageContext.ImageAddress = (EFI_PHYSICAL_ADDRESS)(UINTN) AllocatePages 
(EFI_SIZE_TO_PAGES ((UINT32) ImageContext.ImageSize));

// Skip the reserved space for the stripped PeHeader when load TeImage into 
memory.
if (ImageContext.IsTeImage) {
  ImageContext.ImageAddress = ImageContext.ImageAddress +
                              ((EFI_TE_IMAGE_HEADER *) Pe32Data)->StrippedSize -
                              sizeof (EFI_TE_IMAGE_HEADER);
}

// Load the image to our new buffer
Status = PeCoffLoaderLoadImage (&ImageContext);
}

# PeCoffLoaderGetImageInfo() (in MdePkg/Library/BasePeCoffLib) calculates the 
size of the not-yet loaded image including the size of the header 
(=sizeof(EFI_TE_IMAGE_HEADER))

PeCoffLoaderGetImageInfo() {
// Image Base Address
TeStrippedOffset = (UINT32)Hdr.Te->StrippedSize - sizeof (EFI_TE_IMAGE_HEADER);
ImageContext->ImageAddress = (PHYSICAL_ADDRESS)(Hdr.Te->ImageBase + 
TeStrippedOffset);

// Calculate the size
SectionHeaderOffset = (UINTN)(sizeof (EFI_TE_IMAGE_HEADER));
for (Index = 0; Index < Hdr.Te->NumberOfSections;) {
  (...)


  //
  // In Te image header there is not a field to describe the ImageSize.
  // Actually, the ImageSize equals the RVA plus the VirtualSize of
  // the last section mapped into memory (Must be rounded up to
  // a multiple of Section Alignment). Per the PE/COFF specification, the
  // section headers in the Section Table must appear in order of the RVA
  // values for the corresponding sections. So the ImageSize can be determined
  // by the RVA and the VirtualSize of the last section header in the
  // Section Table.
  //
  if ((++Index) == (UINTN)Hdr.Te->NumberOfSections) {
    ImageContext->ImageSize = (SectionHeader.VirtualAddress + 
SectionHeader.Misc.VirtualSize) - TeStrippedOffset;
  }
}
}

# And finally the TE image is loaded with the function PeCoffLoaderLoadImage() 
(in MdePkg/Library/BasePeCoffLib):

PeCoffLoaderLoadImage() {
BaseAddress = ImageContext->ImageAddress;
// Read the TE header
Status = ImageContext->ImageRead (
                        ImageContext->Handle,
                        0,
                        &ImageContext->SizeOfHeaders,
                        (void *)(UINTN)ImageContext->ImageAddress
                        );

FirstSection = (EFI_IMAGE_SECTION_HEADER *) (
                      (UINTN)ImageContext->ImageAddress +
                      sizeof(EFI_TE_IMAGE_HEADER)
                      );

Section = FirstSection;
for (Index = 0; Index < NumberOfSections; Index++) {
  Status = ImageContext->ImageRead (
                            ImageContext->Handle,
                            Section->PointerToRawData - TeStrippedOffset,
                            &Size,
                            Base
                            );
}
}

The faulty code is in RED. If we suppose the size which is calculated by 
PeCoffLoaderGetImageInfo() is correct and includes the TE image header then 
changing the base address of the (not-yet) loaded image will potentially create 
an overflow (as we load the TE image header from this new base address).

At the least the code in RED must be removed. But some additional code might be 
needed to fix the alignment of the code section and the image size calculation 
to take in account this alignment.
But these fixes would go into MdePkg/Library/BasePeCoffLib.

The reason why it generally works is because the allocated region is rounded to 
a EFI_PAGE_SIZE granularity. So, we could have some spare bytes for the 
overflow.
The PI module that was causing the overflow for us is PEI Core. With our 
toolchain, PEI core is almost exactly 122880Bytes = (EFI_PAGE_SIZE * 30).

Regards,
Olivier

Image.c.patch
Description: Image.c.patch

------------------------------------------------------------------------------
Everyone hates slow websites. So do we.
Make your web apps faster with AppDynamics
Download AppDynamics Lite for free today:
http://p.sf.net/sfu/appdyn_d2d_feb

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/edk2-devel

Re: [edk2] [PATCH] MdeModulePkg/Core/Pei: Critical buffer overflow when loading TE Images

Reply via email to