brunofacca / zen-rails-security-checklist

Checklist of security precautions for Ruby on Rails applications.
MIT License
1.81k stars 150 forks source link

Fix name of json_escape helper #20

Closed nate00 closed 6 years ago

nate00 commented 6 years ago

I couldn't find a json_encode method defined anywhere in Rails. And it's the json_escape documentation that includes the recommendation to use this helper "just in case to_json is overridden".

nate00 commented 6 years ago

Hi Bruno, thanks for the great work on this checklist. It has been hugely useful to me!

There's one part of this bullet point that I couldn't square with the json_escape docs. This checklist bullet says:

The Rails documentation recommends always using json_escape just in case [...] the value is not valid JSON.

But the json_escape docs seem to recommend against exactly that use case:

WARNING: this helper only works with valid JSON. Using this on non-JSON values will open up serious XSS vulnerabilities. For example, if you replace the current_user.to_json in the example above with user input instead, the browser will happily eval() that string as JavaScript.

Am I misunderstanding something, or should we change that clause of this checklist item?

brunofacca commented 6 years ago

I'm glad you found the checklist useful :)

You are right, it should be json_escape. Thanks for your contribution.