Refactor task: Clean up range usage in Zucchini.

Ranges appear in 2 flavors (all values unsigned):

(A) [lo, hi):
  - Pros: No concern about overflow; easy to iterate and verify bounds.
  - Cons: Has invalid state lo > hi (we don't use backward ranges).
(B) {begin, size}:
  - Pros: "Canonical" since size >= 0, so backward ranges automatically impossible.
  - Cons: |begin + size| may overflow; checking for bounds is a bit more work.

The conversion is |lo = begin| and |hi = begin + size| and |size = hi - lo|. Currently Zucchini uses (B) to store values, and (A) to pass parameters. Code to support ranges are also rather scattered:
- BufferRegion in buffer_view.h stores (B) and has interface for (A), but has no validation.
- Validation code are found in algorithm.h:
  - RangeIsBounded(begin, size, bound): Test whether |[begin, begin + size)| fits in |[0, bound)|.
  - RangeCovers(begin, size, value): Tests whether |value| is in |[begin, begin + size)|.
  - InclusiveClamp(value, lo, hi): Saturation value to convert |value| to something in |[lo, hi]|.

Addition mess arises from mixture of {size_t, ptrdiff_t, offset_t (uint32_t)}:
- size_t arises from {std::vector<T>::size(), sizeof, offsetof}.
- offset_t semantically represents offset inside a file; its followed pretty well in code
- ptrdiff_t arises from iterator subtractions and size_t subtractions. Doesn't appear that often?

We assume size_t and ptrdiff_t are 64-bit. Zucchini follows these rules loosely:
- offset_t -> size_t: Implicit.
- size_t -> offset_t:
  - If known-good (e.g., sizeof) then use static_cast<offset_t>.
  - If must-be-good-or-else-fatal then use base::checked_cast<offset_t>.

To avoid undue delay in progress, I propose we do just the necessary work to get fuzzing to consistently work, and worry about aesthetics later.

*** Main Plan ***

(1) Identify range-related checks needed for patch validation, then add new functions + tests.
  - E.g.: Need function to see whether a region is inside another region (https://chromium-review.googlesource.com/1028836).

(2) Go through code (focusing on Zucchini-apply), at data read sites (PatchReader first; Disassemblers will need this but later), label data obtained from raw read as "unsafe_*", followed by "gentle failure" bounds check, then pass the value (but add comments on caveats and things not checked yet). Example:

  uint32_t unsafe_start = 0;
  uint32_t unsafe_size = 0;
  if (!src.GetValue<uint32_t>(&unsafe_start) ||
      !src.GetValue<uint32_t>(&unsafe_size)) {
    return false;  // Fail.
  }
  if (!RangeIsInRange(unsafe_start, unsafe_size, image_start, image_size))
    return false;
  // Caveat: |unsafe_start| may need to be 4-byte aligned, but that gets checked later.
  good_lo_hi_ = {unsafe_start, unsafe_start + unsafe_size};
  return true;

(3) Audit use of base::checked_cast or base::CheckedNumeric: They should not be used at input site, but only downstream where there is more trust, to catch "impossible" cases.

*** Aesthetics ***

These are non-goals for launch, but nice-to-have:
- Replace BufferRegion with stand-alone Region or Range data type that handles low-level checks.
- Avoid storing size_t as much as possible: They take up space (usually 64-bits) and we seldom need these. May need to define custom size type(s) to capture semantics better.
 

Another assumption that may break is that blocks in equivalence map do not "cut across the body of a reference" (AKA "a reference body does not straddle boundaries of equivalence blocks"). We don't need this to be a dedicated check to be conducted ASAP, but can be checked as more reference get encountered. Actually some checks already exist, so maybe it suffices to just make sure that these checks suffice.

This relates to bug 835341.

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/ce5642400b37f5ff2b0a1213522f984bca8a080a

commit ce5642400b37f5ff2b0a1213522f984bca8a080a
Author: Calder Kitagawa <ckitagawa@google.com>
Date: Fri Apr 27 22:37:45 2018

[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}
[modify] https://crrev.com/ce5642400b37f5ff2b0a1213522f984bca8a080a/components/zucchini/patch_read_write_unittest.cc
[modify] https://crrev.com/ce5642400b37f5ff2b0a1213522f984bca8a080a/components/zucchini/patch_reader.cc
[modify] https://crrev.com/ce5642400b37f5ff2b0a1213522f984bca8a080a/components/zucchini/patch_reader.h
[modify] https://crrev.com/ce5642400b37f5ff2b0a1213522f984bca8a080a/components/zucchini/zucchini_apply.cc

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/ebf95f0d9797ccf86cd59d43dec1ee58cd703c41

commit ebf95f0d9797ccf86cd59d43dec1ee58cd703c41
Author: Samuel Huang <huangs@chromium.org>
Date: Sun Apr 29 03:36:08 2018

[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}
[modify] https://crrev.com/ebf95f0d9797ccf86cd59d43dec1ee58cd703c41/components/zucchini/patch_reader.cc

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/63f65c0a8b295b925c1dc92c7bed7625e7d99d46

commit 63f65c0a8b295b925c1dc92c7bed7625e7d99d46
Author: Calder Kitagawa <ckitagawa@google.com>
Date: Wed May 02 15:27:28 2018

[Zucchini]: Stricter patch read

This enforces a stricter patch read in a few ways:
- Raw Deltas of 0 are forbidden. These are unused values which have no
  meaning and should not appear.
- Extra Data length must be equal to the size of the patch element
  minus the total length of equivalences.
- Element match headers must have regions of nonzero length.

This change also annotates a number of "unsafe" values read from the
patch that could be sources of error later if unchecked. It also adds
caveats about under what conditions emitted values are considered to
be safe/unsafe.

Bug: 837096
Change-Id: I1b7614d92b85c0a1546d8dccb7d371d28b2a4cd3
Reviewed-on: https://chromium-review.googlesource.com/1037186
Commit-Queue: Calder Kitagawa <ckitagawa@google.com>
Reviewed-by: Samuel Huang <huangs@chromium.org>
Reviewed-by: Greg Thompson <grt@chromium.org>
Cr-Commit-Position: refs/heads/master@{#555396}
[modify] https://crrev.com/63f65c0a8b295b925c1dc92c7bed7625e7d99d46/components/zucchini/patch_read_write_unittest.cc
[modify] https://crrev.com/63f65c0a8b295b925c1dc92c7bed7625e7d99d46/components/zucchini/patch_reader.cc
[modify] https://crrev.com/63f65c0a8b295b925c1dc92c7bed7625e7d99d46/components/zucchini/patch_reader.h

As of landing https://chromium-review.googlesource.com/1037186

Items (1), (2) and (3) are finished for patch_reader. Since DEX will likely be refactored when the additional reference types are added only the Win32 disassembler is left for (2). patch_reader was the primary concern for security review as fuzzing found a number of issues.

Aesthetics will come in followup work.

Reassigning to huangs@ as my internship is ending. Reducing priority since the bulk of mitigation are in place.