7 ms·

Multiple security vulnerabilities in Rails

229 points by alinajaf 11y ago

deleted 11y ago
[deleted]
deanclatworthy 11y ago
I see a timing attack in the list. It's fairly trivial to mitigate against this in the majority of languages nowadays [1] [2] [3] etc.. I presume this can also be mitigated by implementing rate limiting on your authentication endpoints, although that should also be implemented for other reasons. [1] https://golang.org/pkg/crypto/subtle/#ConstantTimeCompare https://golang.org/pkg/crypto/subtle/#ConstantTimeCompare [2] http://php.net/manual/en/function.hash-equals.php http://php.net/manual/en/function.hash-equals.php [3] http://www.levigross.com/2014/02/07/constant-time-comparison-functions-in...-python-haskell-clojure-and-java/ http://www.levigross.com/2014/02/07/constant-time-comparison...
- thibaut_barrere 11y ago
  The current implementation is here: https://github.com/rails/rails/commit/859ca4474e1608b83d61941724574aba491be7f1 https://github.com/rails/rails/commit/859ca4474e1608b83d6194...
  - koolba 11y ago
    That's still broken. They've just pushed the problem deeper. Now instead of having a timing attack on the number of operations in the compare, the timing attack is pushed to the number of bytes that is hashed by sha256. Also, this opens up a new avenue in that now hash collisions (as unlikely as they may be) would be considered equal.
    - ryanlol 11y ago
      >Also, this opens up a new avenue in that now hash collisions (as unlikely as they may be) would be considered equal. Hash collision implies attacker has control of both the inputs, in this case we'd be talking about a preimage attack. If your attacker can perform preimage attacks on SHA256 they can also most likely hack you via your package manager.
    - zaroth 11y ago
      This is considered best practice for languages where you can't trust your "constant time" comparison won't be optimized out from under you. Performing a timing attack requires control of the bytes being compared. If you can control the bytes of the output of a SHA256 then there are some Bitcoin miners who will pay you a lot of money. If you want to be over-the-top about it you can get some secure randomness and add it to the values being compared before hashing, and then attacker would have even less control over the bytes being compared.
      - ryanlol 11y ago
        I suppose you could theoretically deduce a large enough part of the hash to perform a bruteforce attack though. You don't need that much of the hash to perform wordlist attacks and find likely candidates.
        zaroth 11y ago
        Yes, this is true if you are talking about unsalted passwords, or if the attacker knows the salts. If there's an unknown salt, having the salted hash of a given plaintext input match the first few bytes of the target salted hash does not help you narrow the word list at all. If there is no salt, or the salt is public, then that's a case where you could append some ephemeral CS-PRNG output to both sides before hash-comparing... but probably better to fix the underlying issue. I mean, it's funny to be far enough down the rabbit hole to be doing hash-compare. Then to say we wanted randomly keyed hash-compare is the final step. The nice thing is adding some random bytes imposes a fairly miniscule performance hit and it's purely computation, no additional storage. Probably still too slow to use for every comparison, but for constant-time critical comparison, it literally can't hurt.
      - jmgao 11y ago
        You don't need to control every byte for this to be catastrophic. You can't decode every password like you can with the previous comparison, but if you can generate a rainbow table that contains the password you're trying to crack, you can just do a timing attack using the hashes instead. My intuition is that this might even require fewer attempts than the original comparison assuming a reasonable password length, but I haven't done the math.
  - patates 11y ago
    why return false unless a.bytesize == b.bytesize instead of if a.bytesize != b.bytesize return false disclaimer: never programmed in ruby
    - edu 11y ago
      It's idiomatic ruby. First as the conditional only has one statment it's preferred to write it in its shorthand way, so instead of if a.bytesize != b.bytesize return false We start by writing return false if a.bytesize != b.bytesize And then unless is the negated conditional, so we rewrite it as return false unless a.bytesize == b.bytesize Which some peolpe (myself included) consider easier to read, the 'unless' is easier to note (more chars) than the '!='.
