XSS in Rails 3: "raw(...)" for EVERY SINGLE FIXED(!) STRING??!!

Hi,

Using Rails 3 (git master) and Ruby 1.9.2-head I noticed I have to treat EVERY SINGLE STRING in my app, even things like

  link_to " bla", path

with raw(). This is crazy! It is a FIXED string! I understand it when variables are concerned, but this is taking it a little too far. One might even say the escaping only is necessary if STRING variables are introduced, so including number-variables in a(n otherwise fixed) string should not trigger the need to use raw().

I only just started but the amount of "raw()" I have to insert into my app seems excessive.

Michael

Unfortunately, there's no way to know if a string is a literal or not, short of parsing the Ruby, and considering the sheer number of string literals in Ruby, that seems very infeasible.

/Jonas

P.S.: off the top of my head:

"word" 'word' %(word) %[word] %{word} %q(word) %q[word] %q{word} %Q(word) %Q[word] %Q{word} <<STRING word STRING <<-STRING   word STRING etc...

Making the switch to HTML-safety is quite a pain. The grass is greener on the other side, though!

You mark just a handful of strings as <%= raw ... %> instead of almost every string as <%= h ... %> -- less work down the line, plus no lingering XSS worries.

jeremy

Just out of curiosity, couldn't you use String#tainted? to check whether the string was a literal or not?

Cheers, Daniel Schierbeck

Yes! Using native String tainting would have some nice advantages.

Another is that string interpolation would work: "foo #{bar}" is tainted if bar is tainted, but it is not html_safe? if bar is html_safe?

jeremy

If you are certain that your string is safe, then you can mark it as html_safe by doing this:

<%= link_to "&nbsp;bla".html_safe, path %>

- Prem

What is the reason that #tainted? wouldn't suffice? Basically, all tainted strings are HTML escaped; if you wish to circumvent this, mark your strings as untainted with String#untaint. It seems to me that #html_safe tries to replicate functionality which is already present (and more powerful) within Ruby itself.

Should I cook up a patch for this?

Cheers, Daniel Schierbeck

If you are certain that your string is safe, then you can mark it as html_safe by doing this:

<%= link_to "&nbsp;bla".html_safe, path %>

Prem, yes. The original poster's concern is that literal strings are already known to be HTML-safe, so having to state it again is annoying and not DRY.

What is the reason that #tainted? wouldn't suffice? Basically, all tainted strings are HTML escaped; if you wish to circumvent this, mark your strings as untainted with String#untaint. It seems to me that #html_safe tries to replicate functionality which is already present (and more powerful) within Ruby itself.

Should I cook up a patch for this?

The problem with tainted is that it's a single flag for everybody to share. It may have different meanings in different libraries.

Say, a database driver checks taint to see whether strings should be SQL-escaped.

I think the benefits of using tainting in the common case, 99% of usage, far outweigh these unlikelihoods though.

jeremy

That's a valid concern, but I think trying it out is the best approach -- then we'll se if any problems arise. Having a #html_safe wrapper around the tainting also yields some flexibility.

Cheers, Daniel Schierbeck

NzKoz's original note on why he didn't go with #tainted?: http://groups.google.com/group/rubyonrails-core/browse_thread/thread/d04d32341b4790c4/444e732a0b265f96?lnk=gst&q=taint#444e732a0b265f96

That seems reasonable. Perhaps the tainting could be moved higher up the hierarchy, the ActiveRecord itself? All getters would then return tainted strings. Would that not suffice?

Cheers, Daniel Schierbeck

The main issue with taint is that it’s a blacklist operation. This means that we’d be relying on all libraries that could retrieve persisted user data to properly taint their Strings. In contrast, the html_safe approach assumes that all Strings are unsafe, until they are marked safe.

Rails internally marks its own Strings as safe, which is why you don’t need to mark form_for, link_to, etc. as html_safe.

Yehuda Katz Developer | Engine Yard (ph) 718.877.1325

That seems reasonable. Perhaps the tainting could be moved higher up the hierarchy, the ActiveRecord itself? All getters would then return tainted strings. Would that not suffice?

If we used tainting then all it would take is one library to not return tainted strings, and your application is completely hosed.

It's not just a question of ActiveRecord either. Apps read strings from memcached, redis, http (dozens of libraries here), one bug here and you're hosed. The risk outweighs the slim rewards.

Tainting was an alluring option initially, but the more you drill down into it the more of a red-herring it becomes.

I can see that -- it's sad that we have this great functionality built right into Ruby, yet we're unable to take advantage of it.

Well, nothing to do about that.

Cheers, Daniel Schierbeck