JavaScript: Add setHTMLUnsafe and parseHTMLUnsafe as XSS sinks#21648
Open
sunnyeo wants to merge 1 commit intogithub:mainfrom
Open
JavaScript: Add setHTMLUnsafe and parseHTMLUnsafe as XSS sinks#21648sunnyeo wants to merge 1 commit intogithub:mainfrom
sunnyeo wants to merge 1 commit intogithub:mainfrom
Conversation
Add support for two new HTML Sanitizer API methods that interpret arguments as HTML without sanitization: - `Element.setHTMLUnsafe(html)`: Added to `interpretsArgumentsAsHtml` in DOM.qll, following the same pattern as `insertAdjacentHTML` and `document.write`. Receiver validation via `isDomNode` is inherited from `DomMethodCallNode`. - `Document.parseHTMLUnsafe(html)`: Added to `HtmlParserSink` in DomBasedXssCustomizations.qll, following the same `GlobalVarRefNode` pattern as `DOMParser.parseFromString`. This is a static method on the `Document` class. Both methods are part of the HTML Sanitizer API and are shipping in browsers (Chrome 124+, Firefox 148+). Unlike their safe counterparts (`setHTML`, `parseHTML`), these methods do not sanitize input and are therefore XSS sinks. References: - https://developer.mozilla.org/en-US/docs/Web/API/Element/setHTMLUnsafe - https://developer.mozilla.org/en-US/docs/Web/API/Document/parseHTMLUnsafe_static
Contributor
There was a problem hiding this comment.
Pull request overview
This PR updates the JavaScript DOM-based XSS dataflow modeling to recognize two HTML Sanitizer API “unsafe” methods as sinks, ensuring flows into these APIs are reported as potential XSS.
Changes:
- Add
Element.setHTMLUnsafe(html)(argument 0) as an HTML-interpreting DOM sink viainterpretsArgumentsAsHtml. - Add
Document.parseHTMLUnsafe(html)(argument 0) as an HTML parser sink inHtmlParserSink.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| javascript/ql/lib/semmle/javascript/security/dataflow/DomBasedXssCustomizations.qll | Extends HtmlParserSink to include Document.parseHTMLUnsafe argument 0. |
| javascript/ql/lib/semmle/javascript/security/dataflow/DOM.qll | Extends DOM method-call HTML interpretation to include setHTMLUnsafe argument 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
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 two new HTML Sanitizer API methods as DOM-based XSS sinks:
Element.setHTMLUnsafe(html)— Added tointerpretsArgumentsAsHtmlinDOM.qll, following the same pattern asinsertAdjacentHTMLanddocument.write. Receiver validation viaisDomNodeis inherited fromDomMethodCallNode.Document.parseHTMLUnsafe(html)— Added toHtmlParserSinkinDomBasedXssCustomizations.qll, following the sameGlobalVarRefNodepattern asDOMParser.parseFromString. This is a static method on theDocumentclass.Both methods are part of the HTML Sanitizer API and are shipping in browsers (Chrome 124+, Firefox 148+). Unlike their safe counterparts (
setHTML,parseHTML), these methods do not sanitize input and are therefore XSS sinks.Changes
DOM.qllsetHTMLUnsafe(arg 0) tointerpretsArgumentsAsHtmlDomBasedXssCustomizations.qllDocument.parseHTMLUnsafe(arg 0) toHtmlParserSinkDesign decisions
setHTMLUnsafeininterpretsArgumentsAsHtml: This is a DOM element instance method (likeinsertAdjacentHTML,write), soDomMethodCallNodehandles receiver validation viaisDomNode.parseHTMLUnsafeinHtmlParserSink: This is a static method on theDocumentclass (Document.parseHTMLUnsafe(html)). TheDocumentclass reference is not recognized byisDomNode(which tracks DOM node instances, not class constructors). We useGlobalVarRefNodeto match theDocumentreceiver, following the same pattern asDOMParser.parseFromString.ShadowRoot.setHTMLUnsafe:ShadowRootis not currently tracked bydomValueRef(), soisDomNodedoes not cover it. Adding ShadowRoot support todomValueRef()is a separate enhancement tracked independently.References