

Rails XSS vulnerability in number formatting (CVE-2014-0081) - bensedat
https://groups.google.com/forum/#!topic/rubyonrails-security/tfp6gZCtzr4

======
bensedat
Also just wanted to mention: if you run a Rails app it's worth subscribing to
the rubyonrails-security google group. Low traffic except for blasts like
these to alert you to urgent patches.

------
grey-area
So looking at this CVE, it probably won't affect many apps.

You have to be using number_to_currency (common), but also passing in user
input to the parameters format, negative_format or units which would not be a
good idea anyway and is unlikely to have been done. I can't think why you
would do that without verifying the unit and matching against a small set of
acceptable currency units.

You'd have to be doing this:

    
    
        <%= number_to_currency(number, format: unknown_user_text) %>

------
dmix
Looks like some rarely used view helpers. I doubt many of these are used in
production apps.

Hopefully no Bitcoin apps use the currency helper. But I imagine in the
context of an exchange the numbers come from the blockchain or a wallet, and
aren't user controlled in the way that could be exploited.

~~~
jonknee
Why would they be rarely used? That's a _very_ common task. A quick search on
GitHub shows lots of likely uses:

[https://github.com/search?q=number_to_currency+in%3Afile+lan...](https://github.com/search?q=number_to_currency+in%3Afile+language%3Aruby&type=Code&ref=searchresults)

[https://github.com/search?q=number_to_human+in%3Afile+langua...](https://github.com/search?q=number_to_human+in%3Afile+language%3Aruby&type=Code&ref=searchresults)

[https://github.com/search?q=number_to_percentage+in%3Afile+l...](https://github.com/search?q=number_to_percentage+in%3Afile+language%3Aruby&type=Code&ref=searchresults)

------
dave1010uk
I'm not sure I understand this. Are the number helpers now escaping for a HTML
context? Isn't it best practice to escape user input just before outputting it
(so you know the context) rather than in every helper function?

Disclaimer: not a RoR developer.

~~~
riffraff
some helpers produce escaped output, some don't. Think: "link_to(name, url)"
must produce html that must not be escaped again while "strip(text)" can
produce unescaped text.

~~~
nightpool
Doesn't really affect this case though. Why does the numbers_to_ family need
to produce HTML?

~~~
wilg
As far as I know, it doesn't emit HTML. The Rails view layer uses a string
subclass called SafeBuffer ([http://yehudakatz.com/2010/02/01/safebuffers-and-
rails-3-0/](http://yehudakatz.com/2010/02/01/safebuffers-and-rails-3-0/)) to
manage implicit escaping.

I can't look at the diff for some reason (too many redirects), but I'd guess
this issue was the result of some logic that was implemented before
SafeBuffers were added.

------
johne20
off topic, but is it just me who cringes every time I see a google groups url?
I know I am in for a 10 seconds wait to see any content.

------
diziet
I am not sure if I should feel happy or sad that we wrote internal javascript
versions of these helper functions.

------
IceyEC
the part that isn't being escaped seems to be things like the extra arguments,
ie: if you want to format a param as a user defined format, the number would
be properly escaped but the format wouldn't be

------
hayksaakian
cool helper functions, now that I want to use them, I'll need to update.

------
RailsFailsAgain
Seriously - at what point to the stockholm syndrome fans of this trainwreck of
a web framework stop making apologies for the appallingly consistent flow of
security woes.

~~~
hfghegheotqhgo
Actually, we are seeing a large number of new projects move off Rails to
Erlang, Scala, Node and other platforms due to performance and security
concerns. Less magic, more security.

~~~
venus
If those projects have prospered enough that rewriting in a high-performance
language has become a necessity, then I would say Rails fulfilled its function
brilliantly.

Rails is a good, probably the best, general purpose web framework, and no-one
claims otherwise.

~~~
batiste
This comment is good, probably the best, general purpose comment, and no-one
claims otherwise.

