SEGV when ELF has a malformed .dynamic section

[chenx6]

Describe the bug

When ELF has a malformed .dynamic section, running ./src/patchelf --shrink-rpath --allowed-rpath-prefixes /usr/lib:/foo/lib malformed_elf will cause patchelf crash.

Steps To Reproduce

./configure CFLAGS='-g -O0' CXXFLAGS='-g -O0' --with-asan
make
./src/patchelf --shrink-rpath --allowed-rpath-prefixes /usr/lib:/foo/lib malformed_elf

Expected behavior

patchelf not crash

patchelf --version output

patchelf 0.18.0

Additional context

ELF file: malformed_elf.zip

ASAN output:

$ ./src/patchelf --shrink-rpath --allowed-rpath-prefixes /usr/lib:/foo/lib malformed_elf
AddressSanitizer:DEADLYSIGNAL
=================================================================
==9147==ERROR: AddressSanitizer: SEGV on unknown address 0x624044334509 (pc 0x7fecfad6c26d bp 0x7ffd36d52300 sp 0x7ffd36d51aa8 T0)
==9147==The signal is caused by a READ memory access.
    #0 0x7fecfad6c26d in __strlen_avx2 (/lib64/libc.so.6+0x16c26d) (BuildId: bbeee08e5f56966e641c4f3ba4ea1da9d730d0ab)
    #1 0x7fecfb46935c  (/lib64/libasan.so.8+0x6935c) (BuildId: 3e1694ad218c99a8b1b69231666a27df63cf19d0)
    #2 0x410946 in std::char_traits<char>::length(char const*) /usr/include/c++/13/bits/char_traits.h:409
    #3 0x4143db in std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >::basic_string<std::allocator<char> >(char const*, std::allocator<char> const&) /usr/include/c++/13/bits/basic_string.h:638
    #4 0x41f746 in ElfFile<Elf32_Ehdr, Elf32_Phdr, Elf32_Shdr, unsigned int, unsigned int, Elf32_Dyn, Elf32_Sym, unsigned short, Elf32_Verdef, Elf32_Verdaux, Elf32_Verneed, Elf32_Vernaux, Elf32_Rel, Elf32_Rela, 32u>::modifyRPath(ElfFile<Elf32_Ehdr, Elf32_Phdr, Elf32_Shdr, unsigned int, unsigned int, Elf32_Dyn, Elf32_Sym, unsigned short, Elf32_Verdef, Elf32_Verdaux, Elf32_Verneed, Elf32_Vernaux, Elf32_Rel, Elf32_Rela, 32u>::RPathOp, std::vector<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::allocator<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > > > const&, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >) /home/chenx/devp/repo/patchelf/src/patchelf.cc:1586
    #5 0x40d27f in patchElf2<ElfFile<Elf32_Ehdr, Elf32_Phdr, Elf32_Shdr, unsigned int, unsigned int, Elf32_Dyn, Elf32_Sym, short unsigned int, Elf32_Verdef, Elf32_Verdaux, Elf32_Verneed, Elf32_Vernaux, Elf32_Rel, Elf32_Rela, 32> > /home/chenx/devp/repo/patchelf/src/patchelf.cc:2420
    #6 0x4087d7 in patchElf /home/repo/patchelf/src/patchelf.cc:2463
    #7 0x40b96e in mainWrapped /home/repo/patchelf/src/patchelf.cc:2685
    #8 0x40c523 in main /home/repo/patchelf/src/patchelf.cc:2693
    #9 0x7fecfac281af in __libc_start_call_main (/lib64/libc.so.6+0x281af) (BuildId: bbeee08e5f56966e641c4f3ba4ea1da9d730d0ab)
    #10 0x7fecfac28278 in __libc_start_main@@GLIBC_2.34 (/lib64/libc.so.6+0x28278) (BuildId: bbeee08e5f56966e641c4f3ba4ea1da9d730d0ab)
    #11 0x4059c4 in _start ../sysdeps/x86_64/start.S:115

AddressSanitizer can not provide additional info.
SUMMARY: AddressSanitizer: SEGV (/lib64/libc.so.6+0x16c26d) (BuildId: bbeee08e5f56966e641c4f3ba4ea1da9d730d0ab) in __strlen_avx2
==9147==ABORTING

When the tag of the element in .dynamic section is 0x1 and has a huge value (in the following example, 0x44332211), patchelf will read string from strTab + rdi(dyn->d_un.d_val), which will cause a OOB read.

$ readelf -d ./malformed_elf

Dynamic section at offset 0x600 contains 20 entries:
  Tag        Type                         Name/Value
 0x00000001 (NEEDED)                     0x44332211
 0x0000000c (INIT)                       0x4002c0
 0x0000000d (FINI)                       0x4005a0
 0x00000004 (HASH)                       0x40016c
 0x00000005 (STRTAB)                     0x4001f8
 0x00000006 (SYMTAB)                     0x400198
 0x0000000a (STRSZ)                      80 (bytes)
 0x0000000b (SYMENT)                     16 (bytes)
 0x00000015 (DEBUG)                      0x0
 0x00000003 (PLTGOT)                     0x4106f8
 0x00000002 (PLTRELSZ)                   48 (bytes)
 0x00000014 (PLTREL)                     RELA
 0x00000017 (JMPREL)                     0x400280
 0x00000007 (RELA)                       0x400274
 0x00000008 (RELASZ)                     12 (bytes)
 0x00000009 (RELAENT)                    12 (bytes)
 0x6ffffffe (VERNEED)                    0x400254
 0x6fffffff (VERNEEDNUM)                 1
 0x6ffffff0 (VERSYM)                     0x400248
 0x00000000 (NULL)                       0x0

I think add a check for strTab + rdi(dyn->d_un.d_val) before read string from memory will solve the problem.

diff --git a/src/patchelf.cc b/src/patchelf.cc
index 82b4b46..01be397 100644
--- a/src/patchelf.cc
+++ b/src/patchelf.cc
@@ -1572,6 +1572,9 @@ void ElfFile<ElfFileParamNames>::modifyRPath(RPathOp op,
     Elf_Dyn *dynRPath = nullptr, *dynRunPath = nullptr;
     char * rpath = nullptr;
     for ( ; rdi(dyn->d_tag) != DT_NULL; dyn++) {
+        if ((unsigned char *)(strTab + rdi(dyn->d_un.d_val)) >= fileContents->data() + fileContents->size()) {
+            error("OOB read for strTab");
+        }
         if (rdi(dyn->d_tag) == DT_RPATH) {
             dynRPath = dyn;
             /* Only use DT_RPATH if there is no DT_RUNPATH. */