Check sh_type for symbol table and finish ProcessDIEs if any DIE processing goes wrong

- If symbol table section is malformed, skip them.
- SkipDIE and ProcessDIE return nullptr when processing goes wrong due to malformed debug info, stop processing in this case.

Bug: 1349354
Change-Id: Ia1d3e3591bbd2dad8b9eb351c1882cfc03bfad4b
Reviewed-on: https://chromium-review.googlesource.com/c/breakpad/breakpad/+/3821448
Reviewed-by: Joshua Peraza <jperaza@chromium.org>

src/common/dwarf/dwarf2reader.cc

@@ -927,7 +927,7 @@ void CompilationUnit::ProcessDIEs() {
     lengthstart += 4;
 
   std::stack<uint64_t> die_stack;
-  
+
   while (dieptr < (lengthstart + header_.length)) {
     // We give the user the absolute offset from the beginning of
     // debug_info, since they need it to deal with ref_addr forms.
@@ -953,8 +953,22 @@ void CompilationUnit::ProcessDIEs() {
     const enum DwarfTag tag = abbrev.tag;
     if (!handler_->StartDIE(absolute_offset, tag)) {
       dieptr = SkipDIE(dieptr, abbrev);
+      if (!dieptr) {
+        fprintf(stderr,
+                "An error happens when skipping a DIE's attributes at offset "
+                "%lx. Stopped processing following DIEs in this CU.\n",
+                absolute_offset);
+        exit(1);
+      }
     } else {
       dieptr = ProcessDIE(absolute_offset, dieptr, abbrev);
+      if (!dieptr) {
+        fprintf(stderr,
+                "An error happens when processing a DIE at offset %lx. Stopped "
+                "processing following DIEs in this CU.\n",
+                absolute_offset);
+        exit(1);
+      }
     }
 
     if (abbrev.has_children) {
@@ -1718,7 +1732,7 @@ bool LineInfo::ProcessOneOpcode(ByteReader* reader,
           oplen += templen;
 
           if (handler) {
-            handler->DefineFile(filename, -1, static_cast<uint32_t>(dirindex), 
+            handler->DefineFile(filename, -1, static_cast<uint32_t>(dirindex),
                                 mod_time, filelength);
           }
         }
@@ -1780,7 +1794,7 @@ void LineInfo::ReadLines() {
                           pending_file_num, pending_line_num,
                           pending_column_num);
       if (lsm.end_sequence) {
-        lsm.Reset(header_.default_is_stmt);      
+        lsm.Reset(header_.default_is_stmt);
         have_pending_line = false;
       } else {
         pending_address = lsm.address;
@@ -2267,7 +2281,7 @@ class CallFrameInfo::State {
   // report the problem to reporter_ and return false.
   bool InterpretFDE(const FDE& fde);
 
- private:  
+ private:
   // The operands of a CFI instruction, for ParseOperands.
   struct Operands {
     unsigned register_number;  // A register number.
@@ -2528,19 +2542,19 @@ bool CallFrameInfo::State::DoInstruction() {
       if (!ParseOperands("1", &ops)) return false;
       address_ += ops.offset * cie->code_alignment_factor;
       break;
-      
+
     // Advance the address.
     case DW_CFA_advance_loc2:
       if (!ParseOperands("2", &ops)) return false;
       address_ += ops.offset * cie->code_alignment_factor;
       break;
-      
+
     // Advance the address.
     case DW_CFA_advance_loc4:
       if (!ParseOperands("4", &ops)) return false;
       address_ += ops.offset * cie->code_alignment_factor;
       break;
-      
+
     // Advance the address.
     case DW_CFA_MIPS_advance_loc8:
       if (!ParseOperands("8", &ops)) return false;
@@ -2850,7 +2864,7 @@ bool CallFrameInfo::ReadEntryPrologue(const uint8_t* cursor, Entry* entry) {
   // Validate the length.
   if (length > size_t(buffer_end - cursor))
     return ReportIncomplete(entry);
- 
+
   // The length is the number of bytes after the initial length field;
   // we have that position handy at this point, so compute the end
   // now. (If we're parsing 64-bit-offset DWARF on a 32-bit machine,
@@ -2892,7 +2906,7 @@ bool CallFrameInfo::ReadEntryPrologue(const uint8_t* cursor, Entry* entry) {
 
   // Now advance cursor past the id.
    cursor += offset_size;
- 
+
   // The fields specific to this kind of entry start here.
   entry->fields = cursor;
 
@@ -3120,7 +3134,7 @@ bool CallFrameInfo::ReadFDEFields(FDE* fde) {
     if (size_t(fde->end - cursor) < size + data_size)
       return ReportIncomplete(fde);
     cursor += size;
-    
+
     // In the abstract, we should walk the augmentation string, and extract
     // items from the FDE's augmentation data as we encounter augmentation
     // string characters that specify their presence: the ordering of items
@@ -3158,7 +3172,7 @@ bool CallFrameInfo::ReadFDEFields(FDE* fde) {
 
   return true;
 }
-  
+
 bool CallFrameInfo::Start() {
   const uint8_t* buffer_end = buffer_ + buffer_length_;
   const uint8_t* cursor;
@@ -3215,7 +3229,7 @@ bool CallFrameInfo::Start() {
       reporter_->CIEPointerOutOfRange(fde.offset, fde.id);
       continue;
     }
-      
+
     CIE cie;
 
     // Parse this FDE's CIE header.
@@ -3254,7 +3268,7 @@ bool CallFrameInfo::Start() {
       ok = true;
       continue;
     }
-                         
+
     if (cie.has_z_augmentation) {
       // Report the personality routine address, if we have one.
       if (cie.has_z_personality) {

src/common/dwarf/elf_reader.cc

@@ -196,6 +196,17 @@ class ElfSectionReader {
     // to process its contents.
     if (header_.sh_type == SHT_NOBITS || header_.sh_size == 0)
       return;
+    // extra sh_type check for string table.
+    if ((std::strcmp(name, ".strtab") == 0 ||
+         std::strcmp(name, ".shstrtab") == 0) &&
+        header_.sh_type != SHT_STRTAB) {
+      fprintf(stderr,
+              "Invalid sh_type for string table section: expected "
+              "SHT_STRTAB or SHT_DYNSYM, but got %d\n",
+              header_.sh_type);
+      return;
+    }
+
     contents_aligned_ = mmap(NULL, size_aligned_, PROT_READ, MAP_SHARED,
                              fd, offset_aligned);
     // Set where the offset really should begin.
@@ -877,7 +888,7 @@ class ElfReaderImpl {
     if (reader == NULL)
       reader = new ElfSectionReader<ElfArch>(name, path_, fd_,
                                              section_headers_[num]);
-    return reader;
+    return reader->contents() ? reader : nullptr;
   }
 
   // Parse out the overall header information from the file and assert