* [PATCH 1/6] grub-core/loader/i386/bsdXX.c: Avoid downcasting (char *) to (Elf_Shdr *)
2022-05-26 19:29 [PATCH 0/6] Fix coverity bugs and add checks for elf values in grub-core Alec Brown
@ 2022-05-26 19:29 ` Alec Brown
2022-05-26 19:29 ` [PATCH 2/6] elf: Validate number of elf section header table entries Alec Brown
` (6 subsequent siblings)
7 siblings, 0 replies; 9+ messages in thread
From: Alec Brown @ 2022-05-26 19:29 UTC (permalink / raw)
To: grub-devel; +Cc: daniel.kiper, darren.kenny, alec.r.brown
In bsdXX.c, a couple of untrusted loop bound and untrusted allocation size bugs
were flagged by Coverity in the functions grub_openbsd_find_ramdisk() and
grub_freebsd_load_elfmodule(). These bugs were flagged by coverity because the
variable shdr was downcasting from a char pointer to an Elf_Shdr pointer
whenever it was used to set the base value in for loops. To avoid this, we need
to set shdr as an Elf_Shdr pointer where it is initialized.
In the function read_headers(), the function is reading elf section header data
from a file and passing it to the variable shdr as data for a char pointer. If
we switch the type of shdr to an Elf_Shdr pointer in read_headers() as well as
other functions, then we won't need to downcast to an Elf_Shdr pointer. By doing
this, the issue becomes masked from Coverity's view. In the following patches,
we check limits to ensure the data isn't tainted.
Also, switched use of (char *) to (grub_uint8_t *) to give a better indication
of pointer arithmetic and not suggest use of a C string.
Fixes: CID 314018
Fixes: CID 314030
Fixes: CID 314031
Fixes: CID 314039
Signed-off-by: Alec Brown <alec.r.brown@oracle.com>
---
grub-core/loader/i386/bsdXX.c | 71 ++++++++++++++---------------------
1 file changed, 28 insertions(+), 43 deletions(-)
diff --git a/grub-core/loader/i386/bsdXX.c b/grub-core/loader/i386/bsdXX.c
index e4f4aa365..e6edc6fb6 100644
--- a/grub-core/loader/i386/bsdXX.c
+++ b/grub-core/loader/i386/bsdXX.c
@@ -24,7 +24,7 @@ load (grub_file_t file, const char *filename, void *where, grub_off_t off, grub_
}
static inline grub_err_t
-read_headers (grub_file_t file, const char *filename, Elf_Ehdr *e, char **shdr)
+read_headers (grub_file_t file, const char *filename, Elf_Ehdr *e, Elf_Shdr **shdr)
{
if (grub_file_seek (file, 0) == (grub_off_t) -1)
return grub_errno;
@@ -77,8 +77,7 @@ SUFFIX (grub_freebsd_load_elfmodule_obj) (struct grub_relocator *relocator,
char *argv[], grub_addr_t *kern_end)
{
Elf_Ehdr e;
- Elf_Shdr *s;
- char *shdr = 0;
+ Elf_Shdr *s, *shdr = NULL;
grub_addr_t curload, module;
grub_err_t err;
grub_size_t chunk_size = 0;
@@ -90,9 +89,8 @@ SUFFIX (grub_freebsd_load_elfmodule_obj) (struct grub_relocator *relocator,
if (err)
goto out;
- for (s = (Elf_Shdr *) shdr; s < (Elf_Shdr *) ((char *) shdr
- + e.e_shnum * e.e_shentsize);
- s = (Elf_Shdr *) ((char *) s + e.e_shentsize))
+ for (s = shdr; s < (Elf_Shdr *) ((grub_uint8_t *) shdr + e.e_shnum * e.e_shentsize);
+ s = (Elf_Shdr *) ((grub_uint8_t *) s + e.e_shentsize))
{
if (s->sh_size == 0)
continue;
@@ -113,9 +111,8 @@ SUFFIX (grub_freebsd_load_elfmodule_obj) (struct grub_relocator *relocator,
chunk_src = get_virtual_current_address (ch);
}
- for (s = (Elf_Shdr *) shdr; s < (Elf_Shdr *) ((char *) shdr
- + e.e_shnum * e.e_shentsize);
- s = (Elf_Shdr *) ((char *) s + e.e_shentsize))
+ for (s = shdr; s < (Elf_Shdr *) ((grub_uint8_t *) shdr + e.e_shnum * e.e_shentsize);
+ s = (Elf_Shdr *) ((grub_uint8_t *) s + e.e_shentsize))
{
if (s->sh_size == 0)
continue;
@@ -172,8 +169,7 @@ SUFFIX (grub_freebsd_load_elfmodule) (struct grub_relocator *relocator,
grub_addr_t *kern_end)
{
Elf_Ehdr e;
- Elf_Shdr *s;
- char *shdr = 0;
+ Elf_Shdr *s, *shdr = NULL;
grub_addr_t curload, module;
grub_err_t err;
grub_size_t chunk_size = 0;
@@ -185,9 +181,8 @@ SUFFIX (grub_freebsd_load_elfmodule) (struct grub_relocator *relocator,
if (err)
goto out;
- for (s = (Elf_Shdr *) shdr; s < (Elf_Shdr *) ((char *) shdr
- + e.e_shnum * e.e_shentsize);
- s = (Elf_Shdr *) ((char *) s + e.e_shentsize))
+ for (s = shdr; s < (Elf_Shdr *) ((grub_uint8_t *) shdr + e.e_shnum * e.e_shentsize);
+ s = (Elf_Shdr *) ((grub_uint8_t *) s + e.e_shentsize))
{
if (s->sh_size == 0)
continue;
@@ -214,9 +209,8 @@ SUFFIX (grub_freebsd_load_elfmodule) (struct grub_relocator *relocator,
chunk_src = get_virtual_current_address (ch);
}
- for (s = (Elf_Shdr *) shdr; s < (Elf_Shdr *) ((char *) shdr
- + e.e_shnum * e.e_shentsize);
- s = (Elf_Shdr *) ((char *) s + e.e_shentsize))
+ for (s = shdr; s < (Elf_Shdr *) ((grub_uint8_t *) shdr + e.e_shnum * e.e_shentsize);
+ s = (Elf_Shdr *) ((grub_uint8_t *) s + e.e_shentsize))
{
if (s->sh_size == 0)
continue;
@@ -284,8 +278,7 @@ SUFFIX (grub_freebsd_load_elf_meta) (struct grub_relocator *relocator,
{
grub_err_t err;
Elf_Ehdr e;
- Elf_Shdr *s;
- char *shdr = 0;
+ Elf_Shdr *s, *shdr = NULL;
unsigned symoff, stroff, symsize, strsize;
grub_freebsd_addr_t symstart, symend, symentsize, dynamic;
Elf_Sym *sym;
@@ -306,13 +299,11 @@ SUFFIX (grub_freebsd_load_elf_meta) (struct grub_relocator *relocator,
if (err)
goto out;
- for (s = (Elf_Shdr *) shdr; s < (Elf_Shdr *) (shdr
- + e.e_shnum * e.e_shentsize);
- s = (Elf_Shdr *) ((char *) s + e.e_shentsize))
+ for (s = shdr; s < (Elf_Shdr *) ((grub_uint8_t *) shdr + e.e_shnum * e.e_shentsize);
+ s = (Elf_Shdr *) ((grub_uint8_t *) s + e.e_shentsize))
if (s->sh_type == SHT_SYMTAB)
break;
- if (s >= (Elf_Shdr *) ((char *) shdr
- + e.e_shnum * e.e_shentsize))
+ if (s >= (Elf_Shdr *) ((grub_uint8_t *) shdr + e.e_shnum * e.e_shentsize))
{
err = grub_error (GRUB_ERR_BAD_OS, N_("no symbol table"));
goto out;
@@ -320,7 +311,7 @@ SUFFIX (grub_freebsd_load_elf_meta) (struct grub_relocator *relocator,
symoff = s->sh_offset;
symsize = s->sh_size;
symentsize = s->sh_entsize;
- s = (Elf_Shdr *) (shdr + e.e_shentsize * s->sh_link);
+ s = (Elf_Shdr *) ((grub_uint8_t *) shdr + e.e_shentsize * s->sh_link);
stroff = s->sh_offset;
strsize = s->sh_size;
@@ -426,8 +417,7 @@ SUFFIX (grub_netbsd_load_elf_meta) (struct grub_relocator *relocator,
{
grub_err_t err;
Elf_Ehdr e;
- Elf_Shdr *s, *symsh, *strsh;
- char *shdr = NULL;
+ Elf_Shdr *s, *symsh, *strsh, *shdr = NULL;
unsigned symsize, strsize;
void *sym_chunk;
grub_uint8_t *curload;
@@ -443,20 +433,18 @@ SUFFIX (grub_netbsd_load_elf_meta) (struct grub_relocator *relocator,
return grub_errno;
}
- for (s = (Elf_Shdr *) shdr; s < (Elf_Shdr *) (shdr
- + e.e_shnum * e.e_shentsize);
- s = (Elf_Shdr *) ((char *) s + e.e_shentsize))
+ for (s = shdr; s < (Elf_Shdr *) ((grub_uint8_t *) shdr + e.e_shnum * e.e_shentsize);
+ s = (Elf_Shdr *) ((grub_uint8_t *) s + e.e_shentsize))
if (s->sh_type == SHT_SYMTAB)
break;
- if (s >= (Elf_Shdr *) ((char *) shdr
- + e.e_shnum * e.e_shentsize))
+ if (s >= (Elf_Shdr *) ((grub_uint8_t *) shdr + e.e_shnum * e.e_shentsize))
{
grub_free (shdr);
return GRUB_ERR_NONE;
}
symsize = s->sh_size;
symsh = s;
- s = (Elf_Shdr *) (shdr + e.e_shentsize * s->sh_link);
+ s = (Elf_Shdr *) ((grub_uint8_t *) shdr + e.e_shentsize * s->sh_link);
strsize = s->sh_size;
strsh = s;
@@ -490,9 +478,8 @@ SUFFIX (grub_netbsd_load_elf_meta) (struct grub_relocator *relocator,
curload += sizeof (e);
- for (s = (Elf_Shdr *) shdr; s < (Elf_Shdr *) (shdr
- + e.e_shnum * e.e_shentsize);
- s = (Elf_Shdr *) ((char *) s + e.e_shentsize))
+ for (s = shdr; s < (Elf_Shdr *) ((grub_uint8_t *) shdr + e.e_shnum * e.e_shentsize);
+ s = (Elf_Shdr *) ((grub_uint8_t *) s + e.e_shentsize))
{
Elf_Shdr *s2;
s2 = (Elf_Shdr *) curload;
@@ -564,8 +551,7 @@ SUFFIX(grub_openbsd_find_ramdisk) (grub_file_t file,
{
grub_err_t err;
Elf_Ehdr e;
- Elf_Shdr *s;
- char *shdr = NULL;
+ Elf_Shdr *s, *shdr = NULL;
err = read_headers (file, filename, &e, &shdr);
if (err)
@@ -574,12 +560,11 @@ SUFFIX(grub_openbsd_find_ramdisk) (grub_file_t file,
return err;
}
- for (s = (Elf_Shdr *) shdr; s < (Elf_Shdr *) (shdr
- + e.e_shnum * e.e_shentsize);
- s = (Elf_Shdr *) ((char *) s + e.e_shentsize))
+ for (s = shdr; s < (Elf_Shdr *) ((grub_uint8_t *) shdr + e.e_shnum * e.e_shentsize);
+ s = (Elf_Shdr *) ((grub_uint8_t *) s + e.e_shentsize))
if (s->sh_type == SHT_SYMTAB)
break;
- if (s >= (Elf_Shdr *) ((char *) shdr + e.e_shnum * e.e_shentsize))
+ if (s >= (Elf_Shdr *) ((grub_uint8_t *) shdr + e.e_shnum * e.e_shentsize))
{
grub_free (shdr);
return GRUB_ERR_NONE;
@@ -589,7 +574,7 @@ SUFFIX(grub_openbsd_find_ramdisk) (grub_file_t file,
symentsize = s->sh_entsize;
symoff = s->sh_offset;
- s = (Elf_Shdr *) (shdr + e.e_shentsize * s->sh_link);
+ s = (Elf_Shdr *) ((grub_uint8_t *) shdr + e.e_shentsize * s->sh_link);
stroff = s->sh_offset;
strsize = s->sh_size;
grub_free (shdr);
--
2.27.0
^ permalink raw reply related [flat|nested] 9+ messages in thread* [PATCH 2/6] elf: Validate number of elf section header table entries
2022-05-26 19:29 [PATCH 0/6] Fix coverity bugs and add checks for elf values in grub-core Alec Brown
2022-05-26 19:29 ` [PATCH 1/6] grub-core/loader/i386/bsdXX.c: Avoid downcasting (char *) to (Elf_Shdr *) Alec Brown
@ 2022-05-26 19:29 ` Alec Brown
2022-05-26 19:29 ` [PATCH 3/6] elf: Validate elf section header table index for section name string table Alec Brown
` (5 subsequent siblings)
7 siblings, 0 replies; 9+ messages in thread
From: Alec Brown @ 2022-05-26 19:29 UTC (permalink / raw)
To: grub-devel; +Cc: daniel.kiper, darren.kenny, alec.r.brown
In bsdXX.c and multiboot_elfXX.c, 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, grub_elfXX_get_shnum() is being added to
elfXX.c 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.
In order to get this function to work, the type ElfXX_Shnum is being added where
Elf32_Shnum defines Elf32_Word and Elf64_Shnum defines Elf64_Xword. This new
type is needed because if shnum obtains a value from sh_size, sh_size could be
of type El32_Word for Elf32_Shdr structures or Elf64_Xword for Elf64_Shdr
structures.
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 shnum to match variables being set as well as dropped
casts when unnecessary and fixed spacing errors in bsdXX.c.
Signed-off-by: Alec Brown <alec.r.brown@oracle.com>
---
grub-core/kern/elf.c | 12 +++++
grub-core/kern/elfXX.c | 34 +++++++++++++
grub-core/loader/i386/bsdXX.c | 82 ++++++++++++++++++++++--------
grub-core/loader/multiboot_elfxx.c | 49 +++++++++++-------
include/grub/elf.h | 14 +++++
5 files changed, 150 insertions(+), 41 deletions(-)
diff --git a/grub-core/kern/elf.c b/grub-core/kern/elf.c
index 9d7149b38..66b69293e 100644
--- a/grub-core/kern/elf.c
+++ b/grub-core/kern/elf.c
@@ -167,11 +167,15 @@ grub_elf_open (const char *name, enum grub_file_type type)
#define grub_elfXX_load_phdrs grub_elf32_load_phdrs
#define ElfXX_Phdr Elf32_Phdr
#define ElfXX_Ehdr Elf32_Ehdr
+#define ElfXX_Shdr Elf32_Shdr
+#define ElfXX_Word Elf32_Word
+#define ElfXX_Shnum Elf32_Shnum
#define grub_uintXX_t grub_uint32_t
#define grub_swap_bytes_addrXX grub_swap_bytes32
#define grub_swap_bytes_offXX grub_swap_bytes32
#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
#include "elfXX.c"
@@ -185,11 +189,15 @@ grub_elf_open (const char *name, enum grub_file_type type)
#undef grub_elfXX_load_phdrs
#undef ElfXX_Phdr
#undef ElfXX_Ehdr
+#undef ElfXX_Shdr
+#undef ElfXX_Word
+#undef ElfXX_Shnum
#undef grub_uintXX_t
#undef grub_swap_bytes_addrXX
#undef grub_swap_bytes_offXX
#undef grub_swap_bytes_XwordXX
#undef grub_elfXX_check_endianess_and_bswap_ehdr
+#undef grub_elfXX_get_shnum
\f
/* 64-bit */
@@ -203,10 +211,14 @@ grub_elf_open (const char *name, enum grub_file_type type)
#define grub_elfXX_load_phdrs grub_elf64_load_phdrs
#define ElfXX_Phdr Elf64_Phdr
#define ElfXX_Ehdr Elf64_Ehdr
+#define ElfXX_Shdr Elf64_Shdr
+#define ElfXX_Word Elf64_Word
+#define ElfXX_Shnum Elf64_Shnum
#define grub_uintXX_t grub_uint64_t
#define grub_swap_bytes_addrXX grub_swap_bytes64
#define grub_swap_bytes_offXX grub_swap_bytes64
#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
#include "elfXX.c"
diff --git a/grub-core/kern/elfXX.c b/grub-core/kern/elfXX.c
index 1859d1880..48b7745a5 100644
--- a/grub-core/kern/elfXX.c
+++ b/grub-core/kern/elfXX.c
@@ -205,3 +205,37 @@ grub_elfXX_check_endianess_and_bswap_ehdr (grub_elf_t elf)
return 0;
}
+
+grub_err_t
+grub_elfXX_get_shnum (ElfXX_Ehdr *e, ElfXX_Shnum *shnum)
+{
+ ElfXX_Shdr *s;
+
+ if (shnum == NULL)
+ return grub_error (GRUB_ERR_BAD_ARGUMENT, N_("NULL pointer passed for shnum"));
+
+ /* Set *shnum to 0 so that shnum doesn't return junk on error */
+ *shnum = 0;
+
+ if (e == NULL)
+ return grub_error (GRUB_ERR_BAD_ARGUMENT, N_("NULL pointer passed for elf header"));
+
+ *shnum = e->e_shnum;
+ if (*shnum == SHN_UNDEF)
+ {
+ 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);
+ *shnum = s->sh_size;
+ if (*shnum < SHN_LORESERVE)
+ return grub_error (GRUB_ERR_BAD_NUMBER, N_("invalid number of section header table entries in sh_size: %" PRIuGRUB_UINT64_T), (grub_uint64_t) *shnum);
+ }
+ else
+ {
+ if (*shnum >= SHN_LORESERVE)
+ return grub_error (GRUB_ERR_BAD_NUMBER, N_("invalid number of section header table entries in e_shnum: %" PRIuGRUB_UINT64_T), (grub_uint64_t) *shnum);
+ }
+
+ return GRUB_ERR_NONE;
+}
diff --git a/grub-core/loader/i386/bsdXX.c b/grub-core/loader/i386/bsdXX.c
index e6edc6fb6..6e5eb3643 100644
--- a/grub-core/loader/i386/bsdXX.c
+++ b/grub-core/loader/i386/bsdXX.c
@@ -26,7 +26,10 @@ load (grub_file_t file, const char *filename, void *where, grub_off_t off, grub_
static inline grub_err_t
read_headers (grub_file_t file, const char *filename, Elf_Ehdr *e, Elf_Shdr **shdr)
{
- if (grub_file_seek (file, 0) == (grub_off_t) -1)
+ Elf_Shnum shnum;
+ grub_err_t err;
+
+ if (grub_file_seek (file, 0) == (grub_off_t) -1)
return grub_errno;
if (grub_file_read (file, (char *) e, sizeof (*e)) != sizeof (*e))
@@ -48,15 +51,19 @@ read_headers (grub_file_t file, const char *filename, Elf_Ehdr *e, Elf_Shdr **sh
if (e->e_ident[EI_CLASS] != SUFFIX (ELFCLASS))
return grub_error (GRUB_ERR_BAD_OS, N_("invalid arch-dependent ELF magic"));
- *shdr = grub_calloc (e->e_shnum, e->e_shentsize);
+ err = grub_elf_get_shnum (e, &shnum);
+ if (err)
+ return err;
+
+ *shdr = grub_calloc (shnum, e->e_shentsize);
if (! *shdr)
return grub_errno;
if (grub_file_seek (file, e->e_shoff) == (grub_off_t) -1)
return grub_errno;
- if (grub_file_read (file, *shdr, (grub_uint32_t) e->e_shnum * e->e_shentsize)
- != (grub_ssize_t) ((grub_uint32_t) e->e_shnum * e->e_shentsize))
+ if (grub_file_read (file, *shdr, shnum * e->e_shentsize)
+ != ((grub_ssize_t) shnum * e->e_shentsize))
{
if (grub_errno)
return grub_errno;
@@ -78,6 +85,7 @@ SUFFIX (grub_freebsd_load_elfmodule_obj) (struct grub_relocator *relocator,
{
Elf_Ehdr e;
Elf_Shdr *s, *shdr = NULL;
+ Elf_Shnum shnum;
grub_addr_t curload, module;
grub_err_t err;
grub_size_t chunk_size = 0;
@@ -89,7 +97,11 @@ SUFFIX (grub_freebsd_load_elfmodule_obj) (struct grub_relocator *relocator,
if (err)
goto out;
- for (s = shdr; s < (Elf_Shdr *) ((grub_uint8_t *) shdr + e.e_shnum * e.e_shentsize);
+ err = grub_elf_get_shnum (&e, &shnum);
+ 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))
{
if (s->sh_size == 0)
@@ -111,7 +123,7 @@ SUFFIX (grub_freebsd_load_elfmodule_obj) (struct grub_relocator *relocator,
chunk_src = get_virtual_current_address (ch);
}
- for (s = shdr; s < (Elf_Shdr *) ((grub_uint8_t *) shdr + e.e_shnum * e.e_shentsize);
+ for (s = shdr; s < (Elf_Shdr *) ((grub_uint8_t *) shdr + shnum * e.e_shentsize);
s = (Elf_Shdr *) ((grub_uint8_t *) s + e.e_shentsize))
{
if (s->sh_size == 0)
@@ -154,7 +166,7 @@ SUFFIX (grub_freebsd_load_elfmodule_obj) (struct grub_relocator *relocator,
if (! err)
err = grub_bsd_add_meta (FREEBSD_MODINFO_METADATA
| FREEBSD_MODINFOMD_SHDR,
- shdr, e.e_shnum * e.e_shentsize);
+ shdr, shnum * e.e_shentsize);
out:
grub_free (shdr);
@@ -170,6 +182,7 @@ SUFFIX (grub_freebsd_load_elfmodule) (struct grub_relocator *relocator,
{
Elf_Ehdr e;
Elf_Shdr *s, *shdr = NULL;
+ Elf_Shnum shnum;
grub_addr_t curload, module;
grub_err_t err;
grub_size_t chunk_size = 0;
@@ -181,7 +194,11 @@ SUFFIX (grub_freebsd_load_elfmodule) (struct grub_relocator *relocator,
if (err)
goto out;
- for (s = shdr; s < (Elf_Shdr *) ((grub_uint8_t *) shdr + e.e_shnum * e.e_shentsize);
+ err = grub_elf_get_shnum (&e, &shnum);
+ 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))
{
if (s->sh_size == 0)
@@ -196,7 +213,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_uint32_t) e.e_shnum * e.e_shentsize;
+ chunk_size += (grub_size_t) shnum * e.e_shentsize;
{
grub_relocator_chunk_t ch;
@@ -209,7 +226,7 @@ SUFFIX (grub_freebsd_load_elfmodule) (struct grub_relocator *relocator,
chunk_src = get_virtual_current_address (ch);
}
- for (s = shdr; s < (Elf_Shdr *) ((grub_uint8_t *) shdr + e.e_shnum * e.e_shentsize);
+ for (s = shdr; s < (Elf_Shdr *) ((grub_uint8_t *) shdr + shnum * e.e_shentsize);
s = (Elf_Shdr *) ((grub_uint8_t *) s + e.e_shentsize))
{
if (s->sh_size == 0)
@@ -247,9 +264,9 @@ SUFFIX (grub_freebsd_load_elfmodule) (struct grub_relocator *relocator,
curload = module + sizeof (e);
load (file, argv[0], (grub_uint8_t *) chunk_src + curload - *kern_end, e.e_shoff,
- (grub_uint32_t) e.e_shnum * e.e_shentsize);
+ (grub_size_t) shnum * e.e_shentsize);
e.e_shoff = curload - module;
- curload += (grub_uint32_t) e.e_shnum * e.e_shentsize;
+ 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);
@@ -279,6 +296,7 @@ SUFFIX (grub_freebsd_load_elf_meta) (struct grub_relocator *relocator,
grub_err_t err;
Elf_Ehdr e;
Elf_Shdr *s, *shdr = NULL;
+ Elf_Shnum shnum;
unsigned symoff, stroff, symsize, strsize;
grub_freebsd_addr_t symstart, symend, symentsize, dynamic;
Elf_Sym *sym;
@@ -293,17 +311,21 @@ SUFFIX (grub_freebsd_load_elf_meta) (struct grub_relocator *relocator,
if (err)
goto out;
+ err = grub_elf_get_shnum (&e, &shnum);
+ if (err != GRUB_ERR_NONE)
+ goto out;
+
err = grub_bsd_add_meta (FREEBSD_MODINFO_METADATA |
FREEBSD_MODINFOMD_ELFHDR, &e,
sizeof (e));
if (err)
goto out;
- for (s = shdr; s < (Elf_Shdr *) ((grub_uint8_t *) shdr + e.e_shnum * e.e_shentsize);
+ for (s = shdr; s < (Elf_Shdr *) ((grub_uint8_t *) shdr + shnum * e.e_shentsize);
s = (Elf_Shdr *) ((grub_uint8_t *) s + e.e_shentsize))
if (s->sh_type == SHT_SYMTAB)
break;
- if (s >= (Elf_Shdr *) ((grub_uint8_t *) shdr + e.e_shnum * e.e_shentsize))
+ if (s >= (Elf_Shdr *) ((grub_uint8_t *) shdr + shnum * e.e_shentsize))
{
err = grub_error (GRUB_ERR_BAD_OS, N_("no symbol table"));
goto out;
@@ -418,6 +440,7 @@ SUFFIX (grub_netbsd_load_elf_meta) (struct grub_relocator *relocator,
grub_err_t err;
Elf_Ehdr e;
Elf_Shdr *s, *symsh, *strsh, *shdr = NULL;
+ Elf_Shnum shnum;
unsigned symsize, strsize;
void *sym_chunk;
grub_uint8_t *curload;
@@ -433,11 +456,18 @@ SUFFIX (grub_netbsd_load_elf_meta) (struct grub_relocator *relocator,
return grub_errno;
}
- for (s = shdr; s < (Elf_Shdr *) ((grub_uint8_t *) shdr + e.e_shnum * e.e_shentsize);
+ err = grub_elf_get_shnum (&e, &shnum);
+ if (err != GRUB_ERR_NONE)
+ {
+ grub_free (shdr);
+ return err;
+ }
+
+ for (s = shdr; s < (Elf_Shdr *) ((grub_uint8_t *) shdr + shnum * e.e_shentsize);
s = (Elf_Shdr *) ((grub_uint8_t *) s + e.e_shentsize))
if (s->sh_type == SHT_SYMTAB)
break;
- if (s >= (Elf_Shdr *) ((grub_uint8_t *) shdr + e.e_shnum * e.e_shentsize))
+ if (s >= (Elf_Shdr *) ((grub_uint8_t *) shdr + shnum * e.e_shentsize))
{
grub_free (shdr);
return GRUB_ERR_NONE;
@@ -450,7 +480,7 @@ SUFFIX (grub_netbsd_load_elf_meta) (struct grub_relocator *relocator,
chunk_size = ALIGN_UP (symsize, sizeof (grub_freebsd_addr_t))
+ ALIGN_UP (strsize, sizeof (grub_freebsd_addr_t))
- + sizeof (e) + (grub_uint32_t) e.e_shnum * e.e_shentsize;
+ + sizeof (e) + shnum * e.e_shentsize;
symtarget = ALIGN_UP (*kern_end, sizeof (grub_freebsd_addr_t));
{
@@ -478,17 +508,17 @@ SUFFIX (grub_netbsd_load_elf_meta) (struct grub_relocator *relocator,
curload += sizeof (e);
- for (s = shdr; s < (Elf_Shdr *) ((grub_uint8_t *) shdr + e.e_shnum * e.e_shentsize);
+ for (s = shdr; s < (Elf_Shdr *) ((grub_uint8_t *) shdr + shnum * e.e_shentsize);
s = (Elf_Shdr *) ((grub_uint8_t *) s + e.e_shentsize))
{
Elf_Shdr *s2;
s2 = (Elf_Shdr *) curload;
grub_memcpy (curload, s, e.e_shentsize);
if (s == symsh)
- s2->sh_offset = sizeof (e) + (grub_uint32_t) e.e_shnum * e.e_shentsize;
+ s2->sh_offset = sizeof (e) + shnum * e.e_shentsize;
else if (s == strsh)
s2->sh_offset = ALIGN_UP (symsize, sizeof (grub_freebsd_addr_t))
- + sizeof (e) + (grub_uint32_t) e.e_shnum * e.e_shentsize;
+ + sizeof (e) + shnum * e.e_shentsize;
else
s2->sh_offset = 0;
s2->sh_addr = s2->sh_offset;
@@ -552,6 +582,7 @@ SUFFIX(grub_openbsd_find_ramdisk) (grub_file_t file,
grub_err_t err;
Elf_Ehdr e;
Elf_Shdr *s, *shdr = NULL;
+ Elf_Shnum shnum;
err = read_headers (file, filename, &e, &shdr);
if (err)
@@ -560,11 +591,18 @@ SUFFIX(grub_openbsd_find_ramdisk) (grub_file_t file,
return err;
}
- for (s = shdr; s < (Elf_Shdr *) ((grub_uint8_t *) shdr + e.e_shnum * e.e_shentsize);
+ err = grub_elf_get_shnum (&e, &shnum);
+ if (err != GRUB_ERR_NONE)
+ {
+ grub_free (shdr);
+ return err;
+ }
+
+ for (s = shdr; s < (Elf_Shdr *) ((grub_uint8_t *) shdr + shnum * e.e_shentsize);
s = (Elf_Shdr *) ((grub_uint8_t *) s + e.e_shentsize))
if (s->sh_type == SHT_SYMTAB)
break;
- if (s >= (Elf_Shdr *) ((grub_uint8_t *) shdr + e.e_shnum * e.e_shentsize))
+ if (s >= (Elf_Shdr *) ((grub_uint8_t *) shdr + shnum * e.e_shentsize))
{
grub_free (shdr);
return GRUB_ERR_NONE;
diff --git a/grub-core/loader/multiboot_elfxx.c b/grub-core/loader/multiboot_elfxx.c
index 6801f0ddf..0d6b6612f 100644
--- a/grub-core/loader/multiboot_elfxx.c
+++ b/grub-core/loader/multiboot_elfxx.c
@@ -17,19 +17,23 @@
*/
#if defined(MULTIBOOT_LOAD_ELF32)
-# define XX 32
-# define E_MACHINE MULTIBOOT_ELF32_MACHINE
-# define ELFCLASSXX ELFCLASS32
-# define Elf_Ehdr Elf32_Ehdr
-# define Elf_Phdr Elf32_Phdr
-# define Elf_Shdr Elf32_Shdr
+# define XX 32
+# define E_MACHINE MULTIBOOT_ELF32_MACHINE
+# define ELFCLASSXX ELFCLASS32
+# define Elf_Ehdr Elf32_Ehdr
+# define Elf_Phdr Elf32_Phdr
+# define Elf_Shdr Elf32_Shdr
+# define Elf_Shnum Elf32_Shnum
+# define grub_elf_get_shnum grub_elf32_get_shnum
#elif defined(MULTIBOOT_LOAD_ELF64)
-# define XX 64
-# define E_MACHINE MULTIBOOT_ELF64_MACHINE
-# define ELFCLASSXX ELFCLASS64
-# define Elf_Ehdr Elf64_Ehdr
-# define Elf_Phdr Elf64_Phdr
-# define Elf_Shdr Elf64_Shdr
+# define XX 64
+# define E_MACHINE MULTIBOOT_ELF64_MACHINE
+# define ELFCLASSXX ELFCLASS64
+# define Elf_Ehdr Elf64_Ehdr
+# define Elf_Phdr Elf64_Phdr
+# define Elf_Shdr Elf64_Shdr
+# define Elf_Shnum Elf64_Shnum
+# define grub_elf_get_shnum grub_elf64_get_shnum
#else
#error "I'm confused"
#endif
@@ -58,7 +62,8 @@ 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;
- int i;
+ Elf_Shnum shnum;
+ unsigned int i;
void *source = NULL;
if (ehdr->e_ident[EI_MAG0] != ELFMAG0
@@ -75,6 +80,10 @@ CONCAT(grub_multiboot_load_elf, XX) (mbi_load_data_t *mld)
if (ehdr->e_type != ET_EXEC && ehdr->e_type != ET_DYN)
return grub_error (GRUB_ERR_UNKNOWN_OS, N_("this ELF file is not of the right type"));
+ err = grub_elf_get_shnum (ehdr, &shnum);
+ 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");
@@ -213,11 +222,11 @@ CONCAT(grub_multiboot_load_elf, XX) (mbi_load_data_t *mld)
#error Please complete this
#endif
- if (ehdr->e_shnum)
+ if (shnum)
{
grub_uint8_t *shdr, *shdrptr;
- shdr = grub_calloc (ehdr->e_shnum, ehdr->e_shentsize);
+ shdr = grub_calloc (shnum, ehdr->e_shentsize);
if (!shdr)
return grub_errno;
@@ -227,8 +236,8 @@ CONCAT(grub_multiboot_load_elf, XX) (mbi_load_data_t *mld)
return grub_errno;
}
- if (grub_file_read (mld->file, shdr, (grub_uint32_t) ehdr->e_shnum * ehdr->e_shentsize)
- != (grub_ssize_t) ehdr->e_shnum * ehdr->e_shentsize)
+ if (grub_file_read (mld->file, shdr, shnum * ehdr->e_shentsize)
+ != (grub_ssize_t) shnum * ehdr->e_shentsize)
{
if (!grub_errno)
grub_error (GRUB_ERR_FILE_READ_ERROR, N_("premature end of file %s"),
@@ -236,7 +245,7 @@ CONCAT(grub_multiboot_load_elf, XX) (mbi_load_data_t *mld)
return grub_errno;
}
- for (shdrptr = shdr, i = 0; i < ehdr->e_shnum;
+ for (shdrptr = shdr, i = 0; i < shnum;
shdrptr += ehdr->e_shentsize, i++)
{
Elf_Shdr *sh = (Elf_Shdr *) shdrptr;
@@ -281,7 +290,7 @@ CONCAT(grub_multiboot_load_elf, XX) (mbi_load_data_t *mld)
}
sh->sh_addr = target;
}
- GRUB_MULTIBOOT (add_elfsyms) (ehdr->e_shnum, ehdr->e_shentsize,
+ GRUB_MULTIBOOT (add_elfsyms) (shnum, ehdr->e_shentsize,
ehdr->e_shstrndx, shdr);
}
@@ -296,3 +305,5 @@ CONCAT(grub_multiboot_load_elf, XX) (mbi_load_data_t *mld)
#undef Elf_Ehdr
#undef Elf_Phdr
#undef Elf_Shdr
+#undef Elf_Shnum
+#undef grub_elf_get_shnum
diff --git a/include/grub/elf.h b/include/grub/elf.h
index c478933ee..ce2f5609a 100644
--- a/include/grub/elf.h
+++ b/include/grub/elf.h
@@ -23,6 +23,7 @@
/* Standard ELF types. */
#include <grub/types.h>
+#include <grub/err.h>
/* Type for a 16-bit quantity. */
typedef grub_uint16_t Elf32_Half;
@@ -56,6 +57,9 @@ typedef grub_uint16_t Elf64_Section;
typedef Elf32_Half Elf32_Versym;
typedef Elf64_Half Elf64_Versym;
+/* Type for number of section header table entries */
+typedef Elf32_Word Elf32_Shnum;
+typedef Elf64_Xword Elf64_Shnum;
/* The ELF file header. This appears at the start of every ELF file. */
@@ -2531,6 +2535,10 @@ typedef Elf32_Addr Elf32_Conflict;
#define R_RISCV_SET32 56
#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_elf64_get_shnum (Elf64_Ehdr *e, Elf64_Shnum *shnum);
+
#ifdef GRUB_TARGET_WORDSIZE
#if GRUB_TARGET_WORDSIZE == 32
@@ -2548,6 +2556,7 @@ typedef Elf32_Sword Elf_Sword;
typedef Elf32_Sym Elf_Sym;
typedef Elf32_Word Elf_Word;
typedef Elf32_Xword Elf_Xword;
+typedef Elf32_Shnum Elf_Shnum;
#define ELF_ST_BIND(val) ELF32_ST_BIND(val)
#define ELF_ST_TYPE(val) ELF32_ST_TYPE(val)
@@ -2557,6 +2566,8 @@ typedef Elf32_Xword Elf_Xword;
#define ELF_R_TYPE(val) ELF32_R_TYPE(val)
#define ELF_R_INFO(sym, type) ELF32_R_INFO(sym, type)
+#define grub_elf_get_shnum grub_elf32_get_shnum
+
#elif GRUB_TARGET_WORDSIZE == 64
typedef Elf64_Addr Elf_Addr;
@@ -2573,6 +2584,7 @@ typedef Elf64_Sword Elf_Sword;
typedef Elf64_Sym Elf_Sym;
typedef Elf64_Word Elf_Word;
typedef Elf64_Xword Elf_Xword;
+typedef Elf64_Shnum Elf_Shnum;
#define ELF_ST_BIND(val) ELF64_ST_BIND (val)
#define ELF_ST_TYPE(val) ELF64_ST_TYPE (val)
@@ -2581,6 +2593,8 @@ typedef Elf64_Xword Elf_Xword;
#define ELF_R_TYPE(val) ELF64_R_TYPE(val)
#define ELF_R_INFO(sym, type) ELF64_R_INFO(sym, type)
+#define grub_elf_get_shnum grub_elf64_get_shnum
+
#endif /* GRUB_TARGET_WORDSIZE == 64 */
#endif
--
2.27.0
^ permalink raw reply related [flat|nested] 9+ messages in thread* [PATCH 3/6] elf: Validate elf section header table index for section name string table
2022-05-26 19:29 [PATCH 0/6] Fix coverity bugs and add checks for elf values in grub-core Alec Brown
2022-05-26 19:29 ` [PATCH 1/6] grub-core/loader/i386/bsdXX.c: Avoid downcasting (char *) to (Elf_Shdr *) Alec Brown
2022-05-26 19:29 ` [PATCH 2/6] elf: Validate number of elf section header table entries Alec Brown
@ 2022-05-26 19:29 ` Alec Brown
2022-05-26 19:29 ` [PATCH 4/6] elf: Validate number of elf program header table entries Alec Brown
` (4 subsequent siblings)
7 siblings, 0 replies; 9+ messages in thread
From: Alec Brown @ 2022-05-26 19:29 UTC (permalink / raw)
To: grub-devel; +Cc: daniel.kiper, darren.kenny, alec.r.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
\f
/* 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
^ permalink raw reply related [flat|nested] 9+ messages in thread* [PATCH 4/6] elf: Validate number of elf program header table entries
2022-05-26 19:29 [PATCH 0/6] Fix coverity bugs and add checks for elf values in grub-core Alec Brown
` (2 preceding siblings ...)
2022-05-26 19:29 ` [PATCH 3/6] elf: Validate elf section header table index for section name string table Alec Brown
@ 2022-05-26 19:29 ` Alec Brown
2022-05-26 19:29 ` [PATCH 5/6] util/grub-module-verifierXX.c: Add e_shoff check in get_shdr() Alec Brown
` (3 subsequent siblings)
7 siblings, 0 replies; 9+ messages in thread
From: Alec Brown @ 2022-05-26 19:29 UTC (permalink / raw)
To: grub-devel; +Cc: daniel.kiper, darren.kenny, alec.r.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.
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>
---
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 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
\f
/* 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 6e5eb3643..2cc1daa49 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 8e0384505..3e72c6caa 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_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
@@ -38,6 +39,7 @@
# define Elf_Shnum Elf64_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
#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_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_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 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
--
2.27.0
^ permalink raw reply related [flat|nested] 9+ messages in thread* [PATCH 5/6] util/grub-module-verifierXX.c: Add e_shoff check in get_shdr()
2022-05-26 19:29 [PATCH 0/6] Fix coverity bugs and add checks for elf values in grub-core Alec Brown
` (3 preceding siblings ...)
2022-05-26 19:29 ` [PATCH 4/6] elf: Validate number of elf program header table entries Alec Brown
@ 2022-05-26 19:29 ` Alec Brown
2022-05-26 19:29 ` [PATCH 6/6] util/grub-module-verifierXX.c: Changed get_shnum() return type Alec Brown
` (2 subsequent siblings)
7 siblings, 0 replies; 9+ messages in thread
From: Alec Brown @ 2022-05-26 19:29 UTC (permalink / raw)
To: grub-devel; +Cc: daniel.kiper, darren.kenny, alec.r.brown
In util/grub-module-verifierXX.c, the function get_shdr() is used to obtain the
section header at a given index but isn't checking that there is an offset for
the section header table. To validate that there is, we can check that e_shoff
isn't 0.
Signed-off-by: Alec Brown <alec.r.brown@oracle.com>
---
util/grub-module-verifierXX.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/util/grub-module-verifierXX.c b/util/grub-module-verifierXX.c
index 4e6cf133f..cf3ff0dfa 100644
--- a/util/grub-module-verifierXX.c
+++ b/util/grub-module-verifierXX.c
@@ -134,6 +134,9 @@ grub_target_to_host_real (const struct grub_module_verifier_arch *arch, grub_uin
static Elf_Shdr *
get_shdr (const struct grub_module_verifier_arch *arch, Elf_Ehdr *e, Elf_Word index)
{
+ if (grub_target_to_host (e->e_shoff) == 0)
+ grub_util_error ("Invalid section header offset");
+
return (Elf_Shdr *) ((char *) e + grub_target_to_host (e->e_shoff) +
index * grub_target_to_host16 (e->e_shentsize));
}
--
2.27.0
^ permalink raw reply related [flat|nested] 9+ messages in thread* [PATCH 6/6] util/grub-module-verifierXX.c: Changed get_shnum() return type
2022-05-26 19:29 [PATCH 0/6] Fix coverity bugs and add checks for elf values in grub-core Alec Brown
` (4 preceding siblings ...)
2022-05-26 19:29 ` [PATCH 5/6] util/grub-module-verifierXX.c: Add e_shoff check in get_shdr() Alec Brown
@ 2022-05-26 19:29 ` Alec Brown
2022-05-27 14:00 ` [PATCH 0/6] Fix coverity bugs and add checks for elf values in grub-core Daniel Kiper
2022-05-30 9:02 ` Darren Kenny
7 siblings, 0 replies; 9+ messages in thread
From: Alec Brown @ 2022-05-26 19:29 UTC (permalink / raw)
To: grub-devel; +Cc: daniel.kiper, darren.kenny, alec.r.brown
In util/grub-module-verifierXX.c, the function get_shnum() returns the variable
shnum, which is of the type Elf_Word. In the function, shnum can be obtained by
the e_shnum member of an Elf_Ehdr or the sh_size member of an Elf_Shdr. The
sh_size member can either be grub_uint32_t or grub_uint64_t, depending on the
architecture, but Elf_Word is only grub_uint32_t. To account for when sh_size is
grub_uint64_t, we can set shnum to have type Elf_Shnum and have get_shnum()
return an Elf_Shnum.
Signed-off-by: Alec Brown <alec.r.brown@oracle.com>
---
util/grub-module-verifierXX.c | 10 ++++++----
1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/util/grub-module-verifierXX.c b/util/grub-module-verifierXX.c
index cf3ff0dfa..8e0cd91d9 100644
--- a/util/grub-module-verifierXX.c
+++ b/util/grub-module-verifierXX.c
@@ -18,6 +18,7 @@
# define Elf_Rel Elf32_Rel
# define Elf_Word Elf32_Word
# define Elf_Half Elf32_Half
+# define Elf_Shnum Elf32_Shnum
# define Elf_Section Elf32_Section
# define ELF_R_SYM(val) ELF32_R_SYM(val)
# define ELF_R_TYPE(val) ELF32_R_TYPE(val)
@@ -36,6 +37,7 @@
# define Elf_Rel Elf64_Rel
# define Elf_Word Elf64_Word
# define Elf_Half Elf64_Half
+# define Elf_Shnum Elf64_Shnum
# define Elf_Section Elf64_Section
# define ELF_R_SYM(val) ELF64_R_SYM(val)
# define ELF_R_TYPE(val) ELF64_R_TYPE(val)
@@ -141,11 +143,11 @@ 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
+static Elf_Shnum
get_shnum (const struct grub_module_verifier_arch *arch, Elf_Ehdr *e)
{
Elf_Shdr *s;
- Elf_Word shnum;
+ Elf_Shnum shnum;
shnum = grub_target_to_host16 (e->e_shnum);
if (shnum == SHN_UNDEF)
@@ -153,12 +155,12 @@ get_shnum (const struct grub_module_verifier_arch *arch, Elf_Ehdr *e)
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);
+ grub_util_error ("Invalid number of section header table entries in sh_size: %" PRIuGRUB_UINT64_T, (grub_uint64_t) shnum);
}
else
{
if (shnum >= SHN_LORESERVE)
- grub_util_error ("Invalid number of section header table entries in e_shnum: %d", shnum);
+ grub_util_error ("Invalid number of section header table entries in e_shnum: %" PRIuGRUB_UINT64_T, (grub_uint64_t) shnum);
}
return shnum;
--
2.27.0
^ permalink raw reply related [flat|nested] 9+ messages in thread* Re: [PATCH 0/6] Fix coverity bugs and add checks for elf values in grub-core
2022-05-26 19:29 [PATCH 0/6] Fix coverity bugs and add checks for elf values in grub-core Alec Brown
` (5 preceding siblings ...)
2022-05-26 19:29 ` [PATCH 6/6] util/grub-module-verifierXX.c: Changed get_shnum() return type Alec Brown
@ 2022-05-27 14:00 ` Daniel Kiper
2022-05-30 9:02 ` Darren Kenny
7 siblings, 0 replies; 9+ messages in thread
From: Daniel Kiper @ 2022-05-27 14:00 UTC (permalink / raw)
To: Alec Brown; +Cc: grub-devel, darren.kenny
On Thu, May 26, 2022 at 03:29:46PM -0400, Alec Brown wrote:
> Coverity identified several untrusted loop bounds and untrusted allocation size
> bugs in grub-core/loader/i386/bsdXX.c and grub-core/loader/multiboot_elfXX.c.
> Upon review of these bugs, I found that specific checks weren't being made to
> various elf header values based on the elf manual page. The first four patches
> in this patch series address the coverity bugs, as well as adds functions to
> check for the correct elf header values. The last two patches adds fixes to
> previous work done in util/grub-module-verifierXX.c that also relates to making
> checks of elf header values.
>
> The Coverity bugs being addressed are:
> CID 314018
> CID 314030
> CID 314031
> CID 314039
>
> Alec Brown (6):
> grub-core/loader/i386/bsdXX.c: Avoid downcasting (char *) to (Elf_Shdr *)
> elf: Validate number of elf section header table entries
> elf: Validate elf section header table index for section name string table
> elf: Validate number of elf program header table entries
> util/grub-module-verifierXX.c: Add e_shoff check in get_shdr()
> util/grub-module-verifierXX.c: Changed get_shnum() return type
For all patches Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>.
Thank you for fixing these issues!
Daniel
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH 0/6] Fix coverity bugs and add checks for elf values in grub-core
2022-05-26 19:29 [PATCH 0/6] Fix coverity bugs and add checks for elf values in grub-core Alec Brown
` (6 preceding siblings ...)
2022-05-27 14:00 ` [PATCH 0/6] Fix coverity bugs and add checks for elf values in grub-core Daniel Kiper
@ 2022-05-30 9:02 ` Darren Kenny
7 siblings, 0 replies; 9+ messages in thread
From: Darren Kenny @ 2022-05-30 9:02 UTC (permalink / raw)
To: Alec Brown, grub-devel; +Cc: daniel.kiper, alec.r.brown
Hi Alec,
All of these look great, so:
Reviewed-by: Darren Kenny <darren.kenny@oracle.com>
Thanks for looking at the Coverity issues,
Darren.
On Thursday, 2022-05-26 at 15:29:46 -04, Alec Brown wrote:
> Coverity identified several untrusted loop bounds and untrusted allocation size
> bugs in grub-core/loader/i386/bsdXX.c and grub-core/loader/multiboot_elfXX.c.
> Upon review of these bugs, I found that specific checks weren't being made to
> various elf header values based on the elf manual page. The first four patches
> in this patch series address the coverity bugs, as well as adds functions to
> check for the correct elf header values. The last two patches adds fixes to
> previous work done in util/grub-module-verifierXX.c that also relates to making
> checks of elf header values.
>
> The Coverity bugs being addressed are:
> CID 314018
> CID 314030
> CID 314031
> CID 314039
>
> Alec Brown (6):
> grub-core/loader/i386/bsdXX.c: Avoid downcasting (char *) to (Elf_Shdr *)
> elf: Validate number of elf section header table entries
> elf: Validate elf section header table index for section name string table
> elf: Validate number of elf program header table entries
> util/grub-module-verifierXX.c: Add e_shoff check in get_shdr()
> util/grub-module-verifierXX.c: Changed get_shnum() return type
>
> grub-core/kern/elf.c | 18 ++++++++++++++++++
> grub-core/kern/elfXX.c | 101 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> grub-core/loader/i386/bsdXX.c | 142 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++---------------------------------------------------------
> grub-core/loader/multiboot_elfxx.c | 79 ++++++++++++++++++++++++++++++++++++++++++++++++++++++-------------------------
> include/grub/elf.h | 23 +++++++++++++++++++++++
> util/grub-module-verifierXX.c | 13 +++++++++----
> 6 files changed, 290 insertions(+), 86 deletions(-)
^ permalink raw reply [flat|nested] 9+ messages in thread