libelf: Fix out-of-bounds read in elf_getarsym for unterminated symbol table

elf_getarsym parses the archive symbol table (the "/" or "/SYM64/"
member).  After validating only the size of the index, it walks the
declared number of symbols and, for each one, computes a hash with
_dl_elf_hash and advances to the next name with rawmemchr (or an
open-coded loop on platforms without rawmemchr).  Both of these scan
for a terminating NUL byte without any bound.

Nothing guarantees that the string area following the offset table
actually contains that many NUL-terminated names, or any NUL at all.
A crafted archive whose symbol-table string area is not NUL terminated
makes the name scan run past the end of the heap buffer holding the
string data (or past the mapped file) -- an out-of-bounds read.

Remember the end of the string area and use a bounded memchr to locate
each name terminator.  If a name is not terminated within the string
area the archive is malformed, so fail with ELF_E_INVALID_ARCHIVE
instead of reading out of bounds.

Reproduced under AddressSanitizer with a crafted archive whose string
area contains no NUL byte:

  ==ERROR: AddressSanitizer: heap-buffer-overflow ... READ of size 1
    #0 _dl_elf_hash dl-hash.h:45
    #1 elf_getarsym elf_getarsym.c:294
  0 bytes after 116-byte region ... allocated by elf_getarsym.c:215

The function is reachable from eu-nm -s and eu-readelf, which call
elf_getarsym on untrusted .a files.

Signed-off-by: Sayed Kaif <skaif@digiscrypt.com>

diff --git a/libelf/elf_getarsym.c b/libelf/elf_getarsym.c
index 281f0c1c56022bef6d8adc17b96be9707b971156..ec05c684abb6160901926865b4d2e3d335c0ad49 100644 (file)
--- a/libelf/elf_getarsym.c
+++ b/libelf/elf_getarsym.c
@@ -199,6 +199,7 @@ elf_getarsym (Elf *elf, size_t *ptr)
        {
          void *file_data; /* unit32_t[n] or uint64_t[n] */
          char *str_data;
+         char *str_end;
          size_t sz = n * w;
 
          if (elf->map_address == NULL)
@@ -238,6 +239,7 @@ elf_getarsym (Elf *elf, size_t *ptr)
                }
 
              str_data = (char *) new_str;
+             str_end = new_str + (index_size - sz);
            }
          else
            {
@@ -254,6 +256,7 @@ elf_getarsym (Elf *elf, size_t *ptr)
                  file_data = memcpy (temp_data, elf->map_address + off, sz);
                }
              str_data = (char *) (elf->map_address + off + sz);
+             str_end = (char *) (elf->map_address + off + index_size - w);
            }
 
          /* Now we can build the data structure.  */
@@ -291,16 +294,25 @@ elf_getarsym (Elf *elf, size_t *ptr)
              else
                arsym[cnt].as_off = (*u32)[cnt];
 
+             /* The symbol name must be NUL terminated within the string
+                table.  Otherwise the archive symbol table is corrupt and
+                hashing or scanning the name would read out of bounds.  */
+             char *endp = (str_data < str_end
+                           ? memchr (str_data, '\0', str_end - str_data)
+                           : NULL);
+             if (unlikely (endp == NULL))
+               {
+                 if (elf->map_address == NULL)
+                   {
+                     free (elf->state.ar.ar_sym);
+                     elf->state.ar.ar_sym = NULL;
+                   }
+                 __libelf_seterrno (ELF_E_INVALID_ARCHIVE);
+                 goto out;
+               }
+
              arsym[cnt].as_hash = _dl_elf_hash (str_data);
-#if HAVE_DECL_RAWMEMCHR
-             str_data = rawmemchr (str_data, '\0') + 1;
-#else
-             char c;
-             do {
-               c = *str_data;
-               str_data++;
-             } while (c);
-#endif
+             str_data = endp + 1;
            }
 
          /* At the end a special entry.  */