diff options
author | ckitagawa <ckitagawa@chromium.org> | 2020-01-21 16:39:58 +0000 |
---|---|---|
committer | Copybara-Service <copybara-worker@google.com> | 2021-07-25 21:00:18 -0700 |
commit | 3545aaf3da52a1f86aac3a9173c179ebd6efe566 (patch) | |
tree | 7a316fb95ff44e88feb0c29f8b0d545053a3c7a8 /disassembler_elf.cc | |
parent | 1940ec984a6a137ee087b811fc154f42e9b689e0 (diff) | |
download | zucchini-3545aaf3da52a1f86aac3a9173c179ebd6efe566.tar.gz |
[Zucchini] Fix checked_cast failure
This fixes a case where 32-bit Zucchini with size being 32-bit will
crash when handling a 64-bit ELF file if there is a segment that
extends beyond the image bounds.
This is a quick fix, but there is a larger issue at play here. See the
bug to discuss options.
Bug: 1035603
Change-Id: I74887b8c6b8779642910de1bc31dccfcdc0d1bec
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2008356
Commit-Queue: Calder Kitagawa <ckitagawa@chromium.org>
Reviewed-by: Samuel Huang <huangs@chromium.org>
Cr-Commit-Position: refs/heads/master@{#733625}
NOKEYCHECK=True
GitOrigin-RevId: f3ba433da667d8cda30ac4db8402a9137c281ee1
Diffstat (limited to 'disassembler_elf.cc')
-rw-r--r-- | disassembler_elf.cc | 13 |
1 files changed, 10 insertions, 3 deletions
diff --git a/disassembler_elf.cc b/disassembler_elf.cc index ad2cafd..4727d1c 100644 --- a/disassembler_elf.cc +++ b/disassembler_elf.cc @@ -10,6 +10,7 @@ #include <utility> #include "base/logging.h" +#include "base/numerics/checked_math.h" #include "base/numerics/safe_conversions.h" #include "components/zucchini/abs32_utils.h" #include "components/zucchini/algorithm.h" @@ -275,10 +276,16 @@ bool DisassemblerElf<Traits>::ParseHeader() { // Visits |segments_| to get estimate on |offset_bound|. for (const typename Traits::Elf_Phdr* segment = segments_; segment != segments_ + segments_count_; ++segment) { - if (!image_.covers({segment->p_offset, segment->p_filesz})) + // |image_.covers()| is a sufficient check except when size_t is 32 bit and + // parsing ELF64. In such cases a value-in-range check is needed on the + // segment. This fixes crbug/1035603. + offset_t segment_end; + base::CheckedNumeric<offset_t> checked_segment_end = segment->p_offset; + checked_segment_end += segment->p_filesz; + if (!checked_segment_end.AssignIfValid(&segment_end) || + !image_.covers({segment->p_offset, segment->p_filesz})) { return false; - offset_t segment_end = - base::checked_cast<offset_t>(segment->p_offset + segment->p_filesz); + } offset_bound = std::max(offset_bound, segment_end); } |