From: Yaseen <[email protected]>
resolve_symbol() in relocate.c validates the symbol name offset with
st_name >= d_size, then forms a pointer into the string table and
passes it to strcmp(). Nothing checks that a NUL byte actually exists
between st_name and the end of the string table. A crafted ET_REL
object whose .strtab has the referenced name at the final bytes with
no terminating NUL makes strcmp() read past the string table -- an
out-of-bounds read.
dwfl_module_getsym() has the same pattern: it checks the offset and
returns the pointer to the caller without verifying termination. Any
caller that passes the result to strlen, strcmp, printf %s, etc. will
read out of bounds on a malformed file.
eu-unstrip already does this correctly at unstrip.c:888 using memrchr
to verify the name is terminated. The libdwfl paths were missed.
The path is reachable from dwfl_module_getdwarf() on an ET_REL module:
dwfl_module_getdwarf
-> load_dw
-> __libdwfl_relocate
-> relocate_section
-> resolve_symbol
-> strcmp(name, n) <-- unterminated name, reads past .strtab
Reproduced under AddressSanitizer with a crafted ET_REL whose .strtab
contains a symbol name with no NUL within the declared section size.
The referrer and a provider module are loaded through dwfl_report_elf
and dwfl_module_getdwarf. The elf_memory path (which gives ASan a
heap redzone at the exact section boundary) shows:
==ERROR: AddressSanitizer: heap-buffer-overflow
READ of size 11 at 0x... thread T0
#0 ... in strcmp
#1 ... in relocate
#2 ... in relocate_section
#3 ... in __libdwfl_relocate_section
0x... is located 0 bytes after 1166-byte region
A valid control object with the terminating NUL added exits cleanly.
Add memchr checks after the offset validation. If no NUL is found
within the remaining string table, return DWFL_E_BADSTROFF.
* libdwfl/relocate.c (resolve_symbol): Check that the referrer
symbol name is NUL terminated within the string table before
using it in strcmp.
(resolve_symbol): Check the same for the provider symbol name.
* libdwfl/dwfl_module_getsym.c (__libdwfl_getsym): Check that the
symbol name is NUL terminated before returning it to the caller.
Signed-off-by: Yaseen <[email protected]>
---
libdwfl/dwfl_module_getsym.c | 4 +++-
libdwfl/relocate.c | 6 ++++++
2 files changed, 9 insertions(+), 1 deletion(-)
diff --git a/libdwfl/dwfl_module_getsym.c b/libdwfl/dwfl_module_getsym.c
index 3a364a7..1631ae1 100644
--- a/libdwfl/dwfl_module_getsym.c
+++ b/libdwfl/dwfl_module_getsym.c
@@ -190,7 +190,9 @@ __libdwfl_getsym (Dwfl_Module *mod, int ndx, GElf_Sym *sym,
GElf_Addr *addr,
if (addr != NULL)
*addr = st_value;
- if (unlikely (sym->st_name >= symstrdata->d_size))
+ if (unlikely (sym->st_name >= symstrdata->d_size)
+ || unlikely (memchr (symstrdata->d_buf + sym->st_name, '\0',
+ symstrdata->d_size - sym->st_name) == NULL))
{
__libdwfl_seterrno (DWFL_E_BADSTROFF);
return NULL;
diff --git a/libdwfl/relocate.c b/libdwfl/relocate.c
index dc9f45e..b8df09d 100644
--- a/libdwfl/relocate.c
+++ b/libdwfl/relocate.c
@@ -285,6 +285,9 @@ resolve_symbol (Dwfl_Module *referer, struct
reloc_symtab_cache *symtab,
}
if (unlikely (sym->st_name >= symtab->symstrdata->d_size))
return DWFL_E_BADSTROFF;
+ if (unlikely (memchr (symtab->symstrdata->d_buf + sym->st_name, '\0',
+ symtab->symstrdata->d_size - sym->st_name) == NULL))
+ return DWFL_E_BADSTROFF;
const char *name = symtab->symstrdata->d_buf;
name += sym->st_name;
@@ -319,6 +322,9 @@ resolve_symbol (Dwfl_Module *referer, struct
reloc_symtab_cache *symtab,
/* Get this candidate symbol's name. */
if (unlikely (sym->st_name >= m->symstrdata->d_size))
return DWFL_E_BADSTROFF;
+ if (unlikely (memchr (m->symstrdata->d_buf + sym->st_name, '\0',
+ m->symstrdata->d_size - sym->st_name) ==
NULL))
+ return DWFL_E_BADSTROFF;
const char *n = m->symstrdata->d_buf;
n += sym->st_name;
--
2.43.0
