[minitech]

> * Properly serialize your JSON strings. That's what we use: https://gist.github.com/eliseumds/6192135660267e2c64180a8a9c....

That doesn’t look like “properly”. The double escaping is overcomplicated and no safer compared to a direct

  window.__productreview_data = ${escapedReduxStateJsonString};

(and forgets about \v, maybe others), the transformation doesn’t preserve “</_escaped_script”, and it doesn’t address a vulnerability involving <!-- that’s contrivable.

Closer to correct:

  JSON.stringify(data)
    .replace(/\u2028/g, '\\u2028')
    .replace(/\u2029/g, '\\u2029')
    .replace(/</g, '\\x3c')

Better, if you put the JSON in an inert <script> (type="application/json"), it’s only necessary to escape < (or /<[/!]/g). This is a good idea so you can use restrictive CSPs.

[eliseumds]

Thanks for pointing out the `<!--` vulnerability. In regards to rendering the string inside a JSON.parse, we do that because of performance: https://v8.dev/blog/cost-of-javascript-2019. From what I remember, we had some issues with IE11, thus the replacement for the other characters.

We'll consider "application/json", makes sense.

[minitech]

Given a correct function that converts a JSON-representable value to embed-safe JSON, you can use it on the JSON to get your JSON.parse performance:

  const inlineJSON = data =>
    JSON.stringify(data)
      .replace(/\u2028/g, '\\u2028')
      .replace(/\u2029/g, '\\u2029')
      .replace(/</g, '\\x3c');

with:

  const escapedReduxStateJsonString = inlineJSON(JSON.stringify(data));

But yeah, the isolated <script> thing is usually even better (more compact in addition to the security benefit).

[andrewingram]

I've been recommended this library rather than figuring it for myself:

https://github.com/yahoo/serialize-javascript

You can use it in JSON mode like this: `serialize(obj, {isJSON: true});`