[Zucchini] Fix under/overfow bug in DEX

This bug was found by the fuzzer. If a large int32 value is present for
a RelCode32 the result of mapping the location to its target results in
integer overflow or underflow as found by UBSAN.

In the particular example found by the fuzzer a value of 1292632068 is
read from the image. The result of |(1292632067 - 1) * kInstrUnitSize|,
where |kInstrUnitSize = 2| results in an overflow. This is only
possible for RelCode32 so we only need the fix there. The solution is
to check for overflow and if it occurs just to skip the reference. In
a regular DEX file these should be very rare if ever present.

I've tested the updated version on a subset of the corpus with no ill
effects.

Bug: 862095
Change-Id: Ifedeeaf1ae7e72a147421ecb917ec1751f4bb8d4
Reviewed-on: https://chromium-review.googlesource.com/1131225
Commit-Queue: Calder Kitagawa <ckitagawa@chromium.org>
Reviewed-by: Samuel Huang <huangs@chromium.org>
Cr-Commit-Position: refs/heads/master@{#574160}
NOKEYCHECK=True
GitOrigin-RevId: c87921042aae8f5fa594f96b33635ef76cfd1028

1 files changed, 10 insertions, 5 deletions

diff --git a/disassembler_dex.cc b/disassembler_dex.cc
index e94dbc9..9d2ee3a 100644
--- a/disassembler_dex.cc
+++ b/disassembler_dex.cc
@@ -1373,12 +1373,17 @@ std::unique_ptr<ReferenceReader> DisassemblerDex::MakeReadCodeToRelCode32(
   auto mapper = base::BindRepeating(
       [](DisassemblerDex* dis, offset_t location) {
         // Address is relative to the current instruction, which begins 1 unit
-        // before |location|. This needs to be subtracted out.
-        int32_t unsafe_delta = dis->image_.read<int32_t>(location);
-        offset_t unsafe_target = static_cast<offset_t>(
-            location + (unsafe_delta - 1) * kInstrUnitSize);
+        // before |location|. This needs to be subtracted out. Use int64_t to
+        // avoid underflow and overflow.
+        int64_t unsafe_delta = dis->image_.read<int32_t>(location);
+        int64_t unsafe_target = location + (unsafe_delta - 1) * kInstrUnitSize;
+
         // TODO(huangs): Check that |unsafe_target| stays within code item.
-        return unsafe_target;
+        offset_t checked_unsafe_target =
+            static_cast<offset_t>(base::CheckedNumeric<offset_t>(unsafe_target)
+                                      .ValueOrDefault(kInvalidOffset));
+        return checked_unsafe_target < kOffsetBound ? checked_unsafe_target
+                                                    : kInvalidOffset;
       },
       base::Unretained(this));
   return std::make_unique<InstructionReferenceReader>(