[Zucchini] DisassemblerWin32: Fix rogue |reloc_region_| usage.

ClusterFuzz found a 32-bit PE file that triggers DCHECK(). The cause (in
DisassemblerWin32):
* ParseAndStoreRelocBlocks() reads malformed reloc region and rejects
  it, but writes bad result to |reloc_region_| (with kInvalidOffset).
* MakeReadRelocs() passes the invalid |reloc_region_|, which causes
  uint32_t overflow (for 32-bit size_t), and triggering DCHECK().
* MakeWriteRelocs() was not reached, but has similar problem.

Main fix:
* Initialize |reloc_region_| to invalid state.
* In ParseAndStoreRelocBlocks():
  * On first run: Only write |reloc_region_| on success.
  * On later runs: Return validity of |reloc_region_|.
* In MakeReadRelocs() and MakeWriteRelocs(): Already calls
  ParseAndStoreRelocBlocks(). Change this to return vacuous
  ReferenceReader / ReferenceWriter on failure, i.e., if
  |reloc_region_| is invalid.
  * Need to add EmptyReferenceWriter for this.

DisassemblerWin32 is also too lax in dealing with malformed input, so
this CL also implments more stringent checks:
* Disallow ImageSectionHeader (+ ImageOptionalHeader) from overlapping
  with ImageDOSHeader.
* Apply more stringent checks for |size_of_optional_header| and
  |number_of_rva_and_sizes|: Consistency and bounds.

Bug: 917042
Change-Id: I661303d49a57025a877585eb175f369d067652a4
Reviewed-on: https://chromium-review.googlesource.com/c/1390766
Reviewed-by: Samuel Huang <huangs@chromium.org>
Reviewed-by: Etienne Pierre-Doray <etiennep@chromium.org>
Commit-Queue: Etienne Pierre-Doray <etiennep@chromium.org>
Cr-Commit-Position: refs/heads/master@{#619316}
NOKEYCHECK=True
GitOrigin-RevId: d46be686cb9e764befdd39aa168dde62b5314e37

1 files changed, 3 insertions, 0 deletions

diff --git a/type_win_pe.h b/type_win_pe.h
index d385ca7..56996fe 100644
--- a/type_win_pe.h
+++ b/type_win_pe.h
@@ -120,6 +120,9 @@ struct ImageOptionalHeader {
   uint32_t size_of_heap_commit;
   uint32_t loader_flags;
   uint32_t number_of_rva_and_sizes;
+
+  // The number of elements is actually |number_of_rva_and_sizes|, so accesses
+  // to |data_directory| should be checked against the bound.
   ImageDataDirectory data_directory[kImageNumberOfDirectoryEntries];  // 0x60
   /* 0xE0 */
 };