Implement Encode.forJSON()

[OpenGG]

The XSS prevention cheat sheet mentioned the anti-pattern, placing json inside js block.

<script>
  var initData = <%= data.to_json %>; // Do NOT do this without encoding the data with one of the techniques listed below.
</script>

This has been a common practice, and is an important part of redux's server rendering. But owasp-java-encoder doesn't has the right encoding function to deal with it.

Is it consider doable and necessary, to implement Encode.forJSON()?

From what I read on the cheat sheet, converting < to \u003c would be good enough.

[jmanico]

To safely embed JSON I continue to advise NOT encoding it. Instead serialize it to safely embed it. Like so... https://github.com/yahoo/serialize-javascript <script>window.__INITIAL_STATE = <%= serialize(data.to_json) %></script> This is so an attack like { haxorXSS: '</script>' } would render safely as {"haxorXSS":"\\u003C\\u002Fscript\\u003E"} **** You can encode it, but you do not need a JSON specific encoder. What you can do is HTML escape it per https://www.owasp.org/index.php/XSS_(Cross_Site_Scripting)_Prevention_Cheat_Sheet#RULE_.233.1_-_HTML_escape_JSON_values_in_an_HTML_context_and_read_the_data_with_JSON.parse ... (from the cheatsheet) <div id="init_data" style="display: none"> <%= html_escape(data.to_json) %> </div> // external js file var dataElement = document.getElementById('init_data'); // decode and parse the content of the div var initData = JSON.parse(dataElement.textContent); So a new encoding routine should not be needed. Aloha, Jim

…

On 5/9/18 1:16 AM, OpenGG wrote: The XSS prevention cheat sheet <https://www.owasp.org/index.php/XSS_%28Cross_Site_Scripting%29_Prevention_Cheat_Sheet#RULE_.233.1_-_HTML_escape_JSON_values_in_an_HTML_context_and_read_the_data_with_JSON.parse> mentioned the anti-pattern, placing json inside js block. <script> var initData = <%= data.to_json %>; // Do NOT do this without encoding the data with one of the techniques listed below. </script> This has been a common practice, and is an important part of redux's server rendering <https://redux.js.org/recipes/server-rendering>. But |owasp-java-encoder| doesn't has the right encoding function to deal with it. Is this consider doable and necessary, to implement |Encode.forJSON()|? From what I read on the cheat sheet, converting |<| to |\u003c| would be good enough. — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#16>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAgcCeTEkGYsm6Tl6N2z-rL_jyzKGpLzks5twtAogaJpZM4T4JEh>.

-- Jim Manico Manicode Security https://www.manicode.com

[jmanico]

So I am digging into this more. The Yahoo serializer is old and only supports certain features. The more I look into this the more I think the escaping method listed below is best because HTML escaping in this context is always safe regardless of the JSON features... <div id="init_data" style="display: none"> <%= Encoder.encodeForHTML(data.to_json) %> </div> And parsing JSON with JSON.parse is always safe. This will work without needing to worry about the serialization feature set. // external js file var dataElement = document.getElementById('init_data'); // decode and parse the content of the div var initData = JSON.parse(dataElement.textContent); ****** I also wonder (but have not tested) if just JS escaping here is the right move. Note it's a quoted string and JS encoded. I know this step is safe... <script> var initData = '<%= Encoder.encodeForJavascript(data.to_json) %>'; </script> And parsing JSON with JSON.parse is always safe. // decode and parse the content of the div var initData = JSON.parse(initData); Can you do a quick test to see if basic JS encoding works since you're sticking it in a JS variable? Aloha, Jim

…

To safely embed JSON I continue to advise NOT encoding it. Instead serialize it to safely embed it. Like so... https://github.com/yahoo/serialize-javascript <script>window.__INITIAL_STATE = <%= serialize(data.to_json) %></script> This is so an attack like { haxorXSS: '</script>' } would render safely as {"haxorXSS":"\\u003C\\u002Fscript\\u003E"} **** You can encode it, but you do not need a JSON specific encoder. What you can do is HTML escape it per https://www.owasp.org/index.php/XSS_(Cross_Site_Scripting)_Prevention_Cheat_Sheet#RULE_.233.1_-_HTML_escape_JSON_values_in_an_HTML_context_and_read_the_data_with_JSON.parse ... (from the cheatsheet) <div id="init_data" style="display: none"> <%= html_escape(data.to_json) %> </div> // external js file var dataElement = document.getElementById('init_data'); // decode and parse the content of the div var initData = JSON.parse(dataElement.textContent); So a new encoding routine should not be needed. Aloha, Jim On 5/9/18 1:16 AM, OpenGG wrote: > > The XSS prevention cheat sheet > <https://www.owasp.org/index.php/XSS_%28Cross_Site_Scripting%29_Prevention_Cheat_Sheet#RULE_.233.1_-_HTML_escape_JSON_values_in_an_HTML_context_and_read_the_data_with_JSON.parse> > mentioned the anti-pattern, placing json inside js block. > > <script> > var initData = <%= data.to_json %>; // Do NOT do this without > encoding the data with one of the techniques listed below. > </script> > > This has been a common practice, and is an important part of redux's > server rendering <https://redux.js.org/recipes/server-rendering>. But > |owasp-java-encoder| doesn't has the right encoding function to deal > with it. > > Is this consider doable and necessary, to implement |Encode.forJSON()|? > > From what I read on the cheat sheet, converting |<| to |\u003c| would > be good enough. > > — > You are receiving this because you are subscribed to this thread. > Reply to this email directly, view it on GitHub > <#16>, or mute the > thread > <https://github.com/notifications/unsubscribe-auth/AAgcCeTEkGYsm6Tl6N2z-rL_jyzKGpLzks5twtAogaJpZM4T4JEh>. > -- Jim Manico Manicode Security https://www.manicode.com

-- Jim Manico Manicode Security https://www.manicode.com

[OpenGG]

@jmanico

• Encoding everything in the unicode format \u0000 inflates the output, and may not be the default setting for most JSON encoders.
• An extra HTML-encode-js-decode operation bloats the problem and make it more complicated. Consider the situation that multiple JSON being embed into one HTML.
• var initData = JSON.parse('<%= Encoder.forJavaScript(data.to_json) %>'); does work, but not every browser supports JSON.parse(), hence server-side encoding introduces client-side dependency.

For now I can go with JSON.parse('<%= Encoder.forJavaScript(data.to_json) %>'), but I really hope for a more intuitive Encode.forJSON().

As a common practice, embedding JSON in HTML is popular. Implementing such encoding function can help the community moving forward.

[jmanico]

Encoding is about the slot you put data into HTML, not about the content you’re embedding. There is no all-purpose JSON encoding format that will universally work in all HTML locations. I’m glad the JS trick worked for Javascript locations like.. <script> JSON.parse('<%= Encoder.encodeForJavascript(data.to_json) %>') </script> If you don’t like the name encodeForJavascript may I suggest building a wrapper class with your encodeForJSON and just call our function? Again, we can’t guarantee an encoding JSON format that will work in all situations. It depends where you embed JSON. Browsers that do support JSON.parse are really old and unsafe to handle JSON, but folks did use eval() for that back in the day - try using JQuerys function for JSON parsing. It’s standard practice to push all JSON through JSON.parse. So still, I’m close marking this “wont fix” because I’d don’t see a clear way to do it.

…

On May 9, 2018, at 7:06 PM, OpenGG ***@***.***> wrote: @jmanico Encoding everything in the unicode format \u0000 inflates the output, and may not be the default setting for most JSON encoders. An extra HTML-encode-js-decode operation bloats the problem and make it more complicated. Consider the situation that multiple JSON being embed into one HTML. var initData = JSON.parse('<%= Encoder.encodeForJavascript(data.to_json) %>'); does work, but not every browser supports JSON.parse(), hence server-side encoding introduce client-side dependency. For now I can go with JSON.parse('<%= Encoder.encodeForJavascript(data.to_json) %>'), but I really hope for a more intuitive Encode.forJSON(). As a common practice, embedding JSON in HTML is popular. Implementing such encoding function can help the community moving forward. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.

[OpenGG]

@jmanico

Again, we can’t guarantee an encoding JSON format that will work in all situations. It depends where you embed JSON.

Make sense, Consider the following two cases, one encoding function Encode.forJSON() won't work well in both of them.

<a onclick="javascript:x=${Encode.forJSON(data)}">

<script>
x = ${Encode.forJSON(data)}
</script>

This could be the limitation of encoder libraries like owasp-java-encoder. Without further information of the context, an encoding library can't properly escape complex data type like JSON.

I will look into contextual automatic escaping templates, and see if they can do better. Meanwhile, please mark this issue as "wont fix".

[jmanico]

I agree that automatic encoding templates are the best path forward. Thanks for jumping in here with this thread.