PECOFF: Fix section name computation

[labath]

Summary

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.

[zturner]

What about just making this function return a StringRef?

[labath]

An excellent idea. Updating to use StringRef.

lit/Modules/PECOFF/sections-names.yaml

+# RUN: yaml2obj %s > %t
+# RUN: lldb-test object-file %t | FileCheck %s
+
+# CHECK: Name: .text{{$}}
+# CHECK: Name: 1234567{{$}}
+# CHECK: Name: 12345678{{$}}
+# CHECK: Name: 123456789{{$}}
+
+--- !COFF
+OptionalHeader:
+  AddressOfEntryPoint: 4616
+  ImageBase:       1073741824
+  SectionAlignment: 4096
+  FileAlignment:   512
+  MajorOperatingSystemVersion: 6
+  MinorOperatingSystemVersion: 0
+  MajorImageVersion: 0
+  MinorImageVersion: 0
+  MajorSubsystemVersion: 6
+  MinorSubsystemVersion: 0
+  Subsystem:       IMAGE_SUBSYSTEM_WINDOWS_CUI
+  DLLCharacteristics: [ IMAGE_DLL_CHARACTERISTICS_HIGH_ENTROPY_VA, IMAGE_DLL_CHARACTERISTICS_DYNAMIC_BASE, IMAGE_DLL_CHARACTERISTICS_NX_COMPAT, IMAGE_DLL_CHARACTERISTICS_TERMINAL_SERVER_AWARE ]
+  SizeOfStackReserve: 1048576
+  SizeOfStackCommit: 4096
+  SizeOfHeapReserve: 1048576
+  SizeOfHeapCommit: 4096
+header:
+  Machine:         IMAGE_FILE_MACHINE_AMD64
+  Characteristics: [ IMAGE_FILE_EXECUTABLE_IMAGE, IMAGE_FILE_LARGE_ADDRESS_AWARE ]
+sections:
+  - Name:            .text
+    Characteristics: [ IMAGE_SCN_CNT_CODE, IMAGE_SCN_MEM_EXECUTE, IMAGE_SCN_MEM_READ ]
+    VirtualAddress:  4096
+    VirtualSize:     64 # '@', if it makes its way into the name field
+    SectionData:     DEADBEEFBAADF00D
+  - Name:            1234567
+    Characteristics: [ IMAGE_SCN_CNT_INITIALIZED_DATA, IMAGE_SCN_MEM_READ ]
+    VirtualAddress:  442368
+    VirtualSize:     64
+    SectionData:     DEADBEEFBAADF00D
+  - Name:            12345678
+    Characteristics: [ IMAGE_SCN_CNT_INITIALIZED_DATA, IMAGE_SCN_MEM_READ ]
+    VirtualAddress:  446464
+    VirtualSize:     64
+    SectionData:     DEADBEEFBAADF00D
+  - Name:            123456789
+    Characteristics: [ IMAGE_SCN_CNT_INITIALIZED_DATA, IMAGE_SCN_MEM_READ ]
+    VirtualAddress:  450560
+    VirtualSize:     64
+    SectionData:     DEADBEEFBAADF00D
+symbols:         []
+...

source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.h

protected:
   static void DumpCOFFHeader(lldb_private::Stream *s,
                              const coff_header_t &header);
   static void DumpOptCOFFHeader(lldb_private::Stream *s,
                                 const coff_opt_header_t &header);
   void DumpSectionHeaders(lldb_private::Stream *s);
   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);
+  std::string GetSectionName(const section_header_t &sect);
 
   typedef std::vector<section_header_t> SectionHeaderColl;
   typedef SectionHeaderColl::iterator SectionHeaderCollIter;
   typedef SectionHeaderColl::const_iterator SectionHeaderCollConstIter;
 
 private:
   bool CreateBinary();
 

source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp

if (section_header_data.ValidOffsetForDataOfSize(
         }
       }
     }
   }
 
   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);
+std::string 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;
-  }
-  sect_name = sect.name;
-  return true;
+    if (const char *name = m_data.GetCStr(&string_file_offset))
+      return name;
+    return "";
+  return hdr_name;
 }
 
 //----------------------------------------------------------------------
 // GetNListSymtab
 //----------------------------------------------------------------------
 Symtab *ObjectFilePECOFF::GetSymtab() {
   ModuleSP module_sp(GetModule());
   if (module_sp) {
void ObjectFilePECOFF::CreateSections(SectionList &unified_section_list) {
   m_sections_ap.reset(new SectionList());
 
   ModuleSP module_sp(GetModule());
   if (module_sp) {
     std::lock_guard<std::recursive_mutex> guard(module_sp->GetMutex());
     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]);
+      std::string sect_name = GetSectionName(m_sect_headers[idx]);
       ConstString const_sect_name(sect_name.c_str());
       static ConstString g_code_sect_name(".code");
       static ConstString g_CODE_sect_name("CODE");
       static ConstString g_data_sect_name(".data");
       static ConstString g_DATA_sect_name("DATA");
       static ConstString g_bss_sect_name(".bss");
       static ConstString g_BSS_sect_name("BSS");
       static ConstString g_debug_sect_name(".debug");

 }
 //----------------------------------------------------------------------
 // DumpSectionHeader
 //
 // Dump a single ELF section header to the specified output stream
 //----------------------------------------------------------------------
 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,
             sh.lineoff, sh.nreloc, sh.nline, sh.flags);
 }
 
 //----------------------------------------------------------------------
 // DumpSectionHeaders