From 673bf5912068218539f524b6dfe74dfb38fe67b8 Mon Sep 17 00:00:00 2001 From: Calder Kitagawa Date: Fri, 27 Apr 2018 22:37:45 +0000 Subject: [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 Reviewed-by: Samuel Huang Cr-Commit-Position: refs/heads/master@{#554536} NOKEYCHECK=True GitOrigin-RevId: ce5642400b37f5ff2b0a1213522f984bca8a080a --- zucchini_apply.cc | 13 ------------- 1 file changed, 13 deletions(-) (limited to 'zucchini_apply.cc') diff --git a/zucchini_apply.cc b/zucchini_apply.cc index 63b8ce0..b67bf63 100644 --- a/zucchini_apply.cc +++ b/zucchini_apply.cc @@ -26,13 +26,6 @@ bool ApplyEquivalenceAndExtraData(ConstBufferView old_image, for (auto equivalence = equiv_source.GetNext(); equivalence.has_value(); equivalence = equiv_source.GetNext()) { - // TODO(ckitagawa): Ensure guards don't overflow. Move these validation - // check to the patch reader. - // Validate that the |equivalence| is within the |new_image|. - if (equivalence->dst_end() > new_image.size()) { - LOG(ERROR) << "Out of bounds equivalence"; - return false; - } MutableBufferView::iterator next_dst_it = new_image.begin() + equivalence->dst_offset; CHECK(next_dst_it >= dst_it); @@ -47,12 +40,6 @@ bool ApplyEquivalenceAndExtraData(ConstBufferView old_image, // copy should be valid. dst_it = std::copy(extra_data->begin(), extra_data->end(), dst_it); CHECK_EQ(dst_it, next_dst_it); - - // Validate that the |equivalence| is within the |old_image|. - if (equivalence->src_end() > old_image.size()) { - LOG(ERROR) << "Out of bounds equivalence"; - return false; - } dst_it = std::copy_n(old_image.begin() + equivalence->src_offset, equivalence->length, dst_it); CHECK_EQ(dst_it, next_dst_it + equivalence->length); -- cgit v1.2.3