Skip to content

Fix SVG URL sanitization bypass#21440

Closed
metsw24-max wants to merge 1 commit into
emberjs:mainfrom
metsw24-max:svg-url-sanitization-bypass
Closed

Fix SVG URL sanitization bypass#21440
metsw24-max wants to merge 1 commit into
emberjs:mainfrom
metsw24-max:svg-url-sanitization-bypass

Conversation

@metsw24-max

Copy link
Copy Markdown

Fix a namespace-dependent URL sanitization bypass affecting SVG anchor elements.

requiresSanitization() previously compared tagName directly against uppercase allow/block lists. While HTML elements report uppercase tag names (e.g. A), SVG elements preserve their authored case (e.g. a), causing sanitization checks to be skipped for SVG anchors.

This change normalizes tag names to uppercase before evaluating URL sanitization requirements, making the behavior consistent across namespaces and matching the normalization already performed by sanitizeAttributeValue().

Changes

  • Normalize tagName to uppercase in requiresSanitization().
  • Ensure SVG elements are evaluated against the same URL sanitization rules as their HTML counterparts.
  • Add an integration test covering <svg><a href="{{...}}"> with both unsafe and valid URLs.
  • Verify sanitization remains unchanged for existing HTML elements.

Before

<svg><a href="javascript:foo()"></a></svg>

could bypass the URL sanitization gate because the SVG anchor's tag name was reported as a instead of A.

After

SVG anchor elements are correctly identified as requiring URL sanitization, and unsafe URLs are rewritten as expected:

<svg><a href="unsafe:javascript:foo()"></a></svg>

Testing

Added an integration test that verifies:

  • javascript: URLs in SVG anchor href attributes are sanitized.
  • Valid HTTP URLs continue to work normally.
  • Sanitization behavior remains stable across rerenders.

@NullVoxPopuli

Copy link
Copy Markdown
Contributor

thanks for your submission, @metsw24-max !

however, this looks like a duplicate of #21442 ?

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.

2 participants