fix(security): break residual taint flows into ffmpeg args and asset URLs (round 2) - #55
Merged
Merged
Conversation
…URLs Round 2 for the 9 alerts CodeQL kept open after PR #54: - py/command-line-injection x6: build ffmpeg output filenames from store-backed frame.id/script.id instead of request parameters, and coerce offset_ms to int before it enters the adelay filter string. Runtime values are identical (ids matched by lookup), so behavior is unchanged; the request-parameter taint chain is simply broken. - js/xss-through-dom x3: strip HTML metacharacters from pass-through http/blob URLs in getAssetUrl (no-op for well-formed URLs).
|
|
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.
Follow-up to #54. After merge, CodeQL kept 9 alerts open because its taint analysis does not recognize the round-1 guards as barriers for these specific queries. This PR breaks the residual flows with zero behavior change.
Root cause (from SARIF code flows of analysis 1599605142/1599609579)
frame_id/script_id/offset_msinterpolated into ffmpeg output filenames / adelay filter strings._validate_safe_id()(regex allow-list) and_safe_resolve_path()(startswith guard) are barriers for path-injection but not for the command-line-injection query.getAssetUrl's http/blob pass-through branch (return path); the protocol allow-list is not a recognized barrier.Fix (no behavior change)
frame.id/script.id(identical values — matched by lookup) instead of request params;offset_ms = int(offset_ms)before entering the filter stringpath.replace(/[<>\"'\]/g, "")` on the pass-through branch — recognized metachar sanitizer, no-op for well-formed URLsVerification