aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorSamuel Huang <huangs@chromium.org>2018-04-29 03:36:08 +0000
committerEdward Lesmes <ehmaldonado@google.com>2021-07-23 22:16:20 +0000
commit93ffc913ec7d369818a109b9ca6de7adb278e163 (patch)
tree231de7095cb989d4220beb4a5d9f13e931a74ddc
parent673bf5912068218539f524b6dfe74dfb38fe67b8 (diff)
downloadzucchini-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.cc26
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;
}