nfm 11y ago
Doesn't look too bad, although there are a lot of CVEs to go through: - A timing attack if you're using HTTP basic auth - A couple of GC related DoS attacks - An issue with `accepts_nested_attributes_for` if you're using both the `allow_destroy` and `reject_if` options - A validation bypass exploit if you're calling `SomeModel.new(params[:some_model])` instead of using StrongParams - An information leak exploit if you're calling `render params[:something]` with raw user input - A bunch of potential XSS exploits The `render` issue looks like it could cause the most harm, but hopefully shouldn't be too prevalent. The XSS issues should be a quick fix as you only have to update `rails-html-sanitizer`, not Rails itself.
- ryanlol 11y ago
  >- A timing attack if you're using HTTP basic auth I'd say that qualifies as pretty bad. How the hell does that even happen? Using time constant string comparison is authentication 101. That's really not something you can mess up by mistake, it's something you mess up by not understanding what you're doing. And that's is all ignoring the fact that there's no reason to not use hashing here.
  - semiquaver 11y ago
    The vast majority of rails applications do not use HTTP basic authentication, and I would guess that most of the ones that do use nginx or apache to provide it. This was probably not caught until now because hardly anyone uses it.
    - ryanlol 11y ago
      I've seen a bunch of companies use rails HTTP basic auth internally. And it's not that it wasn't caught until now, it's that it wasn't caught before the commit was accepted.
      - eropple 11y ago
        I expect (though I have not looked) that it's old code that wasn't eyeballed for security so much as for consistency and correctness. Once in, it doesn't leave.
andersonmvd 11y ago
I've grouped the patches for 4.1 and 4.2 here: https://drive.google.com/file/d/0BwnrE2iUdypUMkpqWVVPTXNzNVU/view https://drive.google.com/file/d/0BwnrE2iUdypUMkpqWVVPTXNzNVU... -- because download one by one is boring. Don't trust me, verify each file before patching. Some comments: [CVE-2015-7581] Object leak vulnerability for wildcard controller routes in Action Pack: Look for routes that contain ":controller" and change it to something else. Hopefully you didn't have this weird name in your routes. [CVE-2015-7578/79] Possible XSS vulnerability in rails-html-sanitizer: You're safe if you use a single page application that properly encode for you. Stripping tags isn't the best way anyway to filter XSS, so if you're encoding, you're good. [CVE-2016-0753] Possible Input Validation Circumvention in Active Model: params.permit! is negligence, you should not be doing that anyway [CVE-2016-0752] Possible Information Leak Vulnerability in Action View: render params[:id] is not defensive programming, so you should not be doing that too [CVE-2015-7577] Nested attributes rejection proc bypass in Active Record: Only if using nested_attributes and rejection proc. Wasn't my case. Just patch. [CVE-2016-0751] Possible Object Leak and Denial of Service attack in Action Pack: DoS is bad, just patch. [CVE-2015-7576] Timing attack vulnerability in basic authentication in Action Controller: Just patch. -- Doesn't look THAT bad, but need to be patched fast.
- Someone1234 11y ago
  Wow you have a terrible attitude about security. "None of these are an issue, just program in this [very specific way that requires pre-knowledge of these vulnerabilities] and you're safe, anything else is basically negligence." Your opinions about rails-html-sanitizer are particularly troubling as even if you use the sanitizer as suggested in the docs you're vulnerable and your retort is "well you should encode AND sanitise, not just rely on the sanitiser doing what the documentation says it should do!" Why? I have no issue with the wording in the official CVEs. But this attempt at whitewashing the, frankly, pretty serious issues is deplorable.
  - andersonmvd 11y ago
    Some programming styles are not defensive from a security perspective. As a programmer one should acknowledge that using "render" passing a param right from the request, without validating it, is not a good thing, right? That's my point. Some issues here can be solved just by taking the right approach, but won't solve for all of them, of course. XSS mitigation works better with encoding rather than sanitization. If you want me to explain I explain. But hey, you won't do any good complaining in face of this situation. Time to help people fix it. Peace.
deleted 11y ago
[deleted]
matdrewin 11y ago
On one hand, I find that Rails often has security issues. On the other hand, perhaps it is just indicative of its popularity and interest. When a framework has no security issues, is it because there are none or is it just that no one can be bothered to look for some?
- elliotec 11y ago
  Excellent point. Also the fact is there will be people working on these immediately.
- masterleep 11y ago
  The last entries on the Rails security list before this were from June of 2015. Does that seem like a lot to you?
