redfin
Review XSS threat from JSON.stringify
So I just read this article about XSS vulnerabilities in React SSR that result from using JSON.stringify to serialize data, and I immediately thought of y'all. I went through the current codebase's use of JSON.stringify, and I think that they are all safe, but it's been a while and I don't exactly understand all of the uses. It could definitely use another pair of eyes.
It's worth noting that the case I was most worried about was the reading out of late arrivals, but that helpfully uses a wrapper function to escape HTML entities (renderMiddleware.js#981). And the code that reads out the initial data seems to use express-state, which also handles safely encoding (renderMiddleware.js#938). So the two most worrisome cases are taken care of, but it'd still probably be good to get another pair of eyes looking at the code. Could someone else volunteer to do an audit?
One use I cannot for the life of me decipher is at renderMiddleware.js#945:
renderScriptsAsync([{
text: `${res.locals.state};rfBootstrap();`,
}], res);
I can't figure out where res.locals.state gets set, if ever. Probably harmless, but worth double-checking.
One use I cannot for the life of me decipher is at renderMiddleware.js#945:
@aickin IIRC that's part of express-state. The stuff that gets exposed with res.expose ends up there.
Also, if anybody wants to get rid of express-state, I'll thumbs-up that PR. We're not really using it much, and it ties react-server to express. It could be replaced by properly-escaped JSON.stringify. ;)
@roblg got it, then it should be safe.
The other one that scared me at first was in FragmentDataCache.jsx, but then I realized it gets sanitized since it's inside JSX.
@aickin great article & reminder...I think this is still a good review to do. The serialize-javascript library suggested in the article looks like a great option. I could see issues with these areas:
- https://github.com/redfin/react-server/blob/master/packages/react-server/core/ReactServerAgent.js#L101
- https://github.com/redfin/react-server/blob/master/packages/react-server-core-middleware/src/coreJsMiddleware.js#L28
- https://github.com/redfin/react-server/blob/master/packages/react-server/core/logging/response.js#L76
- https://github.com/redfin/react-server/blob/master/packages/react-server/core/ReactServerAgent/Cache.js#L193
- https://github.com/redfin/react-server/blob/master/packages/react-server/core/renderMiddleware.js#L552
