harden msf/pdb parsing against malformed radbin input#859
Open
momomuchu wants to merge 1 commit into
Open
Conversation
radbin crashed converting crafted PDB/MSF files. Four related guards: - msf_raw_stream_table_from_data: a zero MSF page_size survived the ClampTop (which only caps the ceiling) and reached CeilIntegerDiv as a divisor, causing a divide-by-zero. Reject page_size == 0 first. - msf_raw_stream_table_from_data: a stream_table_size below 4 made the 4-byte stream-count read go out of bounds and (directory_size - 4) underflow into a ~1G stream count. Require directory_size >= 4. - pdb_info_from_data: for unrecognized PDB info versions auth_guid was left null but still dereferenced when a hash table was present. Only copy it when it was actually set. - pdb_gsi_from_data: a malformed/out-of-range DBI stream number could hand this a null (or wrapped-huge-size) String8, which was then treated as a GsiHeader pointer. Reject a null pointer / out-of-range size before reading the header. Adds p2r_malformed_input_hardening (rdi_from_pdb tests): builds minimal MSF7 containers hitting each path and runs radbin against them. The test originally asserted radbin exits 0, which failed on Windows CI even on the unmodified good_baseline_sanity case (radbin returned a benign non-zero exit code there while still converting cleanly). Exit code is not a reliable cross-platform crash signal, so the test now captures radbin's stdout+stderr and checks for its crash handler's "...The process is terminating." signature instead, which the linux and windows signal/exception handlers both print identically.
momomuchu
force-pushed
the
fix/msf-pdb-malformed-input-hardening
branch
from
3d175b7 to
1d1fc58
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #832, #833, #834 (#835 is a dup of #834).
Four malformed-input crashes in the MSF/PDB parsers, all reachable by running radbin on a crafted .pdb:
msf_raw_stream_table_from_data. Guardpage_size != 0before theCeilIntegerDiv.auth_guidunset, thenpdb_info_from_datawrites through it. Null-check before the copy.pdb_gsi_from_dataas a wrapped, huge-sized String8, so(GsiHeader *)data.strgets walked past the buffer. Reject a nullstr/ out-of-range size before treating the bytes as a header.While guarding
msf_raw_stream_table_from_datafor #832 I hit an adjacent one in the same function:stream_table_size < 4makesdirectory_size - 4underflow to ~1G (and reads the 4-byte stream count out of a shorter buffer), so I guarded that too withdirectory_size >= 4.Added a torture case per bug under
p2r_malformed_input_hardening. Each input crashes on master and exits cleanly with the fix; reverting any one guard brings its crash back. The normal path is unaffected (radbin --rdi radbinstill round-trips to a valid rdi).Built and verified on the ubuntu + clang path (the one the CI torture job runs). I didn't exercise the msvc build, but the guards are plain portable C with no platform-specific bits.