[PATCH] D76706: [llvm-readobj] - Fix the crash when DT_STRTAB is broken.

[George Rimar via Phabricator via llvm-commits]

grimar created this revision.
grimar added reviewers: jhenderson, MaskRay.
Herald added subscribers: rupprecht, hiraditya, emaste.
Herald added a reviewer: espindola.
grimar retitled this revision from "[llvm-readobj] - Fix a crash when DT_STRTAB is broken." to "[llvm-readobj] - Fix the crash when DT_STRTAB is broken.".

We might have a crash scenario when we have an invalid DT_STRTAB value
that is larger than the file size. I've added a test case to demonstrate.


https://reviews.llvm.org/D76706

Files:
  llvm/lib/Object/ELF.cpp
  llvm/test/tools/llvm-readobj/ELF/loadname.test


Index: llvm/test/tools/llvm-readobj/ELF/loadname.test
===================================================================
--- llvm/test/tools/llvm-readobj/ELF/loadname.test
+++ llvm/test/tools/llvm-readobj/ELF/loadname.test
@@ -1,6 +1,8 @@
 ## Check we are able to dump library soname properly.
 
-# RUN: yaml2obj %s -o %t.o
+## Test a valid object case first. We set 'FileSize' to 0x48, because this is no-op,
+## i.e. this value would be set if we had no 'FileSize' at all.
+# RUN: yaml2obj -DDTSTRTABVAL=0x0 -DPHDRFILESIZE="0x48" %s -o %t.o
 # RUN: llvm-readobj %t.o | FileCheck %s --check-prefix LLVM
 # RUN: llvm-readelf --dynamic-table %t.o | FileCheck %s --check-prefix GNU
 
@@ -33,7 +35,7 @@
     Link:  .dynstr
     Entries:
       - Tag:   DT_STRTAB
-        Value: 0x0000000000000000
+        Value: [[DTSTRTABVAL]]
       - Tag:   DT_STRSZ
         Value: 0x0000000000000007
       - Tag:   DT_SONAME
@@ -41,9 +43,22 @@
       - Tag:   DT_NULL
         Value: 0x0000000000000000
 ProgramHeaders:
-  - Type:  PT_LOAD
-    Flags: [ PF_R ]
-    VAddr: 0x0000
+  - Type:     PT_LOAD
+    Flags:    [ PF_R ]
+    VAddr:    0x0000
+    FileSize: [[PHDRFILESIZE]]
     Sections:
       - Section: .dynstr
       - Section: .dynamic
+
+## Check we do not crash when an object contain a DT_STRTAB entry which contains an offset that goes
+## past the end of a file. Note that we have to set p_filesz for PT_LOAD larger than DT_STRTAB value
+## to trigger this particular warning.
+
+# RUN: yaml2obj -DDTSTRTABVAL=0xFFFE -DPHDRFILESIZE=0xFFFF %s -o %t.err.1.o
+# RUN: llvm-readobj %t.err.1.o 2>&1 | FileCheck %s -DFILE=%t.err.1.o --check-prefixes=BROKEN-OFFSET,BROKEN-OFFSET-LLVM
+# RUN: llvm-readelf --dynamic-table %t.err.1.o 2>&1 | FileCheck %s -DFILE=%t.err.1.o --check-prefixes=BROKEN-OFFSET,BROKEN-OFFSET-GNU
+
+# BROKEN-OFFSET:      warning: '[[FILE]]': Unable to parse DT_STRTAB: can't map virtual address 0xfffe to a segment with index 1: offset goes past the end of file
+# BROKEN-OFFSET-LLVM: LoadName: <String table is empty or was not found>
+# BROKEN-OFFSET-GNU:  0x000000000000000e (SONAME) Library soname: [<String table is empty or was not found>]
Index: llvm/lib/Object/ELF.cpp
===================================================================
--- llvm/lib/Object/ELF.cpp
+++ llvm/lib/Object/ELF.cpp
@@ -580,7 +580,15 @@
   if (Delta >= Phdr.p_filesz)
     return createError("virtual address is not in any segment: 0x" +
                        Twine::utohexstr(VAddr));
-  return base() + Phdr.p_offset + Delta;
+
+  uint64_t Offset = Phdr.p_offset + Delta;
+  if (Offset >= getBufSize())
+    return createError("can't map virtual address 0x" +
+                       Twine::utohexstr(VAddr) + " to a segment with index " +
+                       Twine(&Phdr - (*ProgramHeadersOrError).data() + 1) +
+                       ": offset goes past the end of file");
+
+  return base() + Offset;
 }
 
 template class llvm::object::ELFFile<ELF32LE>;


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D76706.252322.patch
Type: text/x-patch
Size: 2973 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20200324/15359cf1/attachment.bin>