From f1f7b5272fdf105592fc262a9adaab55a3621122 Mon Sep 17 00:00:00 2001 From: Zequan Wu Date: Wed, 10 Aug 2022 10:55:51 -0700 Subject: [PATCH] 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 --- src/common/dwarf/dwarf2reader.cc | 40 +++++++++++++++++++++----------- src/common/dwarf/elf_reader.cc | 13 ++++++++++- 2 files changed, 39 insertions(+), 14 deletions(-) diff --git a/src/common/dwarf/dwarf2reader.cc b/src/common/dwarf/dwarf2reader.cc index 33a63fc4..9cd513da 100644 --- a/src/common/dwarf/dwarf2reader.cc +++ b/src/common/dwarf/dwarf2reader.cc @@ -927,7 +927,7 @@ void CompilationUnit::ProcessDIEs() { lengthstart += 4; std::stack 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(dirindex), + handler->DefineFile(filename, -1, static_cast(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) { diff --git a/src/common/dwarf/elf_reader.cc b/src/common/dwarf/elf_reader.cc index c6d9f08a..9a7b09f7 100644 --- a/src/common/dwarf/elf_reader.cc +++ b/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(name, path_, fd_, section_headers_[num]); - return reader; + return reader->contents() ? reader : nullptr; } // Parse out the overall header information from the file and assert