trollian 11y ago
Nobody's actually still exposing rails sites to the internet are they? I mean except as a honeypot. The rails team's approach to security (it's the application's problem) has been consistent. At this point if you're letting the wild Internet touch your rails apps it's probably your own fault.
- josephagoss 11y ago
  Is this the general view? What is the best course of action here? Stop using Rails or somehow protect it better?
  - rimantas 11y ago
    The best course of action is to ignore trolls.
  - VeejayRampay 11y ago
    The HN handle is "trollian". Enough said.
dain 11y ago
Aah Aaron. Thanks. Everywhere he codes he refactors, fixes performance issues, finds bugs, he's so my hero.
- VeejayRampay 11y ago
  tenderlove is the best. Not only does he work on both Ruby and Ruby on Rails, he's also very active in the community (conferences, workshops, talks) and working hard on promoting a spirit of humility and respect (see the humorous Adequate HQ, the Friday Hug, the overwhelmingly positive and generous attitude he's always had towards anyone, famous or not, in the Ruby community). I have nothing but respect for that man and I'm glad he's working hard everyday on the language I so love. Thank you Aaron.
  - dain 11y ago
    Really really well said. So grateful to have him in our lovely community.
igravious 11y ago
A quick `bundle update` appears to be just the ticket: … Installing rails-html-sanitizer 1.0.3 (was 1.0.2) Installing actionmailer 4.2.5.1 (was 4.2.5) Installing activemodel 4.2.5.1 (was 4.2.5) Installing activerecord 4.2.5.1 (was 4.2.5) Installing railties 4.2.5.1 (was 4.2.5) Installing rails 4.2.5.1 (was 4.2.5) … Bundle updated!
tetraverse 11y ago
What was the name of that still-in-development OS that is going to mitigate against most forms of conventional attacks. random quote: 'I used to consume cannabis on a daily basis, I suffer no short term memory loss, as far as I can remember....'
forced-request 11y ago
CVE-2016-0752 is explained in more detail here: https://nvisium.com/blog/2016/01/26/rails-dynamic-render-to-rce-cve-2016-0752/ https://nvisium.com/blog/2016/01/26/rails-dynamic-render-to-...
- secretmike 11y ago
  Great overview, thanks!

229 points by alinajaf 11y ago

deleted 11y ago

[deleted]

I see a timing attack in the list. It's fairly trivial to mitigate against this in the majority of languages nowadays [1] [2] [3] etc.. I presume this can also be mitigated by implementing rate limiting on your authentication endpoints, although that should also be implemented for other reasons. [1] https://golang.org/pkg/crypto/subtle/#ConstantTimeCompare https://golang.org/pkg/crypto/subtle/#ConstantTimeCompare [2] http://php.net/manual/en/function.hash-equals.php http://php.net/manual/en/function.hash-equals.php [3] http://www.levigross.com/2014/02/07/constant-time-comparison-functions-in...-python-haskell-clojure-and-java/ http://www.levigross.com/2014/02/07/constant-time-comparison...

thibaut_barrere 11y ago
The current implementation is here: https://github.com/rails/rails/commit/859ca4474e1608b83d61941724574aba491be7f1 https://github.com/rails/rails/commit/859ca4474e1608b83d6194...
- koolba 11y ago
  That's still broken. They've just pushed the problem deeper. Now instead of having a timing attack on the number of operations in the compare, the timing attack is pushed to the number of bytes that is hashed by sha256. Also, this opens up a new avenue in that now hash collisions (as unlikely as they may be) would be considered equal.
  - ryanlol 11y ago
    >Also, this opens up a new avenue in that now hash collisions (as unlikely as they may be) would be considered equal. Hash collision implies attacker has control of both the inputs, in this case we'd be talking about a preimage attack. If your attacker can perform preimage attacks on SHA256 they can also most likely hack you via your package manager.
  - zaroth 11y ago
    This is considered best practice for languages where you can't trust your "constant time" comparison won't be optimized out from under you. Performing a timing attack requires control of the bytes being compared. If you can control the bytes of the output of a SHA256 then there are some Bitcoin miners who will pay you a lot of money. If you want to be over-the-top about it you can get some secure randomness and add it to the values being compared before hashing, and then attacker would have even less control over the bytes being compared.
    - ryanlol 11y ago
      I suppose you could theoretically deduce a large enough part of the hash to perform a bruteforce attack though. You don't need that much of the hash to perform wordlist attacks and find likely candidates.
      - zaroth 11y ago
        Yes, this is true if you are talking about unsalted passwords, or if the attacker knows the salts. If there's an unknown salt, having the salted hash of a given plaintext input match the first few bytes of the target salted hash does not help you narrow the word list at all. If there is no salt, or the salt is public, then that's a case where you could append some ephemeral CS-PRNG output to both sides before hash-comparing... but probably better to fix the underlying issue. I mean, it's funny to be far enough down the rabbit hole to be doing hash-compare. Then to say we wanted randomly keyed hash-compare is the final step. The nice thing is adding some random bytes imposes a fairly miniscule performance hit and it's purely computation, no additional storage. Probably still too slow to use for every comparison, but for constant-time critical comparison, it literally can't hurt.
    - jmgao 11y ago
      You don't need to control every byte for this to be catastrophic. You can't decode every password like you can with the previous comparison, but if you can generate a rainbow table that contains the password you're trying to crack, you can just do a timing attack using the hashes instead. My intuition is that this might even require fewer attempts than the original comparison assuming a reasonable password length, but I haven't done the math.
- patates 11y ago
  why return false unless a.bytesize == b.bytesize instead of if a.bytesize != b.bytesize return false disclaimer: never programmed in ruby
  - edu 11y ago
    It's idiomatic ruby. First as the conditional only has one statment it's preferred to write it in its shorthand way, so instead of if a.bytesize != b.bytesize return false We start by writing return false if a.bytesize != b.bytesize And then unless is the negated conditional, so we rewrite it as return false unless a.bytesize == b.bytesize Which some peolpe (myself included) consider easier to read, the 'unless' is easier to note (more chars) than the '!='.

nfm 11y ago

Doesn't look too bad, although there are a lot of CVEs to go through: - A timing attack if you're using HTTP basic auth - A couple of GC related DoS attacks - An issue with `accepts_nested_attributes_for` if you're using both the `allow_destroy` and `reject_if` options - A validation bypass exploit if you're calling `SomeModel.new(params[:some_model])` instead of using StrongParams - An information leak exploit if you're calling `render params[:something]` with raw user input - A bunch of potential XSS exploits The `render` issue looks like it could cause the most harm, but hopefully shouldn't be too prevalent. The XSS issues should be a quick fix as you only have to update `rails-html-sanitizer`, not Rails itself.

ryanlol 11y ago
>- A timing attack if you're using HTTP basic auth I'd say that qualifies as pretty bad. How the hell does that even happen? Using time constant string comparison is authentication 101. That's really not something you can mess up by mistake, it's something you mess up by not understanding what you're doing. And that's is all ignoring the fact that there's no reason to not use hashing here.
- semiquaver 11y ago
  The vast majority of rails applications do not use HTTP basic authentication, and I would guess that most of the ones that do use nginx or apache to provide it. This was probably not caught until now because hardly anyone uses it.
  - ryanlol 11y ago
    I've seen a bunch of companies use rails HTTP basic auth internally. And it's not that it wasn't caught until now, it's that it wasn't caught before the commit was accepted.
    - eropple 11y ago
      I expect (though I have not looked) that it's old code that wasn't eyeballed for security so much as for consistency and correctness. Once in, it doesn't leave.

andersonmvd 11y ago

I've grouped the patches for 4.1 and 4.2 here: https://drive.google.com/file/d/0BwnrE2iUdypUMkpqWVVPTXNzNVU/view https://drive.google.com/file/d/0BwnrE2iUdypUMkpqWVVPTXNzNVU... -- because download one by one is boring. Don't trust me, verify each file before patching. Some comments: [CVE-2015-7581] Object leak vulnerability for wildcard controller routes in Action Pack: Look for routes that contain ":controller" and change it to something else. Hopefully you didn't have this weird name in your routes. [CVE-2015-7578/79] Possible XSS vulnerability in rails-html-sanitizer: You're safe if you use a single page application that properly encode for you. Stripping tags isn't the best way anyway to filter XSS, so if you're encoding, you're good. [CVE-2016-0753] Possible Input Validation Circumvention in Active Model: params.permit! is negligence, you should not be doing that anyway [CVE-2016-0752] Possible Information Leak Vulnerability in Action View: render params[:id] is not defensive programming, so you should not be doing that too [CVE-2015-7577] Nested attributes rejection proc bypass in Active Record: Only if using nested_attributes and rejection proc. Wasn't my case. Just patch. [CVE-2016-0751] Possible Object Leak and Denial of Service attack in Action Pack: DoS is bad, just patch. [CVE-2015-7576] Timing attack vulnerability in basic authentication in Action Controller: Just patch. -- Doesn't look THAT bad, but need to be patched fast.

Someone1234 11y ago
Wow you have a terrible attitude about security. "None of these are an issue, just program in this [very specific way that requires pre-knowledge of these vulnerabilities] and you're safe, anything else is basically negligence." Your opinions about rails-html-sanitizer are particularly troubling as even if you use the sanitizer as suggested in the docs you're vulnerable and your retort is "well you should encode AND sanitise, not just rely on the sanitiser doing what the documentation says it should do!" Why? I have no issue with the wording in the official CVEs. But this attempt at whitewashing the, frankly, pretty serious issues is deplorable.
- andersonmvd 11y ago
  Some programming styles are not defensive from a security perspective. As a programmer one should acknowledge that using "render" passing a param right from the request, without validating it, is not a good thing, right? That's my point. Some issues here can be solved just by taking the right approach, but won't solve for all of them, of course. XSS mitigation works better with encoding rather than sanitization. If you want me to explain I explain. But hey, you won't do any good complaining in face of this situation. Time to help people fix it. Peace.

deleted 11y ago

[deleted]

matdrewin 11y ago

On one hand, I find that Rails often has security issues. On the other hand, perhaps it is just indicative of its popularity and interest. When a framework has no security issues, is it because there are none or is it just that no one can be bothered to look for some?

elliotec 11y ago
Excellent point. Also the fact is there will be people working on these immediately.
masterleep 11y ago
The last entries on the Rails security list before this were from June of 2015. Does that seem like a lot to you?

trollian 11y ago

Nobody's actually still exposing rails sites to the internet are they? I mean except as a honeypot. The rails team's approach to security (it's the application's problem) has been consistent. At this point if you're letting the wild Internet touch your rails apps it's probably your own fault.

josephagoss 11y ago
Is this the general view? What is the best course of action here? Stop using Rails or somehow protect it better?
- rimantas 11y ago
  The best course of action is to ignore trolls.
- VeejayRampay 11y ago
  The HN handle is "trollian". Enough said.

dain 11y ago

Aah Aaron. Thanks. Everywhere he codes he refactors, fixes performance issues, finds bugs, he's so my hero.

VeejayRampay 11y ago
tenderlove is the best. Not only does he work on both Ruby and Ruby on Rails, he's also very active in the community (conferences, workshops, talks) and working hard on promoting a spirit of humility and respect (see the humorous Adequate HQ, the Friday Hug, the overwhelmingly positive and generous attitude he's always had towards anyone, famous or not, in the Ruby community). I have nothing but respect for that man and I'm glad he's working hard everyday on the language I so love. Thank you Aaron.
- dain 11y ago
  Really really well said. So grateful to have him in our lovely community.

igravious 11y ago

A quick `bundle update` appears to be just the ticket: … Installing rails-html-sanitizer 1.0.3 (was 1.0.2) Installing actionmailer 4.2.5.1 (was 4.2.5) Installing activemodel 4.2.5.1 (was 4.2.5) Installing activerecord 4.2.5.1 (was 4.2.5) Installing railties 4.2.5.1 (was 4.2.5) Installing rails 4.2.5.1 (was 4.2.5) … Bundle updated!

tetraverse 11y ago

What was the name of that still-in-development OS that is going to mitigate against most forms of conventional attacks. random quote: 'I used to consume cannabis on a daily basis, I suffer no short term memory loss, as far as I can remember....'

forced-request 11y ago

CVE-2016-0752 is explained in more detail here: https://nvisium.com/blog/2016/01/26/rails-dynamic-render-to-rce-cve-2016-0752/ https://nvisium.com/blog/2016/01/26/rails-dynamic-render-to-...

secretmike 11y ago
Great overview, thanks!