[PATCH v3 3/5] elf: Validate elf section header table index for section name string table

[Alec Brown]

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>
---
 grub-core/kern/elf.c               |  3 +++
 grub-core/kern/elfXX.c             | 33 ++++++++++++++++++++++++++++++
 grub-core/loader/multiboot_elfxx.c | 10 ++++++++-
 include/grub/elf.h                 |  4 ++++
 4 files changed, 49 insertions(+), 1 deletion(-)

diff --git a/grub-core/kern/elf.c b/grub-core/kern/elf.c
index 5be770583..19440a318 100644
--- a/grub-core/kern/elf.c
+++ b/grub-core/kern/elf.c
@@ -175,6 +175,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"
 
@@ -196,6 +197,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
 
 
 /* 64-bit */
@@ -217,5 +219,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 ed2dc7075..d09b37213 100644
--- a/grub-core/kern/elfXX.c
+++ b/grub-core/kern/elfXX.c
@@ -239,3 +239,36 @@ grub_elfXX_get_shnum (ElfXX_Ehdr *e, ElfXX_Word *shnum)
 
   return GRUB_ERR_NONE;
 }
+
+grub_err_t
+grub_elfXX_get_shstrndx (ElfXX_Ehdr *e, grub_uint32_t *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 1fe9be91b..54aaa24aa 100644
--- a/grub-core/loader/multiboot_elfxx.c
+++ b/grub-core/loader/multiboot_elfxx.c
@@ -25,6 +25,7 @@
 # define Elf_Shdr              Elf32_Shdr
 # define Elf_Word              Elf32_Word
 # define grub_elf_get_shnum    grub_elf32_get_shnum
+# define grub_elf_get_shstrndx grub_elf32_get_shstrndx
 #elif defined(MULTIBOOT_LOAD_ELF64)
 # define XX                    64
 # define E_MACHINE             MULTIBOOT_ELF64_MACHINE
@@ -34,6 +35,7 @@
 # define Elf_Shdr              Elf64_Shdr
 # define Elf_Word              Elf64_Word
 # define grub_elf_get_shnum    grub_elf64_get_shnum
+# define grub_elf_get_shstrndx grub_elf64_get_shstrndx
 #else
 #error "I'm confused"
 #endif
@@ -62,6 +64,7 @@ CONCAT(grub_multiboot_load_elf, XX) (mbi_load_data_t *mld)
   grub_err_t err;
   grub_relocator_chunk_t ch;
   grub_uint32_t load_offset = 0, load_size;
+  grub_uint32_t shstrndx;
   Elf_Word shnum;
   unsigned int i;
   void *source = NULL;
@@ -84,6 +87,10 @@ CONCAT(grub_multiboot_load_elf, XX) (mbi_load_data_t *mld)
   if (err != GRUB_ERR_NONE)
     return err;
 
+  err = grub_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 +298,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
@@ -307,3 +314,4 @@ CONCAT(grub_multiboot_load_elf, XX) (mbi_load_data_t *mld)
 #undef Elf_Shdr
 #undef Elf_Word
 #undef grub_elf_get_shnum
+#undef grub_elf_get_shstrndx
diff --git a/include/grub/elf.h b/include/grub/elf.h
index 229efd349..9c94fd6d3 100644
--- a/include/grub/elf.h
+++ b/include/grub/elf.h
@@ -2533,8 +2533,10 @@ typedef Elf32_Addr Elf32_Conflict;
 #define R_RISCV_32_PCREL        57
 
 extern grub_err_t grub_elf32_get_shnum (Elf32_Ehdr *e, Elf32_Word *shnum);
+extern grub_err_t grub_elf32_get_shstrndx (Elf32_Ehdr *e, grub_uint32_t 
*shstrndx);
 
 extern grub_err_t grub_elf64_get_shnum (Elf64_Ehdr *e, Elf64_Word *shnum);
+extern grub_err_t grub_elf64_get_shstrndx (Elf64_Ehdr *e, grub_uint32_t 
*shstrndx);
 
 #ifdef GRUB_TARGET_WORDSIZE
 #if GRUB_TARGET_WORDSIZE == 32
@@ -2563,6 +2565,7 @@ typedef Elf32_Xword Elf_Xword;
 #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
 
@@ -2589,6 +2592,7 @@ typedef Elf64_Xword Elf_Xword;
 #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
-- 
2.27.0