Rails form helpers uses "escape_once" causing undesirable behavior

Hi, I posted this on lighthouse a few weeks ago:
https://rails.lighthouseapp.com/projects/8994/tickets/3213-html-literals-passed-into-tag-helpers-dont-escape

A patch, with tests, is included. I'd like to use form helpers but
we're forced not to in some situations because of this bug popping
up. However, this patch may break some behavior that people rely
upon.

I would argue that the current behavior is broken. If I pass in the
string "<" to a form helper, I expect it to show up in the browser
as "&lt;", not "<". If some people are passing pre-escaped HTML into
form helpers, that is an ambiguity in their code that they should fix.

Note that applying my patch might cause undesirable behavior in some
people's (broken) applications, but could not possibly introduce any
XSS vulnerabilities. This is because it's replacing a semi-broken
version of HTML escaping with one that's stricter.

If this has been discussed before, I would appreciate a pointer to the
discussion. If it merits further discussion, I would be happy to
argue my case. If this patch is going to be accepted, consider this
message a gentle prod for it to be merged in.

I would argue that the current behavior is broken. If I pass in the
string "&lt;" to a form helper, I expect it to show up in the browser
as "&lt;", not "<". If some people are passing pre-escaped HTML into
form helpers, that is an ambiguity in their code that they should fix.

Unfortunately it's not *quite* that simple because there are places
where we're blindly escaping and then re-escaping those strings.

escape_once is a gross hack and my goal is to remove it in 3.0 in
favour of using the rails_xss stuff I have in a branch / plugin at
present. That will let us distinguish between "&lt;", and h("<"), and
only escape the &lt; case.

So my gut feeling here is that we favour a proper fix in 3.0, and
leave 2-3-stable 'consistently weird' rather than run the risk of
complicating things.

http://github.com/NZKoz/rails_xss
http://github.com/NZKoz/rails/tree/rails_xss

Thanks for the feedback.

I'm not sure when Rails 3 is going to come out---and I'm not going to
pester you about it--but I noticed that your rails_xss branch is
merging in changes from 2-3-stable. (It looks like nice work, by the
way.) What's the chance of getting the XSS stuff out in a Rails 2.3.5
release or something like that? If the XSS branch could work with
Rails 2.3.x, it would make for an easier migration path. I expect
we'll have to do a good deal of work to get our app to run in Rails 3,
so any potentially difficult changes that can be merged in before that
would be helpful.

Keep up the good work! We appreciate it.

Thanks for the feedback.

I'm not sure when Rails 3 is going to come out---and I'm not going to
pester you about it--but I noticed that your rails_xss branch is
merging in changes from 2-3-stable. (It looks like nice work, by the
way.) What's the chance of getting the XSS stuff out in a Rails 2.3.5
release or something like that? If the XSS branch could work with
Rails 2.3.x, it would make for an easier migration path. I expect
we'll have to do a good deal of work to get our app to run in Rails 3,
so any potentially difficult changes that can be merged in before that
would be helpful.

The current plan is to merge the String#html_safe! and the helper
changes into 2.3.5 (or .6, depending on when I get the time), and
leave the 'escape by default' stuff in a plugin. Then for 3.0 the
escape by default stuff is in core.

So people should be able to beta test the functionality in 2.3.x, and
let us fix any surprises before 3.0.

Having said all that, I have a newborn due within 3 weeks, so it's
entirely possible my plans will change and I'll need someone else to
wrap it up. Hope not though :slight_smile: