diff options
author | Samuel Huang <huangs@chromium.org> | 2018-04-29 03:36:08 +0000 |
---|---|---|
committer | Edward Lesmes <ehmaldonado@google.com> | 2021-07-23 22:16:20 +0000 |
commit | 93ffc913ec7d369818a109b9ca6de7adb278e163 (patch) | |
tree | 231de7095cb989d4220beb4a5d9f13e931a74ddc | |
parent | 673bf5912068218539f524b6dfe74dfb38fe67b8 (diff) | |
download | zucchini-93ffc913ec7d369818a109b9ca6de7adb278e163.tar.gz |
[Zucchini] Fix range error for equivalence validation.
CL https://chromium-review.googlesource.com/1028836 adds patch
validation to ensure equivalence blocks for element pairs would not
span outside the regions for respective elements. The check assumes that
equivalences use absolute offsets for entire "old" or "new" archives.
This was mistaken (and missed in review), since equivalences in fact use
"local" offsets relative to the start of each element. As a result,
ensemble patching for Zucchini-apply would mistakenly trigger the check,
leading to patch failure.
This CL fixes the check. This removes the need for RangeIsInRegion()
(now removed), since RangeIsBounded() suffices.
Bug: 837096
TBR=grt@chromium.org
Change-Id: I2b1e575c4a44f112adebaf1487a41f12cf23f570
Reviewed-on: https://chromium-review.googlesource.com/1034274
Reviewed-by: Samuel Huang <huangs@chromium.org>
Commit-Queue: Samuel Huang <huangs@chromium.org>
Cr-Commit-Position: refs/heads/master@{#554664}
NOKEYCHECK=True
GitOrigin-RevId: ebf95f0d9797ccf86cd59d43dec1ee58cd703c41
-rw-r--r-- | patch_reader.cc | 26 |
1 files changed, 7 insertions, 19 deletions
diff --git a/patch_reader.cc b/patch_reader.cc index 90f35f4..7e0b815 100644 --- a/patch_reader.cc +++ b/patch_reader.cc @@ -9,23 +9,11 @@ #include <utility> #include "base/numerics/safe_conversions.h" +#include "components/zucchini/algorithm.h" #include "components/zucchini/crc32.h" namespace zucchini { -namespace { - -bool RangeIsInRegion(offset_t begin, uint32_t size, BufferRegion region) { - // Return false if |begin + size > kOffsetBound|, without overflowing. - if (begin >= kOffsetBound || kOffsetBound - begin < size) - return false; - offset_t end = begin + size; - // Return false if |[begin, end]| leaks outside |region|. - return begin >= region.lo() && end <= region.hi(); -} - -} // namespace - namespace patch { bool ParseElementMatch(BufferSource* source, ElementMatch* element_match) { @@ -265,18 +253,18 @@ bool PatchElementReader::Initialize(BufferSource* source) { bool PatchElementReader::ValidateEquivalences() { EquivalenceSource equivalences_copy = equivalences_; - BufferRegion old_region = element_match_.old_element.region(); - BufferRegion new_region = element_match_.new_element.region(); + const size_t old_region_size = element_match_.old_element.size; + const size_t new_region_size = element_match_.new_element.size; // Validate that each |equivalence| falls within the bounds of the // |element_match_| and are in order. offset_t prev_dst_end = 0; for (auto equivalence = equivalences_copy.GetNext(); equivalence.has_value(); equivalence = equivalences_copy.GetNext()) { - if (!RangeIsInRegion(equivalence->src_offset, equivalence->length, - old_region) || - !RangeIsInRegion(equivalence->dst_offset, equivalence->length, - new_region)) { + if (!RangeIsBounded(equivalence->src_offset, equivalence->length, + old_region_size) || + !RangeIsBounded(equivalence->dst_offset, equivalence->length, + new_region_size)) { LOG(ERROR) << "Out of bounds equivalence detected."; return false; } |