[llvm] ba808b1 - [llvm-readobj] - Validate the DT_STRSZ value to avoid crash.
Georgii Rymar via llvm-commits
llvm-commits at lists.llvm.org
Mon Jun 22 05:31:39 PDT 2020
Author: Georgii Rymar
Date: 2020-06-22T15:24:59+03:00
New Revision: ba808b157e84774e8f384d9436c911c1341105cd
URL: https://github.com/llvm/llvm-project/commit/ba808b157e84774e8f384d9436c911c1341105cd
DIFF: https://github.com/llvm/llvm-project/commit/ba808b157e84774e8f384d9436c911c1341105cd.diff
LOG: [llvm-readobj] - Validate the DT_STRSZ value to avoid crash.
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
Added:
Modified:
llvm/test/tools/llvm-readobj/ELF/dyn-symbols.test
llvm/test/tools/llvm-readobj/ELF/dynamic-malformed.test
llvm/tools/llvm-readobj/ELFDumper.cpp
Removed:
################################################################################
diff --git a/llvm/test/tools/llvm-readobj/ELF/dyn-symbols.test b/llvm/test/tools/llvm-readobj/ELF/dyn-symbols.test
index d72c43aa50ac..0cb338616f94 100644
--- a/llvm/test/tools/llvm-readobj/ELF/dyn-symbols.test
+++ b/llvm/test/tools/llvm-readobj/ELF/dyn-symbols.test
@@ -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
diff --git a/llvm/test/tools/llvm-readobj/ELF/dynamic-malformed.test b/llvm/test/tools/llvm-readobj/ELF/dynamic-malformed.test
index ed72118c4ffc..8b6fc1eca7d2 100644
--- a/llvm/test/tools/llvm-readobj/ELF/dynamic-malformed.test
+++ b/llvm/test/tools/llvm-readobj/ELF/dynamic-malformed.test
@@ -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: [<?>]
diff --git a/llvm/tools/llvm-readobj/ELFDumper.cpp b/llvm/tools/llvm-readobj/ELFDumper.cpp
index 832e7ce9791c..866cecab66ab 100644
--- a/llvm/tools/llvm-readobj/ELFDumper.cpp
+++ b/llvm/tools/llvm-readobj/ELFDumper.cpp
@@ -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) {
More information about the llvm-commits
mailing list