public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
* [PATCH] UefiPayloadPkg/PayloadLoader: Add more checks to verify ELF images
@ 2021-06-12  6:03 Ni, Ray
  2021-06-28 11:03 ` [edk2-devel] " Marvin Häuser
  0 siblings, 1 reply; 2+ messages in thread
From: Ni, Ray @ 2021-06-12  6:03 UTC (permalink / raw)
  To: devel; +Cc: Marvin Häuser, Guo Dong, Benjamin You

More checks are added to verify ELF image.
ParseElfImage() is changed to InitializeElfContext()

Signed-off-by: Ray Ni <ray.ni@intel.com>
Cc: Marvin Häuser <mhaeuser@posteo.de>
Cc: Guo Dong <guo.dong@intel.com>
Cc: Benjamin You <benjamin.you@intel.com>
---
 UefiPayloadPkg/PayloadLoaderPeim/ElfLib.h     |  11 +-
 .../PayloadLoaderPeim/ElfLib/Elf32Lib.c       |  38 ++--
 .../PayloadLoaderPeim/ElfLib/Elf64Lib.c       |  39 ++--
 .../PayloadLoaderPeim/ElfLib/ElfLib.c         | 210 +++++++++++++-----
 .../PayloadLoaderPeim/PayloadLoaderPeim.c     |   6 +-
 5 files changed, 188 insertions(+), 116 deletions(-)

diff --git a/UefiPayloadPkg/PayloadLoaderPeim/ElfLib.h b/UefiPayloadPkg/PayloadLoaderPeim/ElfLib.h
index 9cfc2912cf..0ed93140a9 100644
--- a/UefiPayloadPkg/PayloadLoaderPeim/ElfLib.h
+++ b/UefiPayloadPkg/PayloadLoaderPeim/ElfLib.h
@@ -17,7 +17,6 @@
 #define  ELF_PT_LOAD   1
 
 typedef struct {
-  RETURN_STATUS ParseStatus;             ///< Return the status after ParseElfImage().
   UINT8         *FileBase;               ///< The source location in memory.
   UINTN         FileSize;                ///< The size including sections that don't require loading.
   UINT8         *PreferredImageAddress;  ///< The preferred image to be loaded. No relocation is needed if loaded to this address.
@@ -45,7 +44,10 @@ typedef struct {
 /**
   Parse the ELF image info.
 
-  @param[in]  ImageBase      Memory address of an image.
+  On return, all fields in ElfCt are updated except ImageAddress.
+
+  @param[in]  FileBase       Memory address of an image.
+  @param[in]  MaxFileSize    The maximum file size.
   @param[out] ElfCt          The EFL image context pointer.
 
   @retval EFI_INVALID_PARAMETER   Input parameters are not valid.
@@ -55,8 +57,9 @@ typedef struct {
 **/
 EFI_STATUS
 EFIAPI
-ParseElfImage (
-  IN  VOID                 *ImageBase,
+InitializeElfContext (
+  IN  VOID                 *FileBase,
+  IN  UINTN                MaxFileSize,
   OUT ELF_IMAGE_CONTEXT    *ElfCt
   );
 
diff --git a/UefiPayloadPkg/PayloadLoaderPeim/ElfLib/Elf32Lib.c b/UefiPayloadPkg/PayloadLoaderPeim/ElfLib/Elf32Lib.c
index 3fa100ce4a..79f4ce623b 100644
--- a/UefiPayloadPkg/PayloadLoaderPeim/ElfLib/Elf32Lib.c
+++ b/UefiPayloadPkg/PayloadLoaderPeim/ElfLib/Elf32Lib.c
@@ -115,7 +115,7 @@ ProcessRelocation32 (
   UINT32                    Type;
 
   for ( Index = 0
-      ; RelaEntrySize * Index < RelaSize
+      ; Index < RelaSize / RelaEntrySize
       ; Index++, Rela = ELF_NEXT_ENTRY (Elf32_Rela, Rela, RelaEntrySize)
       ) {
     //
@@ -137,7 +137,6 @@ ProcessRelocation32 (
           // Dynamic section doesn't contain entries of this type.
           //
           DEBUG ((DEBUG_INFO, "Unsupported relocation type %02X\n", Type));
-          ASSERT (FALSE);
         } else {
           *Ptr += (UINT32) Delta;
         }
@@ -164,7 +163,6 @@ ProcessRelocation32 (
           // Calculation: B + A
           //
           if (RelaType == SHT_RELA) {
-            ASSERT (*Ptr == 0);
             *Ptr = (UINT32) Delta + Rela->r_addend;
           } else {
             //
@@ -177,7 +175,6 @@ ProcessRelocation32 (
           // non-Dynamic section doesn't contain entries of this type.
           //
           DEBUG ((DEBUG_INFO, "Unsupported relocation type %02X\n", Type));
-          ASSERT (FALSE);
         }
         break;
 
@@ -236,12 +233,12 @@ RelocateElf32Dynamic (
   //
   // It's abnormal a DYN ELF doesn't contain a dynamic section.
   //
-  ASSERT (DynShdr != NULL);
   if (DynShdr == NULL) {
     return EFI_UNSUPPORTED;
   }
-  ASSERT (DynShdr->sh_type == SHT_DYNAMIC);
-  ASSERT (DynShdr->sh_entsize >= sizeof (*Dyn));
+  if ((DynShdr->sh_type != SHT_DYNAMIC) || DynShdr->sh_entsize < sizeof (*Dyn)) {
+    return EFI_UNSUPPORTED;
+  }
 
   //
   // 2. Locate the relocation section from the dynamic section.
@@ -286,9 +283,6 @@ RelocateElf32Dynamic (
   }
 
   if (RelaOffset == MAX_UINT64) {
-    ASSERT (RelaCount     == 0);
-    ASSERT (RelaEntrySize == 0);
-    ASSERT (RelaSize      == 0);
     //
     // It's fine that a DYN ELF doesn't contain relocation section.
     //
@@ -299,23 +293,22 @@ RelocateElf32Dynamic (
   // Verify the existence of the relocation section.
   //
   RelShdr = GetElf32SectionByRange (ElfCt->FileBase, RelaOffset, RelaSize);
-  ASSERT (RelShdr != NULL);
   if (RelShdr == NULL) {
     return EFI_UNSUPPORTED;
   }
-  ASSERT (RelShdr->sh_type == RelaType);
-  ASSERT (RelShdr->sh_entsize == RelaEntrySize);
+  if ((RelShdr->sh_type != RelaType) || (RelShdr->sh_entsize != RelaEntrySize)) {
+    return EFI_UNSUPPORTED;
+  }
 
   //
   // 3. Process the relocation section.
   //
-  ProcessRelocation32 (
-    (Elf32_Rela *) (ElfCt->FileBase + RelShdr->sh_offset),
-    RelShdr->sh_size, RelShdr->sh_entsize, RelShdr->sh_type,
-    (UINTN) ElfCt->ImageAddress - (UINTN) ElfCt->PreferredImageAddress,
-    TRUE
-    );
-  return EFI_SUCCESS;
+  return ProcessRelocation32 (
+           (Elf32_Rela *) (ElfCt->FileBase + RelShdr->sh_offset),
+           RelShdr->sh_size, RelShdr->sh_entsize, RelShdr->sh_type,
+           (UINTN) ElfCt->ImageAddress - (UINTN) ElfCt->PreferredImageAddress,
+           TRUE
+           );
 }
 
 /**
@@ -331,7 +324,6 @@ RelocateElf32Sections  (
   IN    ELF_IMAGE_CONTEXT      *ElfCt
   )
 {
-  EFI_STATUS       Status;
   Elf32_Ehdr      *Ehdr;
   Elf32_Shdr      *RelShdr;
   Elf32_Shdr      *Shdr;
@@ -351,9 +343,7 @@ RelocateElf32Sections  (
   //
   if (Ehdr->e_type == ET_DYN) {
     DEBUG ((DEBUG_INFO, "DYN ELF: Relocate using dynamic sections...\n"));
-    Status = RelocateElf32Dynamic (ElfCt);
-    ASSERT_EFI_ERROR (Status);
-    return Status;
+    return RelocateElf32Dynamic (ElfCt);
   }
 
   //
diff --git a/UefiPayloadPkg/PayloadLoaderPeim/ElfLib/Elf64Lib.c b/UefiPayloadPkg/PayloadLoaderPeim/ElfLib/Elf64Lib.c
index e364807007..cfe70639ca 100644
--- a/UefiPayloadPkg/PayloadLoaderPeim/ElfLib/Elf64Lib.c
+++ b/UefiPayloadPkg/PayloadLoaderPeim/ElfLib/Elf64Lib.c
@@ -115,7 +115,7 @@ ProcessRelocation64 (
   UINT32                    Type;
 
   for ( Index = 0
-      ; MultU64x64 (RelaEntrySize, Index) < RelaSize
+      ; Index < DivU64x64Remainder (RelaSize, RelaEntrySize, NULL)
       ; Index++, Rela = ELF_NEXT_ENTRY (Elf64_Rela, Rela, RelaEntrySize)
       ) {
     //
@@ -138,7 +138,6 @@ ProcessRelocation64 (
           // Dynamic section doesn't contain entries of this type.
           //
           DEBUG ((DEBUG_INFO, "Unsupported relocation type %02X\n", Type));
-          ASSERT (FALSE);
         } else {
           *Ptr += Delta;
         }
@@ -149,7 +148,6 @@ ProcessRelocation64 (
         // Dynamic section doesn't contain entries of this type.
         //
         DEBUG ((DEBUG_INFO, "Unsupported relocation type %02X\n", Type));
-        ASSERT (FALSE);
         break;
 
       case R_X86_64_RELATIVE:
@@ -173,7 +171,6 @@ ProcessRelocation64 (
           // Calculation: B + A
           //
           if (RelaType == SHT_RELA) {
-            ASSERT (*Ptr == 0);
             *Ptr = Delta + Rela->r_addend;
           } else {
             //
@@ -186,7 +183,6 @@ ProcessRelocation64 (
           // non-Dynamic section doesn't contain entries of this type.
           //
           DEBUG ((DEBUG_INFO, "Unsupported relocation type %02X\n", Type));
-          ASSERT (FALSE);
         }
         break;
 
@@ -245,12 +241,12 @@ RelocateElf64Dynamic (
   //
   // It's abnormal a DYN ELF doesn't contain a dynamic section.
   //
-  ASSERT (DynShdr != NULL);
   if (DynShdr == NULL) {
     return EFI_UNSUPPORTED;
   }
-  ASSERT (DynShdr->sh_type == SHT_DYNAMIC);
-  ASSERT (DynShdr->sh_entsize >= sizeof (*Dyn));
+  if ((DynShdr->sh_type != SHT_DYNAMIC) || DynShdr->sh_entsize < sizeof (*Dyn)) {
+    return EFI_UNSUPPORTED;
+  }
 
   //
   // 2. Locate the relocation section from the dynamic section.
@@ -295,9 +291,6 @@ RelocateElf64Dynamic (
   }
 
   if (RelaOffset == MAX_UINT64) {
-    ASSERT (RelaCount     == 0);
-    ASSERT (RelaEntrySize == 0);
-    ASSERT (RelaSize      == 0);
     //
     // It's fine that a DYN ELF doesn't contain relocation section.
     //
@@ -308,23 +301,22 @@ RelocateElf64Dynamic (
   // Verify the existence of the relocation section.
   //
   RelShdr = GetElf64SectionByRange (ElfCt->FileBase, RelaOffset, RelaSize);
-  ASSERT (RelShdr != NULL);
   if (RelShdr == NULL) {
     return EFI_UNSUPPORTED;
   }
-  ASSERT (RelShdr->sh_type == RelaType);
-  ASSERT (RelShdr->sh_entsize == RelaEntrySize);
+  if ((RelShdr->sh_type != RelaType) || (RelShdr->sh_entsize != RelaEntrySize)) {
+    return EFI_UNSUPPORTED;
+  }
 
   //
   // 3. Process the relocation section.
   //
-  ProcessRelocation64 (
-    (Elf64_Rela *) (ElfCt->FileBase + RelShdr->sh_offset),
-    RelShdr->sh_size, RelShdr->sh_entsize, RelShdr->sh_type,
-    (UINTN) ElfCt->ImageAddress - (UINTN) ElfCt->PreferredImageAddress,
-    TRUE
-    );
-  return EFI_SUCCESS;
+  return ProcessRelocation64 (
+           (Elf64_Rela *) (ElfCt->FileBase + RelShdr->sh_offset),
+           RelShdr->sh_size, RelShdr->sh_entsize, RelShdr->sh_type,
+           (UINTN) ElfCt->ImageAddress - (UINTN) ElfCt->PreferredImageAddress,
+           TRUE
+           );
 }
 
 /**
@@ -340,7 +332,6 @@ RelocateElf64Sections  (
   IN    ELF_IMAGE_CONTEXT      *ElfCt
   )
 {
-  EFI_STATUS       Status;
   Elf64_Ehdr       *Ehdr;
   Elf64_Shdr       *RelShdr;
   Elf64_Shdr       *Shdr;
@@ -360,9 +351,7 @@ RelocateElf64Sections  (
   //
   if (Ehdr->e_type == ET_DYN) {
     DEBUG ((DEBUG_INFO, "DYN ELF: Relocate using dynamic sections...\n"));
-    Status = RelocateElf64Dynamic (ElfCt);
-    ASSERT_EFI_ERROR (Status);
-    return Status;
+    return RelocateElf64Dynamic (ElfCt);
   }
 
   //
diff --git a/UefiPayloadPkg/PayloadLoaderPeim/ElfLib/ElfLib.c b/UefiPayloadPkg/PayloadLoaderPeim/ElfLib/ElfLib.c
index 531b3486d2..70de81c3ac 100644
--- a/UefiPayloadPkg/PayloadLoaderPeim/ElfLib/ElfLib.c
+++ b/UefiPayloadPkg/PayloadLoaderPeim/ElfLib/ElfLib.c
@@ -11,22 +11,32 @@
 /**
   Check if the ELF image is valid.
 
-  @param[in]  ImageBase       Memory address of an image.
+  @param[in]  FileBase        Memory address of an image.
+  @param[in]  MaxFileSize     The maximum file size.
 
   @retval     TRUE if valid.
 
 **/
 BOOLEAN
 IsElfFormat (
-  IN  CONST UINT8             *ImageBase
+  IN  CONST UINT8             *FileBase,
+  IN  UINTN                   MaxFileSize
   )
 {
   Elf32_Ehdr                  *Elf32Hdr;
   Elf64_Ehdr                  *Elf64Hdr;
 
-  ASSERT (ImageBase != NULL);
+  ASSERT (FileBase != NULL);
 
-  Elf32Hdr = (Elf32_Ehdr *)ImageBase;
+  Elf32Hdr = (Elf32_Ehdr *)FileBase;
+  Elf64Hdr = (Elf64_Ehdr *)FileBase;
+
+  //
+  // Make sure MaxFileSize covers e_ident[].
+  //
+  if (MaxFileSize < sizeof (Elf32Hdr->e_ident)) {
+    return FALSE;
+  }
 
   //
   // Start with correct signature "\7fELF"
@@ -50,15 +60,13 @@ IsElfFormat (
   // Check 32/64-bit architecture
   //
   if (Elf32Hdr->e_ident[EI_CLASS] == ELFCLASS64) {
-    Elf64Hdr = (Elf64_Ehdr *)Elf32Hdr;
-    Elf32Hdr = NULL;
-  } else if (Elf32Hdr->e_ident[EI_CLASS] == ELFCLASS32) {
-    Elf64Hdr = NULL;
-  } else {
-    return FALSE;
-  }
+    //
+    // Before accessing fields in Elf64_Ehdr, make sure the MaxFileSize covers the entire header.
+    //
+    if (MaxFileSize < sizeof (Elf64_Ehdr)) {
+      return FALSE;
+    }
 
-  if (Elf64Hdr != NULL) {
     //
     // Support intel architecture only for now
     //
@@ -79,7 +87,7 @@ IsElfFormat (
     if (Elf64Hdr->e_version != EV_CURRENT) {
       return FALSE;
     }
-  } else {
+  } else if (Elf32Hdr->e_ident[EI_CLASS] == ELFCLASS32) {
     //
     // Support intel architecture only for now
     //
@@ -100,7 +108,10 @@ IsElfFormat (
     if (Elf32Hdr->e_version != EV_CURRENT) {
       return FALSE;
     }
+  } else {
+    return FALSE;
   }
+
   return TRUE;
 }
 
@@ -108,6 +119,7 @@ IsElfFormat (
   Calculate a ELF file size.
 
   @param[in]  ElfCt               ELF image context pointer.
+  @param[in]  MaxFileSize         The maximum file size.
   @param[out] FileSize            Return the file size.
 
   @retval EFI_INVALID_PARAMETER   ElfCt or SecPos is NULL.
@@ -117,12 +129,12 @@ IsElfFormat (
 EFI_STATUS
 CalculateElfFileSize (
   IN  ELF_IMAGE_CONTEXT    *ElfCt,
+  IN  UINTN                MaxFileSize,
   OUT UINTN                *FileSize
   )
 {
   EFI_STATUS     Status;
-  UINTN          FileSize1;
-  UINTN          FileSize2;
+  UINT32         Index;
   Elf32_Ehdr     *Elf32Hdr;
   Elf64_Ehdr     *Elf64Hdr;
   UINTN          Offset;
@@ -132,24 +144,34 @@ CalculateElfFileSize (
     return EFI_INVALID_PARAMETER;
   }
 
-  // Use last section as end of file
-  Status = GetElfSectionPos (ElfCt, ElfCt->ShNum - 1, &Offset, &Size);
-  if (EFI_ERROR(Status)) {
-    return EFI_UNSUPPORTED;
-  }
-  FileSize1 = Offset + Size;
-
-  // Use end of section header as end of file
-  FileSize2 = 0;
+  //
+  // Optional section headers might exist in the end of file.
+  //
+  *FileSize = 0;
   if (ElfCt->EiClass == ELFCLASS32) {
     Elf32Hdr   = (Elf32_Ehdr *)ElfCt->FileBase;
-    FileSize2 = Elf32Hdr->e_shoff + Elf32Hdr->e_shentsize * Elf32Hdr->e_shnum;
+    *FileSize = Elf32Hdr->e_shoff + Elf32Hdr->e_shentsize * Elf32Hdr->e_shnum;
   } else if (ElfCt->EiClass == ELFCLASS64) {
     Elf64Hdr   = (Elf64_Ehdr *)ElfCt->FileBase;
-    FileSize2 = (UINTN)(Elf64Hdr->e_shoff + Elf64Hdr->e_shentsize * Elf64Hdr->e_shnum);
+    *FileSize = (UINTN)(Elf64Hdr->e_shoff + Elf64Hdr->e_shentsize * Elf64Hdr->e_shnum);
   }
 
-  *FileSize = MAX(FileSize1, FileSize2);
+  //
+  // Get the end of section body.
+  //
+  for (Index = 0; Index < ElfCt->ShNum; Index++) {
+    Status = GetElfSectionPos (ElfCt, Index, &Offset, &Size);
+    if (EFI_ERROR (Status)) {
+      return Status;
+    }
+    if ((Offset >= MaxFileSize) || (Size > MaxFileSize - Offset)) {
+      //
+      // Section body is outside of file range.
+      //
+      return EFI_UNSUPPORTED;
+    }
+    *FileSize = MAX (*FileSize, Offset + Size);
+  }
 
   return EFI_SUCCESS;
 }
@@ -213,7 +235,8 @@ GetElfSegmentInfo (
 
   On return, all fields in ElfCt are updated except ImageAddress.
 
-  @param[in]  ImageBase      Memory address of an image.
+  @param[in]  FileBase       Memory address of an image.
+  @param[in]  MaxFileSize    The maximum file size.
   @param[out] ElfCt          The EFL image context pointer.
 
   @retval EFI_INVALID_PARAMETER   Input parameters are not valid.
@@ -223,8 +246,9 @@ GetElfSegmentInfo (
 **/
 EFI_STATUS
 EFIAPI
-ParseElfImage (
-  IN  VOID                 *ImageBase,
+InitializeElfContext (
+  IN  VOID                 *FileBase,
+  IN  UINTN                MaxFileSize,
   OUT ELF_IMAGE_CONTEXT    *ElfCt
   )
 {
@@ -238,30 +262,58 @@ ParseElfImage (
   UINTN          End;
   UINTN          Base;
 
-  if (ElfCt == NULL) {
-    return EFI_INVALID_PARAMETER;
-  }
+  ASSERT (ElfCt != NULL);
+
   ZeroMem (ElfCt, sizeof(ELF_IMAGE_CONTEXT));
 
-  if (ImageBase == NULL) {
-    return (ElfCt->ParseStatus = EFI_INVALID_PARAMETER);
+  if (FileBase == NULL) {
+    return EFI_INVALID_PARAMETER;
   }
 
-  ElfCt->FileBase = (UINT8 *)ImageBase;
-  if (!IsElfFormat (ElfCt->FileBase)) {
-    return (ElfCt->ParseStatus = EFI_UNSUPPORTED);
+  ElfCt->FileBase = (UINT8 *)FileBase;
+  if (!IsElfFormat (ElfCt->FileBase, MaxFileSize)) {
+    return EFI_UNSUPPORTED;
   }
 
   Elf32Hdr = (Elf32_Ehdr *)ElfCt->FileBase;
   ElfCt->EiClass = Elf32Hdr->e_ident[EI_CLASS];
   if (ElfCt->EiClass == ELFCLASS32) {
     if ((Elf32Hdr->e_type != ET_EXEC) && (Elf32Hdr->e_type != ET_DYN)) {
-      return (ElfCt->ParseStatus = EFI_UNSUPPORTED);
+      return EFI_UNSUPPORTED;
     }
-    Elf32Shdr = (Elf32_Shdr *)GetElf32SectionByIndex (ElfCt->FileBase, Elf32Hdr->e_shstrndx);
+
+    if ((Elf32Hdr->e_phoff >= MaxFileSize) || ((UINT32) (Elf32Hdr->e_phentsize * Elf32Hdr->e_phnum) > MaxFileSize - Elf32Hdr->e_phoff)) {
+      //
+      // Program headers are outside of the file range.
+      //
+      return EFI_UNSUPPORTED;
+    }
+
+    if ((Elf32Hdr->e_shoff >= MaxFileSize) || ((UINT32) (Elf32Hdr->e_shentsize * Elf32Hdr->e_shnum) > MaxFileSize - Elf32Hdr->e_shoff)) {
+      //
+      // Section headers are outside of the file range.
+      //
+      return EFI_UNSUPPORTED;
+    }
+
+    if (Elf32Hdr->e_entry >= MaxFileSize) {
+      //
+      // Entrypoint is outside of the file range.
+      //
+      return EFI_UNSUPPORTED;
+    }
+
+    Elf32Shdr = GetElf32SectionByIndex (ElfCt->FileBase, Elf32Hdr->e_shstrndx);
     if (Elf32Shdr == NULL) {
-      return (ElfCt->ParseStatus = EFI_UNSUPPORTED);
+      return EFI_UNSUPPORTED;
     }
+    if ((Elf32Shdr->sh_offset >= MaxFileSize) || (Elf32Shdr->sh_size > MaxFileSize - Elf32Shdr->sh_offset)) {
+      //
+      // String section is outside of the file range.
+      //
+      return EFI_UNSUPPORTED;
+    }
+
     ElfCt->EntryPoint = (UINTN)Elf32Hdr->e_entry;
     ElfCt->ShNum      = Elf32Hdr->e_shnum;
     ElfCt->PhNum      = Elf32Hdr->e_phnum;
@@ -270,12 +322,41 @@ ParseElfImage (
   } else {
     Elf64Hdr  = (Elf64_Ehdr *)Elf32Hdr;
     if ((Elf64Hdr->e_type != ET_EXEC) && (Elf64Hdr->e_type != ET_DYN)) {
-      return (ElfCt->ParseStatus = EFI_UNSUPPORTED);
+      return EFI_UNSUPPORTED;
+    }
+
+    if ((Elf64Hdr->e_phoff >= MaxFileSize) || ((UINT32) Elf64Hdr->e_phentsize * Elf64Hdr->e_phnum > MaxFileSize - (UINTN) Elf64Hdr->e_phoff)) {
+      //
+      // Program headers are outside of the file range.
+      //
+      return EFI_UNSUPPORTED;
+    }
+
+    if ((Elf64Hdr->e_shoff >= MaxFileSize) || ((UINT32) Elf64Hdr->e_shentsize * Elf64Hdr->e_shnum > MaxFileSize - (UINTN) Elf64Hdr->e_shoff)) {
+      //
+      // Section headers are outside of the file range.
+      //
+      return EFI_UNSUPPORTED;
+    }
+
+    if (Elf64Hdr->e_entry >= MaxFileSize) {
+      //
+      // Entrypoint is outside of the file range.
+      //
+      return EFI_UNSUPPORTED;
     }
-    Elf64Shdr = (Elf64_Shdr *)GetElf64SectionByIndex (ElfCt->FileBase, Elf64Hdr->e_shstrndx);
+
+    Elf64Shdr = GetElf64SectionByIndex (ElfCt->FileBase, Elf64Hdr->e_shstrndx);
     if (Elf64Shdr == NULL) {
-      return (ElfCt->ParseStatus = EFI_UNSUPPORTED);
+      return EFI_UNSUPPORTED;
+    }
+    if ((Elf64Shdr->sh_offset >= MaxFileSize) || (Elf64Shdr->sh_size > MaxFileSize - Elf64Shdr->sh_offset)) {
+      //
+      // String section is outside of the file range.
+      //
+      return EFI_UNSUPPORTED;
     }
+
     ElfCt->EntryPoint = (UINTN)Elf64Hdr->e_entry;
     ElfCt->ShNum      = Elf64Hdr->e_shnum;
     ElfCt->PhNum      = Elf64Hdr->e_phnum;
@@ -297,6 +378,13 @@ ParseElfImage (
       continue;
     }
 
+    //
+    // Loadable process segments must have congruent values for p_vaddr and p_offset, modulo the page size.
+    //
+    if ((SegInfo.MemAddr % EFI_PAGE_SIZE) != (SegInfo.Offset % EFI_PAGE_SIZE)) {
+      return EFI_UNSUPPORTED;
+    }
+
     if (SegInfo.MemLen != SegInfo.Length) {
       //
       // Not enough space to execute at current location.
@@ -317,8 +405,7 @@ ParseElfImage (
   ElfCt->ImageSize             = End - Base + 1;
   ElfCt->PreferredImageAddress = (VOID *) Base;
 
-  CalculateElfFileSize (ElfCt, &ElfCt->FileSize);
-  return (ElfCt->ParseStatus = EFI_SUCCESS);;
+  return CalculateElfFileSize (ElfCt, MaxFileSize, &ElfCt->FileSize);
 }
 
 /**
@@ -348,10 +435,6 @@ LoadElfImage (
     return EFI_INVALID_PARAMETER;
   }
 
-  if (EFI_ERROR (ElfCt->ParseStatus)) {
-    return ElfCt->ParseStatus;
-  }
-
   if (ElfCt->ImageAddress == NULL) {
     return EFI_INVALID_PARAMETER;
   }
@@ -370,6 +453,8 @@ LoadElfImage (
 /**
   Get a ELF section name from its index.
 
+  ElfCt is returned from InitializeElfContext().
+
   @param[in]  ElfCt               ELF image context pointer.
   @param[in]  SectionIndex        ELF section index.
   @param[out] SectionName         The pointer to the section name.
@@ -389,25 +474,25 @@ GetElfSectionName (
   Elf32_Shdr      *Elf32Shdr;
   Elf64_Shdr      *Elf64Shdr;
   CHAR8           *Name;
+  UINTN           MaxSize;
 
   if ((ElfCt == NULL) || (SectionName == NULL)) {
     return EFI_INVALID_PARAMETER;
   }
 
-  if (EFI_ERROR (ElfCt->ParseStatus)) {
-    return ElfCt->ParseStatus;
-  }
-
-  Name = NULL;
+  Name    = NULL;
+  MaxSize = 0;
   if (ElfCt->EiClass == ELFCLASS32) {
     Elf32Shdr = GetElf32SectionByIndex (ElfCt->FileBase, SectionIndex);
     if ((Elf32Shdr != NULL) && (Elf32Shdr->sh_name < ElfCt->ShStrLen)) {
-      Name = (CHAR8 *)(ElfCt->FileBase + ElfCt->ShStrOff + Elf32Shdr->sh_name);
+      Name    = (CHAR8 *)(ElfCt->FileBase + ElfCt->ShStrOff + Elf32Shdr->sh_name);
+      MaxSize = ElfCt->ShStrLen - Elf32Shdr->sh_name;
     }
   } else if (ElfCt->EiClass == ELFCLASS64) {
     Elf64Shdr = GetElf64SectionByIndex (ElfCt->FileBase, SectionIndex);
     if ((Elf64Shdr != NULL) && (Elf64Shdr->sh_name < ElfCt->ShStrLen)) {
-      Name = (CHAR8 *)(ElfCt->FileBase + ElfCt->ShStrOff + Elf64Shdr->sh_name);
+      Name    = (CHAR8 *)(ElfCt->FileBase + ElfCt->ShStrOff + Elf64Shdr->sh_name);
+      MaxSize = ElfCt->ShStrLen - Elf64Shdr->sh_name;
     }
   }
 
@@ -415,6 +500,13 @@ GetElfSectionName (
     return EFI_NOT_FOUND;
   }
 
+  if (AsciiStrnLenS (Name, MaxSize) == MaxSize) {
+    //
+    // No null terminator is found for the section name.
+    //
+    return EFI_NOT_FOUND;
+  }
+
   *SectionName = Name;
   return EFI_SUCCESS;
 }
@@ -449,10 +541,6 @@ GetElfSectionPos (
     return EFI_INVALID_PARAMETER;
   }
 
-  if (EFI_ERROR (ElfCt->ParseStatus)) {
-    return ElfCt->ParseStatus;
-  }
-
   if (ElfCt->EiClass == ELFCLASS32) {
     Elf32Shdr = GetElf32SectionByIndex (ElfCt->FileBase, Index);
     if (Elf32Shdr != NULL) {
diff --git a/UefiPayloadPkg/PayloadLoaderPeim/PayloadLoaderPeim.c b/UefiPayloadPkg/PayloadLoaderPeim/PayloadLoaderPeim.c
index 44639f9fd2..efedaef1b3 100644
--- a/UefiPayloadPkg/PayloadLoaderPeim/PayloadLoaderPeim.c
+++ b/UefiPayloadPkg/PayloadLoaderPeim/PayloadLoaderPeim.c
@@ -69,8 +69,10 @@ PeiLoadFileLoadPayload (
       return Status;
     }
 
-    ZeroMem (&Context, sizeof (Context));
-    Status = ParseElfImage (Elf, &Context);
+    //
+    // Trust the ELF image loaded from FV.
+    //
+    Status = InitializeElfContext (Elf, MAX_UINTN - (UINTN) Elf, &Context);
   } while (EFI_ERROR (Status));
 
   DEBUG ((
-- 
2.31.1.windows.1


^ permalink raw reply related	[flat|nested] 2+ messages in thread
end of thread, other threads:[~2021-06-28 11:03 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2021-06-12  6:03 [PATCH] UefiPayloadPkg/PayloadLoader: Add more checks to verify ELF images Ni, Ray
2021-06-28 11:03 ` [edk2-devel] " Marvin Häuser

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox