diff options
author | Samuel Huang <huangs@chromium.org> | 2019-11-25 21:02:09 +0000 |
---|---|---|
committer | Copybara-Service <copybara-worker@google.com> | 2021-07-25 20:58:12 -0700 |
commit | a565cf1dc2375b11fbc5525e5b44103376f336d4 (patch) | |
tree | ba975ed5eafae75381758cca140fe3ccb803ef82 /disassembler_elf.h | |
parent | 5de6c9ca574332e36e10a55fffc34a7ef815a0c5 (diff) | |
download | zucchini-a565cf1dc2375b11fbc5525e5b44103376f336d4.tar.gz |
[Zucchini] Fix 4 DisassemblerElf bugs discovered by new fuzzer.
This CL redoes section processing in DisassemblerElf to fix 4 bugs
discovered by zucchini_disassembler_elf_fuzzer that ckitagawa@ recently
added. These bugs all involve some malformed SHF_EXECINSTR section that
sneaks past early checks to reach pointer extraction, then triggers
another check. Summaries:
* Issue 1023095: |sh_type == SHT_NOBITS| helps section bypass bound
checks. DCHECK is triggered in Abs32GapFinder::Abs32GapFinder()
because |sh_size| grossly exceeds bounds.
* Issue 1023183: |sh_size == 0| and |sh_offset < image_.size()| makes
section seem benign, but |sh_offset| is ignored when computing
|offset_bound|. DCHECK is triggred in Abs32GapFinder() ctor because
|sh_offset| > estimated image size (|offset_bound|).
* Issue 1023203: ELF64. |sh_size == 0| and |sh_offset == 0| make
section seem benign. However, |sh_addr| far exceeds 32-bit bound. In
ParseExecSection(), |sh_addr| fails base::checked_cast().
* Issue 1023210: Section has vaid bounds, and |sh_addr == 0| makes
section excluded (heuristically) from AddressTranslator. Section
proceeds to ParseExecSection(), which finds a rel32 whose:
* Location offset is assumed okay.
* Location RVA, by optimization, is converted directly using section
data, and is also okay.
* Target RVA is validated by AddressTranslator.
But in Rel32ReaderX86::GetNext(), location offset -> RVA now uses
AddressTranslator, which by earlier exclusion, results in
kInvalidOffset. This pollutes target RVA and target offset, and
triggers DCHECK.
The above shows mismatches among usage of sections for the following:
* Location / RVA matching (AddressTranslator),
* ELF image size estimation (|offset_bound|),
* Pointer extraction,
against bypasses due to |sh_type == SHT_NOBITS|, |sh_size == 0|, and
|sh_addr == 0|.
To fix the issues, this CL separates decision logic from enactment.
Decision logic is moved to JudgeSections(), which takes a section and
returns a "judgement" consisting of bit field defined from new enum
SectionJudgement. The judgement is enacted in ParseHeader(), which
chooses to invalidate the ELF, ignore the section, or use the section
with greater discretion on applying pointer extraction.
Additional fix: Ignore (non-fatal) sections with SHF_TLS bit, since
these sections can have offset-RVA ranges that conflict with other
section's. Without this fix, Zucchini on Ubuntu won't recognize itself
as an ELF file!
Bug: 1023095, 1023183, 1023203, 1023210, 1022538
Change-Id: Icc86f26db17a61bb41b432177ef6c3dc0dcd1a26
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1933632
Commit-Queue: Samuel Huang <huangs@chromium.org>
Reviewed-by: Calder Kitagawa <ckitagawa@chromium.org>
Cr-Commit-Position: refs/heads/master@{#718809}
NOKEYCHECK=True
GitOrigin-RevId: f2e5fba3a476a96d5a412603385cf47116c51251
Diffstat (limited to 'disassembler_elf.h')
-rw-r--r-- | disassembler_elf.h | 3 |
1 files changed, 3 insertions, 0 deletions
diff --git a/disassembler_elf.h b/disassembler_elf.h index 866c3b6..e279c29 100644 --- a/disassembler_elf.h +++ b/disassembler_elf.h @@ -131,6 +131,9 @@ class DisassemblerElf : public Disassembler { elf::Elf32_Half segments_count_ = 0; const typename Traits::Elf_Phdr* segments_ = nullptr; + // Bit fields to store the role each section may play. + std::vector<int> section_judgements_; + // Translator between offsets and RVAs. AddressTranslator translator_; |