Skip to content

fix: sanitize id_prefix to prevent HTML comment injection in SSR output#17993

Open
mandreko wants to merge 2 commits intosveltejs:mainfrom
mandreko:id-prefix-html-comment-sanitization
Open

fix: sanitize id_prefix to prevent HTML comment injection in SSR output#17993
mandreko wants to merge 2 commits intosveltejs:mainfrom
mandreko:id-prefix-html-comment-sanitization

Conversation

@mandreko
Copy link
Copy Markdown

@mandreko mandreko commented Mar 23, 2026

Summary

  • Strips < and > characters from id_prefix before embedding it in hydration markers
  • Prevents malicious prefixes from breaking out of HTML comments and injecting arbitrary HTML into server-rendered output
  • Adds a test case (id-prefix-html-comment-sanitization) to cover this scenario

Test plan

  • Run existing SSR tests: pnpm test
  • Verify the new sample test id-prefix-html-comment-sanitization passes
  • Verify that a crafted id_prefix like -->injected<-- does not break SSR hydration markers

Strip `<` and `>` from id_prefix before embedding it in hydration markers
to prevent malicious prefixes from breaking out of HTML comments and
injecting arbitrary HTML into server-rendered output.
@changeset-bot
Copy link
Copy Markdown

changeset-bot bot commented Mar 23, 2026

⚠️ No Changeset found

Latest commit: ffdfe5d

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@dummdidumm
Copy link
Copy Markdown
Member

While it's technically correct that you could do HTML injection here, it's basically impossible to exploit. To do so you would have to provide a dynamic idPrefix to render coming from something that is controllable from the outside. Why would anyone do that? So I'm a bit torn whether or not we want to do this.

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