commit 2886e4a08197b458f29d162d428fbf081b698464
author Pavel Labath <pavel@labath.sk> Thu Jan 10 10:23:19 2019 +0000
committer Pavel Labath <pavel@labath.sk> Thu Jan 10 10:23:19 2019 +0000
tree 3a56c24f706bfed9fb7684ea7c607e123d979231
parent b7871405fa91a752ca65df572d2d0a435a9993e1

PECOFF: Fix section name computation

If a section name is exactly 8 bytes long (or has been truncated to 8
bytes), it will not contain the terminating nul character. This means
reading the name as a c string will pick up random data following the
name field (which happens to be the section vm size).

This fixes the name computation to avoid out-of-bounds access and adds a
test.

Reviewers: zturner, stella.stamenova

Subscribers: lldb-commits

Differential Revision: https://reviews.llvm.org/D56124

llvm-svn: 350809

lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp

diff --git a/lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp b/lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
index 3192883..d18ff61 100644
--- a/lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
+++ b/lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
@@ -530,22 +530,20 @@
   return !m_sect_headers.empty();
 }
 
-bool ObjectFilePECOFF::GetSectionName(std::string &sect_name,
-                                      const section_header_t &sect) {
-  if (sect.name[0] == '/') {
-    lldb::offset_t stroff = strtoul(&sect.name[1], NULL, 10);
+llvm::StringRef ObjectFilePECOFF::GetSectionName(const section_header_t &sect) {
+  llvm::StringRef hdr_name(sect.name, llvm::array_lengthof(sect.name));
+  hdr_name = hdr_name.split('\0').first;
+  if (hdr_name.consume_front("/")) {
+    lldb::offset_t stroff;
+    if (!to_integer(hdr_name, stroff, 10))
+      return "";
     lldb::offset_t string_file_offset =
         m_coff_header.symoff + (m_coff_header.nsyms * 18) + stroff;
-    const char *name = m_data.GetCStr(&string_file_offset);
-    if (name) {
-      sect_name = name;
-      return true;
-    }
-
-    return false;
+    if (const char *name = m_data.GetCStr(&string_file_offset))
+      return name;
+    return "";
   }
-  sect_name = sect.name;
-  return true;
+  return hdr_name;
 }
 
 //----------------------------------------------------------------------
@@ -712,9 +710,7 @@
     const uint32_t nsects = m_sect_headers.size();
     ModuleSP module_sp(GetModule());
     for (uint32_t idx = 0; idx < nsects; ++idx) {
-      std::string sect_name;
-      GetSectionName(sect_name, m_sect_headers[idx]);
-      ConstString const_sect_name(sect_name.c_str());
+      ConstString const_sect_name(GetSectionName(m_sect_headers[idx]));
       static ConstString g_code_sect_name(".code");
       static ConstString g_CODE_sect_name("CODE");
       static ConstString g_data_sect_name(".data");
@@ -1086,8 +1082,7 @@
 //----------------------------------------------------------------------
 void ObjectFilePECOFF::DumpSectionHeader(Stream *s,
                                          const section_header_t &sh) {
-  std::string name;
-  GetSectionName(name, sh);
+  std::string name = GetSectionName(sh);
   s->Printf("%-16s 0x%8.8x 0x%8.8x 0x%8.8x 0x%8.8x 0x%8.8x 0x%8.8x 0x%4.4x "
             "0x%4.4x 0x%8.8x\n",
             name.c_str(), sh.vmaddr, sh.vmsize, sh.offset, sh.size, sh.reloff,

lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.h

diff --git a/lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.h b/lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.h
index cf815b6..9fd313f 100644
--- a/lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.h
+++ b/lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.h
@@ -270,7 +270,7 @@
   void DumpSectionHeader(lldb_private::Stream *s, const section_header_t &sh);
   void DumpDependentModules(lldb_private::Stream *s);
 
-  bool GetSectionName(std::string &sect_name, const section_header_t &sect);
+  llvm::StringRef GetSectionName(const section_header_t &sect);
 
   typedef std::vector<section_header_t> SectionHeaderColl;
   typedef SectionHeaderColl::iterator SectionHeaderCollIter;