Skip to content

sanitize urls: svg/xlink href, formaction, and data: on iframe/object#21442

Open
rootvector2 wants to merge 5 commits into
emberjs:mainfrom
rootvector2:svg-anchor-url-sanitize
Open

sanitize urls: svg/xlink href, formaction, and data: on iframe/object#21442
rootvector2 wants to merge 5 commits into
emberjs:mainfrom
rootvector2:svg-anchor-url-sanitize

Conversation

@rootvector2
Copy link
Copy Markdown

@rootvector2 rootvector2 commented Jun 1, 2026

Consolidates the open URL-sanitizer fixes into one PR (per review). Each change closes a way an attacker-controlled value can reach a dangerous protocol that the sanitizer currently misses.

  • SVG anchors (case sensitivity). checkURI/checkDataURI matched element.tagName against the uppercase badTags list, but SVG tagNames come through lowercase (a), so <a href={{value}}> inside an <svg> skipped the javascript:/vbscript: check. Normalization now happens inside checkURI/checkDataURI so it is single-sourced. Also added xlink:href to the attribute list, since that is the SVG href alias and was not covered at all.
  • formaction (sanitize javascript: urls in formaction on button and input #21438). button[formaction] and input[formaction] submit to their URL, so a javascript: value there executes. Added BUTTON/INPUT to badTags and formaction to the attribute list.
  • data: on iframe[src] and object[data] (mark data: urls as unsafe on iframe[src] and object[data] #21441). A data: URL in these loads as a nested document and can run script. Added a data:-protocol check for iframe/object.
  • tab/newline stripping (strip ascii tab/newline from urls before protocol check #21433). Browsers strip ASCII tab/newline/CR from a url before navigating, so java\nscript: runs as javascript:. The fastboot url.parse path kept those chars and reported a null protocol, slipping past the check. Strip them there to match the WHATWG URL parser used on the browser path.

Tests added for each case except the tab/newline strip, which only runs on the fastboot url.parse path (the browser URL parser already strips those chars, so no integration path exercises it).

Reproductions on limber for each case are in the comment below.

Comment thread packages/@glimmer/runtime/lib/dom/sanitized-values.ts Outdated
Comment thread packages/@glimmer/runtime/lib/dom/sanitized-values.ts Outdated
simplify checkURI to inline the tag normalization, and bring in the
other open sanitization gaps so they live behind the same tag/attribute
matching:
- formaction on button/input
- data: protocol on iframe[src] and object[data]
- strip ascii tab/newline/cr before the fastboot url protocol check
@rootvector2
Copy link
Copy Markdown
Author

Reproductions on limber, one per case. Each binds the attacker value to the attribute so it goes through the sanitizer. On a released Glimmer the dangerous protocol survives in the DOM; with this PR it comes back as unsafe:.

The tab/newline strip from #21433 doesn't have a browser repro - that path only runs under fastboot's url.parse. In the browser the WHATWG URL parser already strips \t\n\r, so java\nscript: is caught there regardless.

@rootvector2 rootvector2 changed the title sanitize urls on svg anchor href and xlink:href sanitize urls: svg/xlink href, formaction, and data: on iframe/object Jun 1, 2026
@rootvector2
Copy link
Copy Markdown
Author

Folded the AREA case in here too (it's the same badTags selection, an <area href> in an image map navigates on click just like an <a>). Repro, same shape as the others:

Click the box and on a released Glimmer it fires; with this PR the href comes back as unsafe:.


function checkURI(tagName: Nullable<string>, attribute: string): boolean {
return (tagName === null || has(badTags, tagName)) && has(badAttributes, attribute);
// SVG element tagNames are lowercase (e.g. `a`), so they never match the
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nitpick for the comment wording: SVG tagNames are case-preserved (e.g. linearGradient, clipPath, foreignObject),

– just comment wording, does not affect logic/correctness, as a just happens to be lowercase, and a is the only SVG element in badTags

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good catch, reworded it. a is the only one in the list and it happens to be lowercase, but the comment shouldn't imply that's true of svg tagNames in general.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants