elf: Validate elf section header table index for section name string table

In multiboot_elfxx.c, e_shstrndx is used to obtain the section header table
index of the section name string table, but it wasn't being checked if the value
was there.

According to the elf(5) manual page,
"If the index of section name string table section is larger than or equal to
SHN_LORESERVE (0xff00), this member holds SHN_XINDEX (0xffff) and the real
index of the section name string table section is held in the sh_link member of
the initial entry in section header table. Otherwise, the sh_link member of the
initial entry in section header table contains the value zero."

Since this check wasn't being made, grub_elfXX_get_shstrndx() is being added to
elfXX.c to make this check and use e_shstrndx if it doesn't have SHN_XINDEX as a
value, else use sh_link. We also need to make sure e_shstrndx isn't greater than
or equal to SHN_LORESERVE and sh_link isn't less than SHN_LORESERVE.

Note that even though elf.c and elfXX.c are located in grub-core/kern, they are
compiled as modules and don't need the EXPORT_FUNC() macro to define the functions
in elf.h.

Signed-off-by: Alec Brown <alec.r.brown@oracle.com>
Reviewed-by: Darren Kenny <darren.kenny@oracle.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>

diff --git a/grub-core/kern/elf.c b/grub-core/kern/elf.c
index 66b69293ed05bc0a1804aebae8e5e72f8472394d..c676db69493e41a670da7cab3c66bf9b1042b3c1 100644 (file)
--- a/grub-core/kern/elf.c
+++ b/grub-core/kern/elf.c
@@ -176,6 +176,7 @@ grub_elf_open (const char *name, enum grub_file_type type)
 #define grub_swap_bytes_XwordXX grub_swap_bytes32
 #define grub_elfXX_check_endianess_and_bswap_ehdr grub_elf32_check_endianess_and_bswap_ehdr
 #define grub_elfXX_get_shnum grub_elf32_get_shnum
+#define grub_elfXX_get_shstrndx grub_elf32_get_shstrndx
 
 #include "elfXX.c"
 
@@ -198,6 +199,7 @@ grub_elf_open (const char *name, enum grub_file_type type)
 #undef grub_swap_bytes_XwordXX
 #undef grub_elfXX_check_endianess_and_bswap_ehdr
 #undef grub_elfXX_get_shnum
+#undef grub_elfXX_get_shstrndx
 
 \f
 /* 64-bit */
@@ -220,5 +222,6 @@ grub_elf_open (const char *name, enum grub_file_type type)
 #define grub_swap_bytes_XwordXX grub_swap_bytes64
 #define grub_elfXX_check_endianess_and_bswap_ehdr grub_elf64_check_endianess_and_bswap_ehdr
 #define grub_elfXX_get_shnum grub_elf64_get_shnum
+#define grub_elfXX_get_shstrndx grub_elf64_get_shstrndx
 
 #include "elfXX.c"

diff --git a/grub-core/kern/elfXX.c b/grub-core/kern/elfXX.c
index 48b7745a54fe544cc3e096242474742df492cd4a..4e3254fa595a2e5fc91980ea7328c81fa0b98873 100644 (file)
--- a/grub-core/kern/elfXX.c
+++ b/grub-core/kern/elfXX.c
@@ -239,3 +239,36 @@ grub_elfXX_get_shnum (ElfXX_Ehdr *e, ElfXX_Shnum *shnum)
 
   return GRUB_ERR_NONE;
 }
+
+grub_err_t
+grub_elfXX_get_shstrndx (ElfXX_Ehdr *e, ElfXX_Word *shstrndx)
+{
+  ElfXX_Shdr *s;
+
+  if (shstrndx == NULL)
+    return grub_error (GRUB_ERR_BAD_ARGUMENT, N_("NULL pointer passed for shstrndx"));
+
+  /* Set *shstrndx to 0 so that shstrndx doesn't return junk on error */
+  *shstrndx = 0;
+
+  if (e == NULL)
+    return grub_error (GRUB_ERR_BAD_ARGUMENT, N_("NULL pointer passed for elf header"));
+
+  *shstrndx = e->e_shstrndx;
+  if (*shstrndx == SHN_XINDEX)
+    {
+      if (e->e_shoff == 0)
+       return grub_error (GRUB_ERR_BAD_NUMBER, N_("invalid section header table offset in e_shoff"));
+
+      s = (ElfXX_Shdr *) ((grub_uint8_t *) e + e->e_shoff);
+      *shstrndx = s->sh_link;
+      if (*shstrndx < SHN_LORESERVE)
+       return grub_error (GRUB_ERR_BAD_NUMBER, N_("invalid section header table index in sh_link: %d"), *shstrndx);
+    }
+  else
+    {
+      if (*shstrndx >= SHN_LORESERVE)
+       return grub_error (GRUB_ERR_BAD_NUMBER, N_("invalid section header table index in e_shstrndx: %d"), *shstrndx);
+    }
+  return GRUB_ERR_NONE;
+}

