[PATCH 3/6] 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 | 13 +++++++++++-
 include/grub/elf.h                 |  4 ++++
 4 files changed, 52 insertions(+), 1 deletion(-)

diff --git a/grub-core/kern/elf.c b/grub-core/kern/elf.c
index 66b69293e..c676db694 100644
--- 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
 
 
 /* 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 48b7745a5..4e3254fa5 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_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 0d6b6612f..8e0384505 100644
--- 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_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
@@ -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_elf_get_shnum    grub_elf64_get_shnum
+# define grub_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_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_elf_get_shnum
+#undef grub_elf_get_shstrndx
diff --git a/include/grub/elf.h b/include/grub/elf.h
index ce2f5609a..43c4809ed 100644
--- 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
-- 
2.27.0