Add image bounds checking on ELF parser

Author: dgarske

include/elf.h

@@ -166,7 +166,8 @@ typedef struct elf64_program_header {
 #define GET_E32(name) (is_elf32 ? GET32(e32->name) : GET32(e64->name))
 
 typedef int (*elf_mmu_map_cb)(uint64_t, uint64_t, uint32_t);
-int elf_load_image_mmu(uint8_t *image, uintptr_t *pentry, elf_mmu_map_cb mmu_cb);
+int elf_load_image_mmu(uint8_t *image, uint32_t image_sz, uintptr_t *pentry,
+    elf_mmu_map_cb mmu_cb);
 int elf_load_image(uint8_t *image, uintptr_t *entry, int is_ext);
 int64_t elf_hdr_pht_combined_size(const unsigned char* ehdr);
 int elf_open(const unsigned char *ehdr, int *is_elf32);

src/boot_x86_fsp_payload.c

@@ -119,7 +119,7 @@ void do_boot(const uint32_t *app)
     /* TODO: to remove */
     mptable_setup();
     x86_paging_dump_info();
-    r = elf_load_image_mmu((uint8_t *)app, &e, mmu_cb);
+    r = elf_load_image_mmu((uint8_t *)app, WOLFBOOT_PARTITION_SIZE, &e, mmu_cb);
     wolfBoot_printf("Elf loaded (ret %d), entry 0x%x_%x\r\n", r,
                     (uint32_t)(e >> 32),
                     (uint32_t)(e));

src/elf.c

@@ -58,18 +58,25 @@ static int check_scatter_format(const unsigned char* ehdr, int is_elf32);
 /* Loader for elf32 or elf64 format program headers
  * Returns the entry point function
  */
-int elf_load_image_mmu(uint8_t *image, uintptr_t *pentry, elf_mmu_map_cb mmu_cb)
+int elf_load_image_mmu(uint8_t *image, uint32_t image_sz, uintptr_t *pentry,
+    elf_mmu_map_cb mmu_cb)
 {
     elf32_header* h32 = (elf32_header*)image;
     elf64_header* h64 = (elf64_header*)image;
     uint16_t entry_count, entry_size;
     uint8_t *entry_off;
+    uint32_t ph_offset;
     int is_elf32, is_le, i;
 
 #ifdef DEBUG_ELF
     wolfBoot_printf("Loading elf at %p\r\n", (void*)image);
 #endif
 
+    /* Verify image is large enough for the smallest ELF header */
+    if (image_sz < sizeof(elf32_header)) {
+        return -1; /* image too small */
+    }
+
     /* Verify ELF header */
     if (memcmp(h32->ident, ELF_IDENT_STR, 4) != 0) {
         return -1; /* not valid header identifier */
@@ -80,6 +87,11 @@ int elf_load_image_mmu(uint8_t *image, uintptr_t *pentry, elf_mmu_map_cb mmu_cb)
     is_le = (h32->ident[5] == ELF_ENDIAN_LITTLE);
     (void)is_le;
 
+    /* Verify image is large enough for the actual ELF header type */
+    if (!is_elf32 && image_sz < sizeof(elf64_header)) {
+        return -1; /* image too small for elf64 header */
+    }
+
     /* Verify this is an executable */
     if (GET_H16(type) != ELF_HET_EXEC) {
         return -2; /* not executable */
@@ -94,9 +106,19 @@ int elf_load_image_mmu(uint8_t *image, uintptr_t *pentry, elf_mmu_map_cb mmu_cb)
     *pentry = GET_H64(entry);
 
     /* programs */
-    entry_off = image + GET_H32(ph_offset);
+    ph_offset = GET_H32(ph_offset);
     entry_size = GET_H16(ph_entry_size);
     entry_count = GET_H16(ph_entry_count);
+
+    /* Validate program header table is within image bounds */
+    if (ph_offset >= image_sz ||
+        entry_size == 0 ||
+        entry_count > (image_sz / entry_size) ||
+        ph_offset + ((uint32_t)entry_count * entry_size) > image_sz) {
+        return -3; /* program header table out of bounds */
+    }
+    entry_off = image + ph_offset;
+
 #ifdef DEBUG_ELF
     wolfBoot_printf("Program Headers %d (size %d)\r\n", entry_count, entry_size);
 #endif
@@ -115,6 +137,12 @@ int elf_load_image_mmu(uint8_t *image, uintptr_t *pentry, elf_mmu_map_cb mmu_cb)
             continue;
         }
 
+        /* Validate segment data is within image bounds */
+        if (file_size > 0 &&
+            (offset >= image_sz || file_size > image_sz - offset)) {
+            return -4; /* segment offset/size out of bounds */
+        }
+
 #ifdef DEBUG_ELF
         if (file_size > 0) {
             wolfBoot_printf(

src/update_disk.c

@@ -487,7 +487,8 @@ void RAMFUNCTION wolfBoot_start(void)
 #if defined(WOLFBOOT_ELF) && !defined(WOLFBOOT_FSP)
     /* Load elf sections and return the new entry point */
     /* Skip for FSP, since it expects ELF image directly */
-    if (elf_load_image_mmu((uint8_t*)load_address, (uintptr_t*)&load_address, NULL) != 0){
+    if (elf_load_image_mmu((uint8_t*)load_address, os_image.fw_size,
+            (uintptr_t*)&load_address, NULL) != 0){
         wolfBoot_printf("Invalid elf, falling back to raw binary\n");
     }
 #endif

src/update_ram.c

@@ -298,7 +298,8 @@ void RAMFUNCTION wolfBoot_start(void)
 
 #ifdef WOLFBOOT_ELF
     /* Load elf */
-    if (elf_load_image_mmu((uint8_t*)load_address, (uintptr_t*)&load_address, NULL) != 0){
+    if (elf_load_image_mmu((uint8_t*)load_address, os_image.fw_size,
+            (uintptr_t*)&load_address, NULL) != 0){
         wolfBoot_printf("Invalid elf, falling back to raw binary\n");
     }
 #endif

tools/elf-parser/elf-parser.c

@@ -70,7 +70,7 @@ int main(int argc, char *argv[])
     fclose(f);
 
     if (ret == 0) {
-        ret = elf_load_image_mmu(image, &entry, NULL);
+        ret = elf_load_image_mmu(image, (uint32_t)imageSz, &entry, NULL);
     }
 
     printf("Return %d, Load %p\n", ret, (void*)entry);