[llvm] 76a626b - [llvm-readelf/obj] - Fix the possible crash when dumping group sections.

[Georgii Rymar via llvm-commits]

Author: Georgii Rymar
Date: 2020-11-23T13:05:12+03:00
New Revision: 76a626b2061bc8a33656a49ebcabbfa75c317d4c

URL: https://github.com/llvm/llvm-project/commit/76a626b2061bc8a33656a49ebcabbfa75c317d4c
DIFF: https://github.com/llvm/llvm-project/commit/76a626b2061bc8a33656a49ebcabbfa75c317d4c.diff

LOG: [llvm-readelf/obj] - Fix the possible crash when dumping group sections.

It is possible to trigger a crash/misbehavior when the st_name field of
the signature symbol goes past the end of the string table.

This patch fixes it.

Differential revision: https://reviews.llvm.org/D91943

Added: 
    

Modified: 
    llvm/test/tools/llvm-readobj/ELF/groups.test
    llvm/tools/llvm-readobj/ELFDumper.cpp

Removed: 
    


################################################################################
diff  --git a/llvm/test/tools/llvm-readobj/ELF/groups.test b/llvm/test/tools/llvm-readobj/ELF/groups.test
index 00696470e078..afdc63e9dc0b 100644
--- a/llvm/test/tools/llvm-readobj/ELF/groups.test
+++ b/llvm/test/tools/llvm-readobj/ELF/groups.test
@@ -81,11 +81,16 @@ Sections:
   - Name: .symtab
     Type: SHT_SYMTAB
     Link: [[SYMTABLINK=.strtab]]
+  - Name:    .strtab
+    Type:    SHT_STRTAB
+    Content: [[STRTABCONTENT=<none>]]
 Symbols:
   - Name:    foo
     Section: .text.foo
+    StName:  [[SYM1STNAME=<none>]]
   - Name:    bar
     Section: .text.bar
+    StName:  [[SYM2STNAME=<none>]]
 
 ## Check that we report a warning and continue dumping when a section is included
 ## in two group sections at the same time.
@@ -379,3 +384,37 @@ Symbols:
 # MEMBER-GNU-NEXT:     [Index]    Name
 # MEMBER-GNU-NEXT:     [  255]   <?>
 # MEMBER-GNU-NEXT:     [    6]   .rela.text.bar
+
+## Check warnings that are reported when the st_name field of the signature symbol goes past the end of the string table.
+
+## We set the content of the string table to '0061626300' ('\0abc\0') to fixup the size of the string table.
+## This makes it easier to test the boundary conditions.
+# RUN: yaml2obj %s -DSTRTABCONTENT="0061626300" -DSYM1STNAME=4 -DSYM2STNAME=5 -o %t.signame.o
+# RUN: llvm-readobj --elf-section-groups %t.signame.o 2>&1 | \
+# RUN:   FileCheck -DFILE=%t.signame.o %s --check-prefixes=SIGNAME1-WARN,SIGNAME1-LLVM --implicit-check-not=warning:
+# RUN: llvm-readelf --elf-section-groups %t.signame.o 2>&1 | \
+# RUN:   FileCheck -DFILE=%t.signame.o %s --check-prefixes=SIGNAME1-WARN,SIGNAME1-GNU --implicit-check-not=warning:
+
+# SIGNAME1-WARN: warning: '[[FILE]]': unable to get the name of the symbol with index 2: st_name (0x5) is past the end of the string table of size 0x5
+
+# SIGNAME1-LLVM: Signature: {{$}}
+# SIGNAME1-LLVM: Signature: <?>
+
+# SIGNAME1-GNU: COMDAT group section [    1] `.group' [] contains 2 sections:
+# SIGNAME1-GNU: COMDAT group section [    2] `.group1' [<?>] contains 2 sections:
+
+## Chech we report a warning when the string table that contains the signature symbol name is not null-terminated.
+
+# RUN: yaml2obj %s -DSTRTABCONTENT="0061626361" -DSYM1STNAME=4 -DSYM2STNAME=5 -o %t.signame2.o
+# RUN: llvm-readobj --elf-section-groups %t.signame2.o 2>&1 | \
+# RUN:   FileCheck -DFILE=%t.signame2.o %s --check-prefixes=SIGNAME2-WARN,SIGNAME2-LLVM --implicit-check-not=warning:
+# RUN: llvm-readelf --elf-section-groups %t.signame2.o 2>&1 | \
+# RUN:   FileCheck -DFILE=%t.signame2.o %s --check-prefixes=SIGNAME2-WARN,SIGNAME2-GNU --implicit-check-not=warning:
+
+# SIGNAME2-WARN: warning: '[[FILE]]': unable to get the string table for SHT_SYMTAB section with index 7: SHT_STRTAB string table section [index 8] is non-null terminated
+
+# SIGNAME2-LLVM: Signature: <?>
+# SIGNAME2-LLVM: Signature: <?>
+
+# SIGNAME2-GNU: COMDAT group section [    1] `.group' [<?>] contains 2 sections:
+# SIGNAME2-GNU: COMDAT group section [    2] `.group1' [<?>] contains 2 sections:

diff  --git a/llvm/tools/llvm-readobj/ELFDumper.cpp b/llvm/tools/llvm-readobj/ELFDumper.cpp
index 3e54c3599428..c5eb2d628b0b 100644
--- a/llvm/tools/llvm-readobj/ELFDumper.cpp
+++ b/llvm/tools/llvm-readobj/ELFDumper.cpp
@@ -3595,7 +3595,7 @@ static StringRef tryGetSectionName(const ELFFile<ELFT> &Obj,
 }
 
 template <class ELFT> std::vector<GroupSection> DumpStyle<ELFT>::getGroups() {
-  auto GetSignature = [&](const Elf_Sym &Sym,
+  auto GetSignature = [&](const Elf_Sym &Sym, unsigned SymNdx,
                           const Elf_Shdr &Symtab) -> StringRef {
     Expected<StringRef> StrTableOrErr = Obj.getStringTableForSymtab(Symtab);
     if (!StrTableOrErr) {
@@ -3605,8 +3605,16 @@ template <class ELFT> std::vector<GroupSection> DumpStyle<ELFT>::getGroups() {
       return "<?>";
     }
 
-    // TODO: this might lead to a crash or produce a wrong result, when the
-    // st_name goes past the end of the string table.
+    StringRef Strings = *StrTableOrErr;
+    if (Sym.st_name >= Strings.size()) {
+      reportUniqueWarning(createError(
+          "unable to get the name of the symbol with index " + Twine(SymNdx) +
+          ": st_name (0x" + Twine::utohexstr(Sym.st_name) +
+          ") is past the end of the string table of size 0x" +
+          Twine::utohexstr(Strings.size())));
+      return "<?>";
+    }
+
     return StrTableOrErr->data() + Sym.st_name;
   };
 
@@ -3621,7 +3629,7 @@ template <class ELFT> std::vector<GroupSection> DumpStyle<ELFT>::getGroups() {
     if (Expected<const Elf_Shdr *> SymtabOrErr = Obj.getSection(Sec.sh_link)) {
       if (Expected<const Elf_Sym *> SymOrErr =
               Obj.template getEntry<Elf_Sym>(**SymtabOrErr, Sec.sh_info))
-        Signature = GetSignature(**SymOrErr, **SymtabOrErr);
+        Signature = GetSignature(**SymOrErr, Sec.sh_info, **SymtabOrErr);
       else
         reportUniqueWarning(createError(
             "unable to get the signature symbol for " + describe(Obj, Sec) +