Add embedded JavaScript DOM XSS scanning - #54
Open
juangaitanv wants to merge 9 commits into
Open
Conversation
- Match the canonical frontend DOM-XSS rule by id instead of a tag heuristic, removing the ambiguity error path - Pass embedded-JS mode as an explicit parameter instead of deriving it from path inequality - Gate the inline-script range walk to files that need it - Drop the derivable has_embedded_dom_xss_rules field - Add Finding::has_tag and use it at all tag-check sites - Cut redundant allocations in promise-source tracking, descendant-sink walk, and fallback preference - Flatten parse_with_included_ranges error handling
juangaitanv
marked this pull request as ready for review
July 22, 2026 15:50
yhoztak
reviewed
Jul 24, 2026
yhoztak
reviewed
Jul 24, 2026
yhoztak
reviewed
Jul 24, 2026
yhoztak
reviewed
Jul 24, 2026
- Flag direct assignment flows (element.innerHTML = location.hash) by making assignment nodes actionable for the bare source-to-sink path, matching sources against the assigned value only - Scope anonymous callback taint to a positional closure identity so reused parameter names across callbacks cannot leak taint - Apply sanitizer evaluation to resolved promise expressions (Promise.resolve(DOMPurify.sanitize(...))) - Load the canonical DOM-XSS taint rule for html/django in file-rules mode, matching embedded-rules behavior, with dedup across languages
- Consolidate canonical DOM-XSS rule selection into Rules::frontend_dom_xss_rule_only, shared by embedded and file loaders - Fold the html/django check into load_file_dom_xss_taint_rule so both call sites are a single guarded push - Read assignment RHS from the AST right field instead of splitting on = - Defer node text and function context computation off the per-node hot path until a matching branch or sink needs them - Replace the keep-mask retain in prefer_precise_html_dom_xss with owned keys and a direct retain - Dedupe repeated test scaffolding in embedded DOM XSS and modes tests
Propagation could overwrite a variable's direct source record (e.g. userInput = urlParams.get(...) re-recorded from urlParams as URLSearchParams), shifting rule selection from the DOM-XSS rule to the CWE-116 encoding rule for the same flow. Skip propagation when the target already has a taint record; differential scan vs main restores CWE-79 attribution in xss_comprehensive_test.js with no lost findings.
leenk7991
reviewed
Jul 29, 2026
leenk7991
reviewed
Aug 2, 2026
leenk7991
reviewed
Aug 11, 2026
| } | ||
|
|
||
| let has_embedded_dom_xss_rules = !rules.embedded_dom_xss_rules.is_empty(); | ||
| if matches!(detected_language, "html" | "django") |
Member
There was a problem hiding this comment.
detect_language_from_path never returns "django"
| return None; | ||
| } | ||
| let javascript_rules_dir = format!("{}/javascript", base_rules_dir); | ||
| Rules::load_from_directory(&javascript_rules_dir).ok()?.frontend_dom_xss_rule_only() |
Member
There was a problem hiding this comment.
.ok()? here turns a missing/broken rules/javascript/ into a silent zero-coverage scan that still exits 0
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
3 participants
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
Add DOM XSS analysis for executable JavaScript embedded in HTML and Django templates.
The scanner now parses eligible inline
<script>bodies as JavaScript while preserving the original template path and line coordinates. It reuses the canonical frontend DOM XSS taint rules, tracks browser-controlled values such aslocation.hashand promise callback values fromfetch(...).then(...), and prefers precise taint findings over the broader HTML fallback finding.This closes a coverage gap where Sighthound simple analysis missed
document.writeandinnerHTMLflows inside HTML templates. External, empty, non-executable, and unknown script types remain excluded, safe sanitizer cases remain clean, and native JavaScript scanning behavior is unchanged.Validation covered the two held-out benchmark cases:
location.hashat line 15 reachesdocument.writeat line 20.fetch(*).thenat line 36 reachesinnerHTMLat line 42.The full Rust suite passed with 211 tests, the acceptance suite passed with 8 scenarios and 31 steps, and the release and HTML-only builds passed. Corpus comparison added only the two expected detection keys; measured overhead was 0.08 seconds for DjangoAt and 0.04 seconds for PyGoat.
Related issue
None.
Checklist
make cipasses locally (the same command CI runs — see CONTRIBUTING.md)make bootstraponce so pre-commit/pre-push hooks are active