On-By-Default XSS escaping

5 views
Skip to first unread message

Michael Koziarski

unread,
Jul 27, 2009, 11:25:29 PM7/27/09
to rubyonra...@googlegroups.com
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.

--
Cheers

Koz

Jonas Nicklas

unread,
Jul 28, 2009, 2:14:19 AM7/28/09
to rubyonra...@googlegroups.com
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

Michael Koziarski

unread,
Jul 28, 2009, 3:00:37 AM7/28/09
to rubyonra...@googlegroups.com
> 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.


--
Cheers

Koz

Jack Christensen

unread,
Jul 29, 2009, 6:16:42 PM7/29/09
to rubyonra...@googlegroups.com
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

Michael Koziarski

unread,
Jul 29, 2009, 6:20:29 PM7/29/09
to rubyonra...@googlegroups.com
> 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 ;)

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

unread,
Jul 30, 2009, 4:19:18 AM7/30/09
to rubyonra...@googlegroups.com
Hi list.

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

unread,
Jul 30, 2009, 10:02:26 AM7/30/09
to rubyonra...@googlegroups.com


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

unread,
Jul 30, 2009, 4:09:55 PM7/30/09
to rubyonra...@googlegroups.com
On Thu, Jul 30, 2009 at 4:02 PM, Rob Biedenharn<Rob.Bie...@gmail.com> wrote:
> 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

unread,
Aug 8, 2009, 8:22:40 PM8/8/09
to Ruby on Rails: Core
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.
Reply all
Reply to author
Forward
0 new messages