All of lore.kernel.org
 help / color / mirror / Atom feed
From: Alec Brown <alec.r.brown@oracle.com>
To: grub-devel@gnu.org
Cc: daniel.kiper@oracle.com, darren.kenny@oracle.com
Subject: [PATCH 2/4] util/grub-module-verifierXX.c: Validate number of elf section header table entries
Date: Wed,  2 Feb 2022 19:26:58 -0500	[thread overview]
Message-ID: <1643848020-8197-3-git-send-email-alec.r.brown@oracle.com> (raw)
In-Reply-To: <1643848020-8197-1-git-send-email-alec.r.brown@oracle.com>

In grub-module-verifierXX.c, grub_target_to_host16 (e->e_shnum) is used to
obtain the number of section 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 section header table is larger than or equal to
SHN_LORESERVE (0xff00), e_shnum holds the value zero and the real number of
entries in the section header table is held in the sh_size member of the intial
entry in section header table. Otherwise, the sh_size member of the initial
entry in the section header table holds the value zero."

Since this check wasn't being made, the function get_shnum() is being added to
make this check and use whichever member doesn't have a value of zero. If both
are zero, then we must return an error. We also need to make sure that e_shnum
doesn't have a value greater than or equal to SHN_LORESERVE and sh_size isn't
less than SHN_LORESERVE.

Note that it may look as though the argument *arch isn't being used, it's
actually required in order to use the macros grub_target_to_host*(x) which are
unwinded to grub_target_to_host*_real(arch, (x)) based on defines earlier in the
file.

Fixes: CID 314021
Fixes: CID 314027
Fixes: CID 314033

Signed-off-by: Alec Brown <alec.r.brown@oracle.com>
---
 util/grub-module-verifierXX.c | 35 +++++++++++++++++++++++++++++------
 1 file changed, 29 insertions(+), 6 deletions(-)

diff --git a/util/grub-module-verifierXX.c b/util/grub-module-verifierXX.c
index 92f9ae2a4..61b82141f 100644
--- a/util/grub-module-verifierXX.c
+++ b/util/grub-module-verifierXX.c
@@ -138,6 +138,29 @@ get_shdr (const struct grub_module_verifier_arch *arch, Elf_Ehdr *e, Elf_Word in
 	 	       index * grub_target_to_host16 (e->e_shentsize));
 }
 
+static Elf_Word
+get_shnum (const struct grub_module_verifier_arch *arch, Elf_Ehdr *e)
+{
+  Elf_Shdr *s;
+  Elf_Word shnum;
+
+  shnum = grub_target_to_host16 (e->e_shnum);
+  if (shnum == SHN_UNDEF)
+    {
+      s = get_shdr (arch, e, 0);
+      shnum = grub_target_to_host (s->sh_size);
+      if (shnum < SHN_LORESERVE)
+	grub_util_error ("Invalid number of section header table entries in sh_size: %d", shnum);
+    }
+  else
+    {
+      if (shnum >= SHN_LORESERVE)
+	grub_util_error ("Invalid number of section header table entries in e_shnum: %d", shnum);
+    }
+
+  return shnum;
+}
+
 static Elf_Shdr *
 find_section (const struct grub_module_verifier_arch *arch, Elf_Ehdr *e, const char *name)
 {
@@ -149,7 +172,7 @@ find_section (const struct grub_module_verifier_arch *arch, Elf_Ehdr *e, const c
   str = (char *) e + grub_target_to_host (s->sh_offset);
 
   for (i = 0, s = get_shdr (arch, e, 0);
-       i < grub_target_to_host16 (e->e_shnum);
+       i < get_shnum (arch, e);
        i++, s = get_shdr (arch, e, i))
     if (strcmp (str + grub_target_to_host32 (s->sh_name), name) == 0)
       return s;
@@ -176,12 +199,12 @@ get_symtab (const struct grub_module_verifier_arch *arch, Elf_Ehdr *e, Elf_Word
   Elf_Sym *sym;
 
   for (i = 0, s = get_shdr (arch, e, 0);
-       i < grub_target_to_host16 (e->e_shnum);
+       i < get_shnum (arch, e);
        i++, s = get_shdr (arch, e, i))
     if (grub_target_to_host32 (s->sh_type) == SHT_SYMTAB)
       break;
 
-  if (i == grub_target_to_host16 (e->e_shnum))
+  if (i == get_shnum (arch, e))
     return NULL;
 
   sym = (Elf_Sym *) ((char *) e + grub_target_to_host (s->sh_offset));
@@ -370,7 +393,7 @@ check_relocations (const char * const modname,
   unsigned i;
 
   for (i = 0, s = get_shdr (arch, e, 0);
-       i < grub_target_to_host16 (e->e_shnum);
+       i < get_shnum (arch, e);
        i++, s = get_shdr (arch, e, i))
     if (grub_target_to_host32 (s->sh_type) == SHT_REL || grub_target_to_host32 (s->sh_type) == SHT_RELA)
       {
@@ -382,7 +405,7 @@ check_relocations (const char * const modname,
 	  grub_util_error ("%s: unsupported SHT_RELA", modname);
 
 	/* Find the target segment.  */
-	if (grub_target_to_host32 (s->sh_info) >= grub_target_to_host16 (e->e_shnum))
+	if (grub_target_to_host32 (s->sh_info) >= get_shnum (arch, e))
 	  grub_util_error ("%s: orphaned reloc section", modname);
 	ts = get_shdr (arch, e, grub_target_to_host32 (s->sh_info));
 
@@ -423,7 +446,7 @@ SUFFIX(grub_module_verify) (const char * const filename,
 
   /* Make sure that every section is within the core.  */
   if (size < grub_target_to_host (e->e_shoff)
-      + (grub_uint32_t) grub_target_to_host16 (e->e_shentsize) * grub_target_to_host16(e->e_shnum))
+      + (grub_uint32_t) grub_target_to_host16 (e->e_shentsize) * get_shnum (arch, e))
     {
       grub_util_error ("%s: ELF sections outside core", filename);
     }
-- 
2.27.0



  parent reply	other threads:[~2022-02-03  0:27 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-02-03  0:26 [PATCH 0/4] Clean up code and fix coverity bugs in util/grub-module-verifierXX.c Alec Brown
2022-02-03  0:26 ` [PATCH 1/4] util/grub-module-verifierXX.c: Add function to calculate section headers Alec Brown
2022-02-03  0:26 ` Alec Brown [this message]
2022-02-03  0:26 ` [PATCH 3/4] util/grub-module-verifierXX.c: Validate elf section header table index for section name string table Alec Brown
2022-02-03  0:27 ` [PATCH 4/4] util/grub-module-verifierXX.c: Add module_size parameter to functions for sanity checking Alec Brown
2022-02-08 16:28   ` Daniel Kiper
2022-02-03 11:56 ` [PATCH 0/4] Clean up code and fix coverity bugs in util/grub-module-verifierXX.c Darren Kenny
2022-02-04 14:36   ` Daniel Kiper

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1643848020-8197-3-git-send-email-alec.r.brown@oracle.com \
    --to=alec.r.brown@oracle.com \
    --cc=daniel.kiper@oracle.com \
    --cc=darren.kenny@oracle.com \
    --cc=grub-devel@gnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.