-
Notifications
You must be signed in to change notification settings - Fork 47.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
React 18: bootstrapScriptContent escapes HTML so quotes can’t be used #23063
Comments
@sebmarkbage @acdlite Is the |
Yea it's probably not the correct escape. We should escape somehow though because It's a common security issue in many non-meta frameworks. cc @sophiebits knows the exact pattern needed. If we can't make it generic we should at least rename it "dangerous" and describe how to escape any JSON content someone might add there. |
See the update at the end of my post here: https://sophiebits.com/2012/08/03/preventing-xss-json.html. Basically replacing the It's security-safe (to my belief) but note that this technically can change the meaning of a script with a tagged template literal since the raw text is available, but I doubt that would come up in practice so it's probably fine. The only other ideas I have are having the script encoded in some fully out-of-band way like a base64 data URI or putting the whole script in a string and then eval-ing it (both of which are weird for CSP). |
@devknoll @timneutkens What does Next.js do to escape JSON? Presumably that has received enough exposure. |
For JSON you only need to escape / as \/ (many JSON encoders do this automatically). |
How about optionally accepting an object with {
bootstrapScriptContent: {
__dangerouslySetInnerHTML: 'window.__BOOTSTRAP_REACT_APP__("complex inline ssr data")`
}
} With react 17, I used serialize-javascript to serialize the ssr data without any problem. |
@sophiebits I found this little tidbit while reading the spec. w3c/html#1617 I'm not sure you can use that to inject new scripts but you can certainly cover up future scripts or content and break the page. I think it's probably ok if you also encode the s in |
This API only really promises to encode valid JS into HTML. It doesn't promise to turn JSON into JS.
Edit: Actually because of https://github.com/tc39/proposal-json-superset you probably don't need to escape it in modern browsers. Although many probably will keep doing it as a precaution. You also have to consider Escaping the s in |
I think we're just going to deprecate XHTML support therefore I don't think Additionally, some escape The reason we wouldn't escape them is because the clever hack doesn't work on these while it remains valid JS. |
@calebmer should be available in the next minor |
Fixed in 18.1.0. |
If you use the
bootstrapScriptContent
option ofrenderToPipeableStream()
to provide hydration data (as recommended in the <script> upgrade guide) withJSON.stringify()
it doesn’t work because React escapes HTML characters inbootstrapScriptContent
like quotes. I’ve worked around this by using backticks to deliniate strings.What’s the correct thing to do here?
react/packages/react-dom/src/server/ReactDOMServerFormatConfig.js
Lines 99 to 105 in cdb8a1d
The text was updated successfully, but these errors were encountered: