[llvm-readobj] - Validate the DT_STRSZ value to avoid crash.
authorGeorgii Rymar <grimar@accesssoftek.com>
Fri, 19 Jun 2020 15:37:15 +0000 (18:37 +0300)
committerGeorgii Rymar <grimar@accesssoftek.com>
Mon, 22 Jun 2020 12:24:59 +0000 (15:24 +0300)
It is possible to trigger a crash when a dynamic symbol has a
broken (too large) st_name and the DT_STRSZ is also broken.

We have the following code in the `Elf_Sym_Impl<ELFT>::getName`:

```
template <class ELFT>
Expected<StringRef> Elf_Sym_Impl<ELFT>::getName(StringRef StrTab) const {
  uint32_t Offset = this->st_name;
  if (Offset >= StrTab.size())
    return createStringError(object_error::parse_failed,
                             "st_name (0x%" PRIx32
                             ") is past the end of the string table"
                             " of size 0x%zx",
                             Offset, StrTab.size());
...
```

The problem is that `StrTab` here is a `ELFDumper::DynamicStringTab` member
which is not validated properly on initialization. So it is possible to bypass the
`if` even when the `st_name` is huge.

This patch fixes the issue.

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

llvm/test/tools/llvm-readobj/ELF/dyn-symbols.test
llvm/test/tools/llvm-readobj/ELF/dynamic-malformed.test
llvm/tools/llvm-readobj/ELFDumper.cpp

index d72c43a..0cb3386 100644 (file)
@@ -429,3 +429,64 @@ Sections:
         Value: 0x123
       - Tag:   DT_NULL
         Value: 0
+
+## Check we report a warning when the DT_STRSZ value is broken so that the dynamic string
+## table goes past the end of the file. Document we stop dumping symbols and report an error.
+
+# RUN: yaml2obj %s --docnum=13 -o %t14
+# RUN: not llvm-readobj --dyn-symbols %t14 2>&1 | \
+# RUN:   FileCheck %s -DFILE=%t14 --check-prefix=DYNSTR-INVALID-LLVM
+# RUN: not llvm-readelf --dyn-symbols %t14 2>&1 | \
+# RUN:   FileCheck %s -DFILE=%t14 --check-prefix=DYNSTR-INVALID-GNU
+
+# DYNSTR-INVALID-LLVM: warning: '[[FILE]]': the dynamic string table at 0x78 goes past the end of the file (0x2a8) with DT_STRSZ = 0xffffffff
+# DYNSTR-INVALID-LLVM:      DynamicSymbols [
+# DYNSTR-INVALID-LLVM-NEXT:   Symbol {
+# DYNSTR-INVALID-LLVM-NEXT:     Name:  (0)
+# DYNSTR-INVALID-LLVM-NEXT:     Value: 0x0
+# DYNSTR-INVALID-LLVM-NEXT:     Size: 0
+# DYNSTR-INVALID-LLVM-NEXT:     Binding: Local (0x0)
+# DYNSTR-INVALID-LLVM-NEXT:     Type: None (0x0)
+# DYNSTR-INVALID-LLVM-NEXT:     Other: 0
+# DYNSTR-INVALID-LLVM-NEXT:     Section: Undefined (0x0)
+# DYNSTR-INVALID-LLVM-NEXT:   }
+# DYNSTR-INVALID-LLVM-NEXT: error: '[[FILE]]': st_name (0xffffff00) is past the end of the string table of size 0x6
+
+# DYNSTR-INVALID-GNU: warning: '[[FILE]]': the dynamic string table at 0x78 goes past the end of the file (0x2a8) with DT_STRSZ = 0xffffffff
+# DYNSTR-INVALID-GNU:      Symbol table '.dynsym' contains 3 entries:
+# DYNSTR-INVALID-GNU-NEXT:    Num:    Value          Size Type    Bind   Vis       Ndx Name
+# DYNSTR-INVALID-GNU-NEXT:      0: 0000000000000000     0 NOTYPE  LOCAL  DEFAULT   UND
+# DYNSTR-INVALID-GNU-NEXT: error: '[[FILE]]': st_name (0xffffff00) is past the end of the string table of size 0x6
+
+--- !ELF
+FileHeader:
+  Class:   ELFCLASS64
+  Data:    ELFDATA2LSB
+  Type:    ET_EXEC
+  Machine: EM_X86_64
+Sections:
+  - Name:    .dynstr
+    Type:    SHT_STRTAB
+    Flags:   [ SHF_ALLOC ]
+    Content: '007465737400' ## '\0', 'test', '\0'
+  - Name:  .dynamic
+    Type:  SHT_DYNAMIC
+    Flags: [ SHF_ALLOC ]
+    Link:  .dynstr
+    Entries:
+      - Tag:   DT_STRTAB
+        Value: 0x0
+      - Tag:   DT_STRSZ
+        Value: 0xffffffff
+      - Tag:   DT_NULL
+        Value: 0x0
+DynamicSymbols:
+  - StName:  0xffffff00
+## An arbitrary valid symbol to document we report an error before dumping it.
+  - StName:  0x1
+ProgramHeaders:
+  - Type:  PT_LOAD
+    Flags: [ PF_R ]
+    Sections:
+      - Section: .dynstr
+      - Section: .dynamic
index ed72118..8b6fc1e 100644 (file)
@@ -400,7 +400,8 @@ ProgramHeaders:
 # RUN: llvm-readobj --dynamic-table %t9.2 2>&1 | FileCheck %s -DFILE=%t9.2 --check-prefix=PAST-THE-EOF
 # RUN: llvm-readelf --dynamic-table %t9.2 2>&1 | FileCheck %s -DFILE=%t9.2 --check-prefix=PAST-THE-EOF
 
-# PAST-THE-EOF:      warning: '[[FILE]]': string table at offset 0xb0 with size 0x211 goes past the end of the file (0x2c0)
+# PAST-THE-EOF:      warning: '[[FILE]]': the dynamic string table at 0xb0 goes past the end of the file (0x2c0) with DT_STRSZ = 0x211
+# PAST-THE-EOF:      warning: '[[FILE]]': string table was not found
 # PAST-THE-EOF:      {{[(]?}}NEEDED{{[)]?}}    Shared library: [<?>]
 # PAST-THE-EOF-NEXT: {{[(]?}}FILTER{{[)]?}}    Filter library: [<?>]
 # PAST-THE-EOF-NEXT: {{[(]?}}AUXILIARY{{[)]?}} Auxiliary library: [<?>]
index 832e7ce..866ceca 100644 (file)
@@ -2185,8 +2185,20 @@ void ELFDumper<ELFT>::parseDynamicTable(const ELFFile<ELFT> *Obj) {
       break;
     }
   }
-  if (StringTableBegin)
-    DynamicStringTable = StringRef(StringTableBegin, StringTableSize);
+
+  if (StringTableBegin) {
+    const uint64_t FileSize = ObjF->getELFFile()->getBufSize();
+    const uint64_t Offset =
+        (const uint8_t *)StringTableBegin - ObjF->getELFFile()->base();
+    if (StringTableSize > FileSize - Offset)
+      reportUniqueWarning(createError(
+          "the dynamic string table at 0x" + Twine::utohexstr(Offset) +
+          " goes past the end of the file (0x" + Twine::utohexstr(FileSize) +
+          ") with DT_STRSZ = 0x" + Twine::utohexstr(StringTableSize)));
+    else
+      DynamicStringTable = StringRef(StringTableBegin, StringTableSize);
+  }
+
   SOName = getDynamicString(SONameOffset);
 
   if (DynSymRegion) {