On-By-Default XSS escaping

Hey Guys,

one of the goals on my TODO list for 3.0 is to import django-style auto-escaping into rails' ERB templates. Obviously this has the potential to completely break basically every application out there, so we want to do it carefully. In order to do this I also want to make the escape-by-default stuff available as a plugin for 2.3.4 and above.

The changes will consist of 3 steps:

# Introduce the notion of 'output safety' for strings & Implement a buffer which auto-escapes unsafe strings # Make sure all the helpers which are safe, return output safe strings. # Make the ERB templates use the Safe buffer instead of a string.

Unfortunately the second step is almost impossible to implement securely / accurately in a plugin, so some of this stuff will need to be merged to 2-3-stable in order to make it available.

My theory is that those first two steps should be completely transparent to end-users and there's no possible way that those changes could break an application. But before I did anything I wanted to get feedback from the community.

The Plugin is here: http://github.com/NZKoz/rails_xss My Rails Branch is here: http://github.com/NZKoz/rails/tree/rails_xss

Both of these need a bunch of work before they're release-ready, but they should be good enough for you guys to see where we're going and what's likely to land in a repository some time soon.

So, please let me know if:

# You can think of a way merging my branch would break your application # You see anything crazy with the approach being taken. # Anything else strikes you.

Looks good. Will be nice not to have to worry about XSS so much anymore.

Out of curiosity, what is the reasoning behind not using the ‘taint’ mechanism built into Ruby. Is it because this is more of a white-list approach, whereas ‘taint’ is more of a black-list approach?

/Jonas

Out of curiosity, what is the reasoning behind not using the 'taint' mechanism built into Ruby. Is it because this is more of a white-list approach, whereas 'taint' is more of a black-list approach?

Exactly, plus the database drivers all differed as to whether or not they tainted strings. By using something 'untrusted by default' it was easier to reason about.

Michael Koziarski wrote:

Hey Guys,

one of the goals on my TODO list for 3.0 is to import django-style auto-escaping into rails' ERB templates. Obviously this has the potential to completely break basically every application out there, so we want to do it carefully. In order to do this I also want to make the escape-by-default stuff available as a plugin for 2.3.4 and above.

The changes will consist of 3 steps:

# Introduce the notion of 'output safety' for strings & Implement a buffer which auto-escapes unsafe strings # Make sure all the helpers which are safe, return output safe strings. # Make the ERB templates use the Safe buffer instead of a string.

Unfortunately the second step is almost impossible to implement securely / accurately in a plugin, so some of this stuff will need to be merged to 2-3-stable in order to make it available.

My theory is that those first two steps should be completely transparent to end-users and there's no possible way that those changes could break an application. But before I did anything I wanted to get feedback from the community.

The Plugin is here: GitHub - NZKoz/rails_xss: A plugin for rails 2.3.5 applications which switches the default to escape by default. Later versions should use rails/rails_xss My Rails Branch is here: GitHub - NZKoz/rails at rails_xss

Both of these need a bunch of work before they're release-ready, but they should be good enough for you guys to see where we're going and what's likely to land in a repository some time soon.

So, please let me know if:

# You can think of a way merging my branch would break your application # You see anything crazy with the approach being taken. # Anything else strikes you.

Is this configurable on a controller by controller basis? I work on an app with 150+ controllers and 600+ views. It would be much easier to go migrate one controller at a time than to try the whole app at once.

Jack

Is this configurable on a controller by controller basis? I work on an app with 150+ controllers and 600+ views. It would be much easier to go migrate one controller at a time than to try the whole app at once.

No, there will be no configuration option to disable this, the goal is for it to be 99% backwards compatible, and you *should* be thinking about each of the cases where you don't want escaping. Twitter's stalkdaily.com worm being a case in point :wink:

However everything you do should be completely backwards compatible *apart* from where you want the html output of a custom helper to be embedded directly into your app, and that would only require one of the following changes:

# safe_helper :some_helper after declaring the helper # <%= raw(some_helper) %>

All the built in helpers including and any custom helpers which use tag /content_tag will 'just work'.

Hi list.

Hi list.

# safe_helper :some_helper after declaring the helper # <%= raw(some_helper) %>

Is it necessary to have both a `safe_helper` declaration and an explicit call to `raw`? Wouldn't one or the other be sufficient?

-- troels

If you hadn't limited your reading to the code example:

Ah .. My bad. Then I can make sense of it.

fwiw I welcome this change. It'll cause some pain to get introduced, but it's the right thing and in the end everybody will be better off that way.

Further to this thread, I've attached the proposed change to a lighthouse ticket (#3018):

http://bit.ly/17sBT4

If you're worried about what this may do to your application you should try out the patch attached to that ticket and comment there.