diff --git a/grub-core/loader/multiboot_elfxx.c b/grub-core/loader/multiboot_elfxx.c
index ed5a4ae9d42daf5f7bc41a55a99140b5889017b2..ec4ba3b8d012ff5ce1c1095ac949511035c1c4ec 100644 (file)
--- a/grub-core/loader/multiboot_elfxx.c
+++ b/grub-core/loader/multiboot_elfxx.c
@@ -23,8 +23,10 @@
 # define Elf_Ehdr              Elf32_Ehdr
 # define Elf_Phdr              Elf32_Phdr
 # define Elf_Shdr              Elf32_Shdr
+# define Elf_Word              Elf32_Word
 # define Elf_Shnum             Elf32_Shnum
 # define grub_multiboot_elf_get_shnum          grub_elf32_get_shnum
+# define grub_multiboot_elf_get_shstrndx       grub_elf32_get_shstrndx
 #elif defined(MULTIBOOT_LOAD_ELF64)
 # define XX                    64
 # define E_MACHINE             MULTIBOOT_ELF64_MACHINE
@@ -32,8 +34,10 @@
 # define Elf_Ehdr              Elf64_Ehdr
 # define Elf_Phdr              Elf64_Phdr
 # define Elf_Shdr              Elf64_Shdr
+# define Elf_Word              Elf64_Word
 # define Elf_Shnum             Elf64_Shnum
 # define grub_multiboot_elf_get_shnum          grub_elf64_get_shnum
+# define grub_multiboot_elf_get_shstrndx       grub_elf64_get_shstrndx
 #else
 #error "I'm confused"
 #endif
@@ -63,6 +67,7 @@ CONCAT(grub_multiboot_load_elf, XX) (mbi_load_data_t *mld)
   grub_relocator_chunk_t ch;
   grub_uint32_t load_offset = 0, load_size;
   Elf_Shnum shnum;
+  Elf_Word shstrndx;
   unsigned int i;
   void *source = NULL;
 
@@ -84,6 +89,10 @@ CONCAT(grub_multiboot_load_elf, XX) (mbi_load_data_t *mld)
   if (err != GRUB_ERR_NONE)
     return err;
 
+  err = grub_multiboot_elf_get_shstrndx (ehdr, &shstrndx);
+  if (err != GRUB_ERR_NONE)
+    return err;
+
   /* FIXME: Should we support program headers at strange locations?  */
   if (ehdr->e_phoff + (grub_uint32_t) ehdr->e_phnum * ehdr->e_phentsize > MULTIBOOT_SEARCH)
     return grub_error (GRUB_ERR_BAD_OS, "program header at a too high offset");
@@ -291,7 +300,7 @@ CONCAT(grub_multiboot_load_elf, XX) (mbi_load_data_t *mld)
          sh->sh_addr = target;
        }
       GRUB_MULTIBOOT (add_elfsyms) (shnum, ehdr->e_shentsize,
-                                   ehdr->e_shstrndx, shdr);
+                                   shstrndx, shdr);
     }
 
 #undef phdr
@@ -305,5 +314,7 @@ CONCAT(grub_multiboot_load_elf, XX) (mbi_load_data_t *mld)
 #undef Elf_Ehdr
 #undef Elf_Phdr
 #undef Elf_Shdr
+#undef Elf_Word
 #undef Elf_Shnum
 #undef grub_multiboot_elf_get_shnum
+#undef grub_multiboot_elf_get_shstrndx

diff --git a/include/grub/elf.h b/include/grub/elf.h
index ce2f5609adb160ad1381ffcc2918690534870499..43c4809ed531306223287a419b4f606d26f2c035 100644 (file)
--- a/include/grub/elf.h
+++ b/include/grub/elf.h
@@ -2536,8 +2536,10 @@ typedef Elf32_Addr Elf32_Conflict;
 #define R_RISCV_32_PCREL        57
 
 extern grub_err_t grub_elf32_get_shnum (Elf32_Ehdr *e, Elf32_Shnum *shnum);
+extern grub_err_t grub_elf32_get_shstrndx (Elf32_Ehdr *e, Elf32_Word *shstrndx);
 
 extern grub_err_t grub_elf64_get_shnum (Elf64_Ehdr *e, Elf64_Shnum *shnum);
+extern grub_err_t grub_elf64_get_shstrndx (Elf64_Ehdr *e, Elf64_Word *shstrndx);
 
 #ifdef GRUB_TARGET_WORDSIZE
 #if GRUB_TARGET_WORDSIZE == 32
@@ -2567,6 +2569,7 @@ typedef Elf32_Shnum Elf_Shnum;
 #define ELF_R_INFO(sym, type)  ELF32_R_INFO(sym, type)
 
 #define grub_elf_get_shnum     grub_elf32_get_shnum
+#define grub_elf_get_shstrndx  grub_elf32_get_shstrndx
 
 #elif GRUB_TARGET_WORDSIZE == 64
 
@@ -2594,6 +2597,7 @@ typedef Elf64_Shnum Elf_Shnum;
 #define ELF_R_INFO(sym, type)  ELF64_R_INFO(sym, type)
 
 #define grub_elf_get_shnum     grub_elf64_get_shnum
+#define grub_elf_get_shstrndx  grub_elf64_get_shstrndx
 
 #endif /* GRUB_TARGET_WORDSIZE == 64 */
 #endif