Commit 73089e0c authored by ckitagawa's avatar ckitagawa Committed by Commit Bot

[Zucchini] Reduce forgiveness of bounds checks

The current code is too lax. It doesn't enforce bounds checks
strongly enough. It claims to be for RVAs, but allows all sections
through. This results in downstream code being unable to trust that the
regions created are safely within the image resulting in issues when
Fuzzing if the data is ill formed. To fix the fuzzers we should be
remove this forgiveness. However, long term a better check for RVA
forgiveness should maybe be investigated.

Bug: 1013823, 1013842, 1013871, 1014124
Change-Id: Ic164fc76d687711c496f57b3bfe33ced6b8ad838
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1863070Reviewed-by: default avatarSamuel Huang <huangs@chromium.org>
Reviewed-by: default avatarEtienne Pierre-Doray <etiennep@chromium.org>
Commit-Queue: Calder Kitagawa <ckitagawa@chromium.org>
Cr-Commit-Position: refs/heads/master@{#706511}
parent 4d6b5d36
......@@ -211,14 +211,6 @@ bool DisassemblerElf<Traits>::ParseHeader() {
if (section->sh_size == 0)
continue;
// Be lax with RVAs: Assume they fit in int32_t, even for 64-bit. If
// assumption fails, simply skip the section with warning.
if (!RangeIsBounded(section->sh_addr, section->sh_size, kRvaBound) ||
!RangeIsBounded(section->sh_offset, section->sh_size, kOffsetBound)) {
LOG(WARNING) << "Section " << i << " does not fit in int32_t.";
continue;
}
// Extract dimensions to 32-bit integers to facilitate conversion. Range of
// values was ensured above when checking that the section is bounded.
uint32_t sh_size = base::checked_cast<uint32_t>(section->sh_size);
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment