elf: Validate number of elf program header table entries

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.

Also, changed casts of phnum to match variables being set as well as dropped
casts when unnecessary.

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>

5 files changed, 63 insertions, 9 deletions

diff --git a/grub-core/kern/elf.c b/grub-core/kern/elf.c
index c676db694..077a8500c 100644
--- a/grub-core/kern/elf.c
+++ b/grub-core/kern/elf.c
@@ -177,6 +177,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"
 
@@ -200,6 +201,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 */
@@ -223,5 +225,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 4e3254fa5..aabf4b9d7 100644
--- a/grub-core/kern/elfXX.c
+++ b/grub-core/kern/elfXX.c
@@ -272,3 +272,37 @@ grub_elfXX_get_shstrndx (ElfXX_Ehdr *e, ElfXX_Word *shstrndx)
     }
   return GRUB_ERR_NONE;
 }
+
+grub_err_t
+grub_elfXX_get_phnum (ElfXX_Ehdr *e, ElfXX_Word *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 2d91fd44d..dffa79d56 100644
--- a/grub-core/loader/i386/bsdXX.c
+++ b/grub-core/loader/i386/bsdXX.c
@@ -183,6 +183,7 @@ SUFFIX (grub_freebsd_load_elfmodule) (struct grub_relocator *relocator,
   Elf_Ehdr e;
   Elf_Shdr *s, *shdr = NULL;
   Elf_Shnum shnum;
+  Elf_Word phnum;
   grub_addr_t curload, module;
   grub_err_t err;
   grub_size_t chunk_size = 0;
@@ -198,6 +199,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))
     {
@@ -212,7 +217,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;
 
   {
@@ -269,9 +274,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 ec4ba3b8d..b6c0e23ca 100644
--- a/grub-core/loader/multiboot_elfxx.c
+++ b/grub-core/loader/multiboot_elfxx.c
@@ -27,6 +27,7 @@
 # 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
+# define grub_multiboot_elf_get_phnum		grub_elf32_get_phnum
 #elif defined(MULTIBOOT_LOAD_ELF64)
 # define XX			64
 # define E_MACHINE		MULTIBOOT_ELF64_MACHINE
@@ -38,6 +39,7 @@
 # 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
+# define grub_multiboot_elf_get_phnum		grub_elf64_get_phnum
 #else
 #error "I'm confused"
 #endif
@@ -67,7 +69,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;
+  Elf_Word shstrndx, phnum;
   unsigned int i;
   void *source = NULL;
 
@@ -93,8 +95,12 @@ CONCAT(grub_multiboot_load_elf, XX) (mbi_load_data_t *mld)
   if (err != GRUB_ERR_NONE)
     return err;
 
+  err = grub_multiboot_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;
@@ -103,7 +109,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);
@@ -149,7 +155,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)
         {
@@ -198,7 +204,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)
       {
@@ -220,7 +226,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__)
@@ -318,3 +324,4 @@ CONCAT(grub_multiboot_load_elf, XX) (mbi_load_data_t *mld)
 #undef Elf_Shnum
 #undef grub_multiboot_elf_get_shnum
 #undef grub_multiboot_elf_get_shstrndx
+#undef grub_multiboot_elf_get_phnum
diff --git a/include/grub/elf.h b/include/grub/elf.h
index 43c4809ed..0e70651d4 100644
--- a/include/grub/elf.h
+++ b/include/grub/elf.h
@@ -570,6 +570,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).  */
 
@@ -2537,9 +2538,11 @@ typedef Elf32_Addr Elf32_Conflict;
 
 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_elf32_get_phnum (Elf32_Ehdr *e, Elf32_Word *phnum);
 
 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);
+extern grub_err_t grub_elf64_get_phnum (Elf64_Ehdr *e, Elf64_Word *phnum);
 
 #ifdef GRUB_TARGET_WORDSIZE
 #if GRUB_TARGET_WORDSIZE == 32
@@ -2570,6 +2573,7 @@ typedef Elf32_Shnum Elf_Shnum;
 
 #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
 
@@ -2598,6 +2602,7 @@ typedef Elf64_Shnum Elf_Shnum;
 
 #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