[Zucchini] Validate equivalences on load

A follow-up to
https://chromium-review.googlesource.com/c/chromium/src/+/1028575

This moves patch_apply logic to check bounds of an equivalences from
the call site of GetNext() to an internal function in the patch_reader.
This means the equivalence consumer can use the equivalences without
checking anything to do with bounds.

I have manually tested that this doesn't appear to break any existing
valid patches and it appears to catch all the same errors change
1028575 fixed so I can safely reverse that change.

BUG: 837096

Change-Id: I84ccd9e1493f32d16eace4dd8e67586f559220d3
Reviewed-on: https://chromium-review.googlesource.com/1028836
Commit-Queue: Calder Kitagawa <ckitagawa@google.com>
Reviewed-by: Samuel Huang <huangs@chromium.org>
Cr-Commit-Position: refs/heads/master@{#554536}
NOKEYCHECK=True
GitOrigin-RevId: ce5642400b37f5ff2b0a1213522f984bca8a080a

1 files changed, 42 insertions, 2 deletions

diff --git a/patch_reader.cc b/patch_reader.cc
index 8215405..90f35f4 100644
--- a/patch_reader.cc
+++ b/patch_reader.cc
@@ -13,6 +13,19 @@
 
 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) {
@@ -217,7 +230,7 @@ PatchElementReader::~PatchElementReader() = default;
 
 bool PatchElementReader::Initialize(BufferSource* source) {
   bool ok = patch::ParseElementMatch(source, &element_match_) &&
-            equivalences_.Initialize(source) &&
+            equivalences_.Initialize(source) && ValidateEquivalences() &&
             extra_data_.Initialize(source) && raw_delta_.Initialize(source) &&
             reference_delta_.Initialize(source);
   if (!ok)
@@ -240,7 +253,7 @@ bool PatchElementReader::Initialize(BufferSource* source) {
     }
     auto insert_result = extra_targets_.insert({pool_tag, {}});
     if (!insert_result.second) {  // Element already present.
-      LOG(ERROR) << "Multiple ExtraTargetList found for the same pool_tag";
+      LOG(ERROR) << "Multiple ExtraTargetList found for the same pool_tag.";
       return false;
     }
     if (!insert_result.first->second.Initialize(source))
@@ -249,6 +262,33 @@ bool PatchElementReader::Initialize(BufferSource* source) {
   return true;
 }
 
+bool PatchElementReader::ValidateEquivalences() {
+  EquivalenceSource equivalences_copy = equivalences_;
+
+  BufferRegion old_region = element_match_.old_element.region();
+  BufferRegion new_region = element_match_.new_element.region();
+
+  // 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)) {
+      LOG(ERROR) << "Out of bounds equivalence detected.";
+      return false;
+    }
+    if (prev_dst_end > equivalence->dst_end()) {
+      LOG(ERROR) << "Out of order equivalence detected.";
+      return false;
+    }
+    prev_dst_end = equivalence->dst_end();
+  }
+  return true;
+}
+
 /******** EnsemblePatchReader ********/
 
 base::Optional<EnsemblePatchReader> EnsemblePatchReader::Create(