[PATCH v3 4/5] elf: Validate number of elf program header table entries

[Alec Brown]

In bsdXX.c and multiboot_elfXX.c, e_phnum is used to obtain the number of
program header table entries, but it wasn't being checked if the value was
there.

According to the elf(5) manual page,
"If the number of entries in the program header table is larger than or equal to
PN_XNUM (0xffff), this member holds PN_XNUM (0xffff) and the real number of
entries in the program header table is held in the sh_info member of the
initial entry in section header table.  Otherwise, the sh_info member of the
initial entry contains the value zero."

Since this check wasn't being made, grub_elfXX_get_phnum() is being added to
elfXX.c to make this check and use e_phnum if it doesn't have PN_XNUM as a
value, else use sh_info. We also need to make sure e_phnum isn't greater than
PN_XNUM and sh_info isn't less than PN_XNUM.

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             | 34 ++++++++++++++++++++++++++++++
 grub-core/loader/i386/bsdXX.c      | 11 +++++++---
 grub-core/loader/multiboot_elfxx.c | 19 +++++++++++------
 include/grub/elf.h                 |  5 +++++
 5 files changed, 63 insertions(+), 9 deletions(-)

diff --git a/grub-core/kern/elf.c b/grub-core/kern/elf.c
index 19440a318..ff5655206 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_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
+#define grub_elfXX_get_phnum grub_elf32_get_phnum
 
 #include "elfXX.c"
 
@@ -198,6 +199,7 @@ grub_elf_open (const char *name, enum grub_file_type type)
 #undef grub_elfXX_check_endianess_and_bswap_ehdr
 #undef grub_elfXX_get_shnum
 #undef grub_elfXX_get_shstrndx
+#undef grub_elfXX_get_phnum
 
 
 /* 64-bit */
@@ -220,5 +222,6 @@ grub_elf_open (const char *name, enum grub_file_type type)
 #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
+#define grub_elfXX_get_phnum grub_elf64_get_phnum
 
 #include "elfXX.c"
diff --git a/grub-core/kern/elfXX.c b/grub-core/kern/elfXX.c
index d09b37213..a917e7cce 100644
--- a/grub-core/kern/elfXX.c
+++ b/grub-core/kern/elfXX.c
@@ -272,3 +272,37 @@ grub_elfXX_get_shstrndx (ElfXX_Ehdr *e, grub_uint32_t 
*shstrndx)
     }
   return GRUB_ERR_NONE;
 }
