https://sourceware.org/bugzilla/show_bug.cgi?id=33099

Mark Wielaard <mark at klomp dot org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |mark at klomp dot org
             Status|UNCONFIRMED                 |NEW
     Ever confirmed|0                           |1
   Last reconfirmed|                            |2025-06-20

--- Comment #3 from Mark Wielaard <mark at klomp dot org> ---
Interesting find. I can replicate with valgrind eu-readelf -r

The input file is clearly bogus, because it references a segment with not zero
terminated strings. So technically it isn't a vulnerability:
https://sourceware.org/cgit/elfutils/tree/SECURITY
But even so it would be nice to be able to handle it.

I appreciate your suggested fix since it tries to handle the generic case where
a chunk of data isn't zero terminated and then used for string handling. But
there are cases where we don't malloc, but just use a mmaped region that we
cannot extend to put an extra zero at the end. Also it adds this overhead to
any rawchunk whether or not it is used to store C strings.

So I think we should add a check where the string is fetched from the chunk,
which is in libdwfl/dwfl_module_getsym.c (getting the name for a symbol for a
particular address):

diff --git a/libdwfl/dwfl_module_getsym.c b/libdwfl/dwfl_module_getsym.c
index 8de9a3eb8092..332b0008ba24 100644
--- a/libdwfl/dwfl_module_getsym.c
+++ b/libdwfl/dwfl_module_getsym.c
@@ -185,7 +185,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
+                || memrchr (symstrdata->d_buf + sym->st_name, '\0',
+                           symstrdata->d_size - sym->st_name - 1) == NULL))
     {
       __libdwfl_seterrno (DWFL_E_BADSTROFF);
       return NULL;

Or maybe we need to add the check where the symstrdata is allocated?

-- 
You are receiving this mail because:
You are on the CC list for the bug.

Reply via email to