On-By-Default XSS escaping
Michael Koziarski
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.
--
Cheers
Koz
Jonas Nicklas
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
Michael Koziarski
> 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.
--
Cheers
Koz
Jack Christensen
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
Michael Koziarski
> 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 ;)
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'.
--
Cheers
Koz
troels knak-nielsen
On Thu, Jul 30, 2009 at 12:20 AM, Michael
Koziarski<mic...@koziarski.com> wrote:
>
> # 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
Rob Biedenharn
If you hadn't limited your reading to the code example:
On Jul 29, 2009, at 6:20 PM, Michael Koziarski wrote:
> 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) %>
Note that Koz said "one of the following changes"
-Rob
troels knak-nielsen
> 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.
--
troels
Michael Koziarski
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.