+
+grub_err_t
+grub_elfXX_get_phnum (ElfXX_Ehdr *e, grub_uint32_t *phnum)
+{
+  ElfXX_Shdr *s;
+
+  if (phnum == NULL)
+    return grub_error (GRUB_ERR_BAD_ARGUMENT, N_("NULL pointer passed for 
phnum"));
+
+  /* Set *phnum to 0 so that phnum doesn't return junk on error */
+  *phnum = 0;
+
+  if (e == NULL)
+    return grub_error (GRUB_ERR_BAD_ARGUMENT, N_("NULL pointer passed for elf 
header"));
+
+  *phnum = e->e_phnum;
+  if (*phnum == PN_XNUM)
+    {
+      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);
+      *phnum = s->sh_info;
+      if (*phnum < PN_XNUM)
+       return grub_error (GRUB_ERR_BAD_NUMBER, N_("invalid number of program 
header table entries in sh_info: %d"), *phnum);
+    }
+  else
+    {
+      if (*phnum >= PN_XNUM)
+       return grub_error (GRUB_ERR_BAD_NUMBER, N_("invalid number of program 
header table entries in e_phnum: %d"), *phnum);
+    }
+
+  return GRUB_ERR_NONE;
+}
diff --git a/grub-core/loader/i386/bsdXX.c b/grub-core/loader/i386/bsdXX.c
index 7d9b65d1a..eb1164dd7 100644
--- a/grub-core/loader/i386/bsdXX.c
+++ b/grub-core/loader/i386/bsdXX.c
@@ -185,6 +185,7 @@ SUFFIX (grub_freebsd_load_elfmodule) (struct grub_relocator 
*relocator,
   Elf_Shdr *s;
   Elf_Shdr *shdr = NULL;
   Elf_Word shnum;
+  grub_uint32_t phnum;
   grub_addr_t curload, module;
   grub_err_t err;
   grub_size_t chunk_size = 0;
@@ -200,6 +201,10 @@ SUFFIX (grub_freebsd_load_elfmodule) (struct 
grub_relocator *relocator,
   if (err != GRUB_ERR_NONE)
     goto out;
 
+  err = grub_elf_get_phnum (&e, &phnum);
+  if (err != GRUB_ERR_NONE)
+    goto out;
+
   for (s = shdr; s < (Elf_Shdr *) ((grub_uint8_t *) shdr + shnum * 
e.e_shentsize);
        s = (Elf_Shdr *) ((grub_uint8_t *) s + e.e_shentsize))
     {
@@ -214,7 +219,7 @@ SUFFIX (grub_freebsd_load_elfmodule) (struct grub_relocator 
*relocator,
 
   if (chunk_size < sizeof (e))
     chunk_size = sizeof (e);
-  chunk_size += (grub_uint32_t) e.e_phnum * e.e_phentsize;
+  chunk_size += (grub_size_t) phnum * e.e_phentsize;
   chunk_size += (grub_size_t) shnum * e.e_shentsize;
 
   {
@@ -271,9 +276,9 @@ SUFFIX (grub_freebsd_load_elfmodule) (struct grub_relocator 
*relocator,
   curload +=  (grub_addr_t) shnum * e.e_shentsize;
 
   load (file, argv[0], (grub_uint8_t *) chunk_src + curload - *kern_end, 
e.e_phoff,
-       (grub_uint32_t) e.e_phnum * e.e_phentsize);
+       (grub_size_t) phnum * e.e_phentsize);
   e.e_phoff = curload - module;
-  curload +=  (grub_uint32_t) e.e_phnum * e.e_phentsize;
+  curload +=  (grub_addr_t) phnum * e.e_phentsize;
 
   *kern_end = curload;
 
diff --git a/grub-core/loader/multiboot_elfxx.c 
b/grub-core/loader/multiboot_elfxx.c
index 54aaa24aa..87eb9639a 100644
--- a/grub-core/loader/multiboot_elfxx.c
+++ b/grub-core/loader/multiboot_elfxx.c
@@ -26,6 +26,7 @@
 # define Elf_Word              Elf32_Word
 # define grub_elf_get_shnum    grub_elf32_get_shnum
 # define grub_elf_get_shstrndx grub_elf32_get_shstrndx
+# define grub_elf_get_phnum    grub_elf32_get_phnum
 #elif defined(MULTIBOOT_LOAD_ELF64)
 # define XX                    64
 # define E_MACHINE             MULTIBOOT_ELF64_MACHINE
@@ -36,6 +37,7 @@
 # define Elf_Word              Elf64_Word
 # define grub_elf_get_shnum    grub_elf64_get_shnum
 # define grub_elf_get_shstrndx grub_elf64_get_shstrndx
+# define grub_elf_get_phnum    grub_elf64_get_phnum
 #else
 #error "I'm confused"
 #endif
@@ -64,7 +66,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;
+  grub_uint32_t shstrndx, phnum;
   Elf_Word shnum;
   unsigned int i;
   void *source = NULL;
@@ -91,8 +93,12 @@ CONCAT(grub_multiboot_load_elf, XX) (mbi_load_data_t *mld)
   if (err != GRUB_ERR_NONE)
     return err;
 
+  err = grub_elf_get_phnum (ehdr, &phnum);
+  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)
+  if (ehdr->e_phoff + phnum * ehdr->e_phentsize > MULTIBOOT_SEARCH)
     return grub_error (GRUB_ERR_BAD_OS, "program header at a too high offset");
 
   phdr_base = (char *) mld->buffer + ehdr->e_phoff;
@@ -101,7 +107,7 @@ CONCAT(grub_multiboot_load_elf, XX) (mbi_load_data_t *mld)
   mld->link_base_addr = ~0;
 
   /* Calculate lowest and highest load address.  */
-  for (i = 0; i < ehdr->e_phnum; i++)
+  for (i = 0; i < phnum; i++)
     if (phdr(i)->p_type == PT_LOAD)
       {
        mld->link_base_addr = grub_min (mld->link_base_addr, phdr(i)->p_paddr);
@@ -147,7 +153,7 @@ CONCAT(grub_multiboot_load_elf, XX) (mbi_load_data_t *mld)
                mld->link_base_addr, mld->load_base_addr);
 
   /* Load every loadable segment in memory.  */
-  for (i = 0; i < ehdr->e_phnum; i++)
+  for (i = 0; i < phnum; i++)
     {
       if (phdr(i)->p_type == PT_LOAD)
         {
@@ -196,7 +202,7 @@ CONCAT(grub_multiboot_load_elf, XX) (mbi_load_data_t *mld)
         }
     }
 
-  for (i = 0; i < ehdr->e_phnum; i++)
+  for (i = 0; i < phnum; i++)
     if (phdr(i)->p_vaddr <= ehdr->e_entry
        && phdr(i)->p_vaddr + phdr(i)->p_memsz > ehdr->e_entry)
       {
@@ -218,7 +224,7 @@ CONCAT(grub_multiboot_load_elf, XX) (mbi_load_data_t *mld)
        break;
       }
 
-  if (i == ehdr->e_phnum)
+  if (i == phnum)
     return grub_error (GRUB_ERR_BAD_OS, "entry point isn't in a segment");
 
 #if defined (__i386__) || defined (__x86_64__)
@@ -315,3 +321,4 @@ CONCAT(grub_multiboot_load_elf, XX) (mbi_load_data_t *mld)
 #undef Elf_Word
 #undef grub_elf_get_shnum
 #undef grub_elf_get_shstrndx
+#undef grub_elf_get_phnum
diff --git a/include/grub/elf.h b/include/grub/elf.h
index 9c94fd6d3..80dbd577b 100644
--- a/include/grub/elf.h
+++ b/include/grub/elf.h
@@ -567,6 +567,7 @@ typedef struct
 #define PT_HIOS                0x6fffffff      /* End of OS-specific */
 #define PT_LOPROC      0x70000000      /* Start of processor-specific */
 #define PT_HIPROC      0x7fffffff      /* End of processor-specific */
+#define PN_XNUM                0xffff
 
 /* Legal values for p_flags (segment flags).  */
 
@@ -2534,9 +2535,11 @@ typedef Elf32_Addr Elf32_Conflict;
 
 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_elf32_get_phnum (Elf32_Ehdr *e, grub_uint32_t *phnum);
 
 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);
+extern grub_err_t grub_elf64_get_phnum (Elf64_Ehdr *e, Elf64_Word *phnum);
 
 #ifdef GRUB_TARGET_WORDSIZE
 #if GRUB_TARGET_WORDSIZE == 32
@@ -2566,6 +2569,7 @@ typedef Elf32_Xword Elf_Xword;
 
 #define grub_elf_get_shnum     grub_elf32_get_shnum
 #define grub_elf_get_shstrndx  grub_elf32_get_shstrndx
+#define grub_elf_get_phnum     grub_elf32_get_phnum
 
 #elif GRUB_TARGET_WORDSIZE == 64
 
@@ -2593,6 +2597,7 @@ typedef Elf64_Xword Elf_Xword;
 
 #define grub_elf_get_shnum     grub_elf64_get_shnum
 #define grub_elf_get_shstrndx  grub_elf64_get_shstrndx
+#define grub_elf_get_phnum     grub_elf64_get_phnum
 
 #endif /* GRUB_TARGET_WORDSIZE == 64 */
 #endif
-- 
2.27.0