Fix an XSS in Server Islands. #11508
Conversation
Discussed with @FredKSchott that this is OK to disclose since Server Islands are still experimental. It's generally not safe to use `JSON.stringify` to interpolate potentially attacker controlled data into `<script>` tags as JSON doesn't escape `<>"'` and so one can use it to break out of the script tag and e.g. make a new one with controlled content. See https://pragmaticwebsecurity.com/articles/spasecurity/json-stringify-xss
🦋 Changeset detectedLatest commit: 81612ed The changes in this PR will be included in the next version bump. Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
Wonderful, thank you very much. |
|
Thansk! Unfortunately this doesn't work, because it escapes the quotes too, leading to this in the source: let componentId = "Island";
let componentExport = "default";
let script = document.querySelector('script[data-island-id="ecac82eb-69f6-4e0a-896c-3ddde2c01f79"]');
let data = {
componentExport,
props: {},
slots: {},
};We'll need a different approach. I think it's only the actual props that could be vulnerable, because the component names aren't user-generated, but it is probably a legitimate issue. |
|
I'll fix the encoding of quotes |
|
Reproduced the issue though: |
|
I think we would be safe to just escape function safeJsonStringify(obj: any) {
return JSON.stringify(obj).replaceAll('</script>', '<\\/script>');
} |
|
@ascorbic You can circumvent that with all kinds of tricks. I can't for the life of me find a good npm package to do this. But it might exist. Added manual encoding for now |
|
That looks good. I just tried it and it does the job. Thanks for your help on this. |
|
This was the best I could find: https://github.com/mathiasbynens/jsesc?tab=readme-ov-file#isscriptcontext |
|
Of course all of these will be fixed when we start encrypting the props as planned |
|
Thanks! Patch should go out in a minute. |
Discussed with @FredKSchott that this is OK to disclose since Server Islands are still experimental.
It's generally not safe to use
JSON.stringifyto interpolate potentially attacker controlled data into<script>tags as JSON doesn't escape<>"'and so one can use it to break out of the script tag and e.g. make a new one with controlled content.See https://pragmaticwebsecurity.com/articles/spasecurity/json-stringify-xss
Changes
No behavior change expected. All tests should still pass unless they trigger the encoding change.
Testing
Not at all. I couldn't get tests to run.