Fix path traversal in xir_util dump_bin (CWE-22)#3
Draft
xingy16-amd wants to merge 1 commit into
Draft
Conversation
dump_subgraph1() built output file names by concatenating dirname with
the subgraph name and reg_id keys taken directly from the deserialized
.xmodel protobuf, without sanitization. A crafted .xmodel whose name /
reg_id contains "../" sequences (or an absolute path) could make
xir_util dump_bin write attacker-controlled data outside the intended
output directory, allowing arbitrary file overwrite.
Sanitize the file component in mk_file_name() with one unified rule on
every platform: replace path separators ('/', '\'), the NTFS stream
separator (':') and the characters Windows forbids (<>"|?*) with '_', so
every dumped file stays inside dirname. On Windows, reserved device names
(CON, NUL, COM1..9, LPT1..9, ...) are additionally prefixed.
Co-authored-by: Cursor <[email protected]>
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
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.
Summary
dump_subgraph1()intools/cmd_dump_code.cppbuilt output filenames byconcatenating
dirnamewith the subgraph name (sg->get_name()) andreg_id_to_parameter_valuekeys taken directly from the deserialized.xmodelprotobuf, with no sanitization..xmodelwhose subgraph name / reg_id contains../sequences(or an absolute path) makes
xir_util dump_bin evil.xmodel ./out/writeattacker-controlled bytes outside
./out/— arbitrary file write/overwrite(CWE-22).
mk_file_name()with one unified ruleon every platform: replace path separators (
/,\), the NTFS streamseparator (
:) and the characters Windows forbids (<>"|?*) with_, soevery dumped file stays inside
dirname. On Windows, reserved device names(
CON,NUL,COM1..9,LPT1..9, ...) are additionally prefixed./, soflattening (rather than rejecting) keeps valid models working.
Test plan
xir_util dump_binon a normal.xmodel;confirm
.mc/.ac/.binfiles are still produced under the output dir..xmodelwhose subgraph name contains../../tmp/evilandconfirm the dumped file stays inside the output dir (separators flattened
to
_) instead of escaping it.Made with Cursor