diff options
author | Samuel Huang <huangs@chromium.org> | 2018-12-31 19:59:22 +0000 |
---|---|---|
committer | Copybara-Service <copybara-worker@google.com> | 2021-07-25 20:42:02 -0700 |
commit | 431d119dafdcf886dbf161ba66cfe17100159621 (patch) | |
tree | dde64b243be6559c2d5df153d3cc088982ac8bea /type_win_pe.h | |
parent | fb345573ac09d9e282569d01fceadda5bb570bc4 (diff) | |
download | zucchini-431d119dafdcf886dbf161ba66cfe17100159621.tar.gz |
[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
Diffstat (limited to 'type_win_pe.h')
-rw-r--r-- | type_win_pe.h | 3 |
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 */ }; |