6 ms·

Security hole found in Rails 2.3's http_authentication.rb

83 points by nate 17y ago

tptacek 17y ago
Yep, looks real enough to me. Confusion between "nil" and "false" in Ruby, where an assumed true/false return value actually takes on a third value that also matches a nil password.
- wglb 17y ago
  Not being a ruby programmer, is there a defensive coding idiom that would have prevented this confusion?
  - rcoder 17y ago
    You can always test for nil explicitly. Compare: unless foo # ... foo can be false or nil to reach this branch end unless foo == false # ... this branch is reached unless foo == false end Since terseness is considered such a cardinal virtue amongst Ruby programmers (including myself), however, this kind of issue is often ignored, and difficult to detect unless your tests explicitly check for "truthy" and "non-truthy" values in addition to the true/false literals.
    - wglb 17y ago
      Thanks.
    - danishgolkonda 17y ago
      tell me in detail
  - ionfish 17y ago
    if object.nil? puts "Object is nil!" else puts "Object is not nil." end
  - lsb 17y ago
    Haskell obviously isn't Ruby, but its Maybe monad, where you have Just the login, or Nothing, looks pretty appealing.
bryanwoods 17y ago
Interesting, I'm not getting this flaw at all. Which version of 2.3 is this addressing? Rails 2.3.1 only? I just tried this on a 2.3.2 app and got HTTP Basic: Access denied.
- nate 17y ago
  Rails 2.3.x. The app you tested this on, what's the password procedure like? One part of problem is doing a password procedure like "Users[name]" where Users is a hash of usernames/passwords. Also I believe this just effects folks doing digest auth (authenticate_or_request_with_http_digest) not basic auth (authenticate_or_request_with_http_basic)
kneath 17y ago
I'm curious, did you submit a bug report for this patch? I can't find it on http://rails.lighthouseapp.com http://rails.lighthouseapp.com. It seems odd to me that you would use Tumblr and Hacker News as a sounding board for a security flaw rather than submitting a bug report or notifying the Rails team through existing mailing lists.
- nate 17y ago
  "Security vulnerabilities should be reported via an email to security@rubyonrails.org, do not use lighthouse for reporting security vulnerabilities. All content in lighthouse is publicly available as soon as it is posted." Yes, as I mention in the blog post, I've attempted to contact this security list and a couple members on the core team through their individual email accounts over a week ago. I've only received one response last Thursday that someone would look into it, but the issue seemed to die there. Now that enough time has been given for the security list to look into the problem (and hopefully not ignore it), the best practice I thought would be to tell as many people as possible about it so the fix can be applied and publicized. I felt I'd get a lot bigger audience here at Hacker news than the rails bug tracker. The bigger the audience the more people that can get their Rails 2.3 instances fixed if they are effected and avoid a problem. I was also planning on posting it there, but feel free to do it as well.
  - nate 17y ago
    Here's a ticket now though. https://rails.lighthouseapp.com/projects/8994/tickets/2755-security-hole-found-in-rails-23s-http_authenticationrb https://rails.lighthouseapp.com/projects/8994/tickets/2755-s... And I've also put it up on rubyflow.
  - tptacek 17y ago
    This is an absolutely classic indie/startup security fuckup. (1) Have a security contact, (2) publish a GPG key and accept GPG mail, (3) respond promptly with a "security flaw ID". You literally don't have to do anything more than this --- mechanically --- to get good-faith people not to publish flaws publicly.
bk 17y ago
Two thoughts: 1. It shows that tests are only as good as the thinking that goes into them, i.e., it's up the programmer to think through all the use cases, possibility for breakage, and use the appropriate tools to try a wide range of inputs. 2. This lack of responsiveness to the vulnerability reminds me of the complaints Zed (Shaw) made about the Rails Core back when he announced he was leaving the Ruby community. Preemptive disclaimer: These are not supposed to be snide remarks in any way, they're just two quick associations of a tired brain. So please no flames.
- tlrobinson 17y ago
  Indeed, back in January I came across a directory traversal vulnerability in Rack (http://blade.nagaokaut.ac.jp/cgi-bin/scat.rb/ruby/ruby-talk/324389 http://blade.nagaokaut.ac.jp/cgi-bin/scat.rb/ruby/ruby-talk/...). They had lots of unit tests, but none testing that particular case. Unit testing is certainly no magic bullet. On the other hand, the Rack team was incredibly responsive and put out an update within hours.
- ankhmoop 17y ago
  It's also an interesting example of where an Option/Maybe monad would save you. Coupled with a type system, the issue would even be caught at compile time (or edit-time, in the case of a type-aware editor/IDE).
oomkiller 17y ago
IMO, you should write your own code, instead of relying on someone else's possibly buggy code. If you are going to use somebody else's code, you should at LEAST look at it to make sure its correct, especially something as terse as this.
csbartus 17y ago
a sad story, comparing to the recent arc/hn security hacking: http://news.ycombinator.com/item?id=639976 http://news.ycombinator.com/item?id=639976 reading that post was delighting me: a serious bug, a fix added, a community supporting & being thankful. here at rails, a framework i would like to love it, a serious bug, no fix, a community throwing the shit over the fence and each other. this immediately reminds me of the recent pr0n story in rails, where some members of the community were offended, by whatever reasons, by other community members. the only good, non-technical advancement in rails in the last period i saw was the documentation project supported by the community and backed by rails eminents. i'm expecting a firm and strong support here from rails, either would it come from dhh, or anybody else. otherwise even if i would like to work with rails and build my life on it i can't afford a serious security hole being treated like this. ruby and almost in the same way rails are close to my heart. i'm always defending them in such cases, even making myself being too subjective. i can accept rails edginess, mac-alike metrosexualism (no ofense to anyone!), handsomeness and awesomeness, funky geekness BUT in some cases, like this, we should act normally, promptly, professionaly as any other community does. EDIT: the official rails core answer: http://groups.google.com/group/rubyonrails-security/browse_thread/thread/20e17a978d2ccbd3?hl=en http://groups.google.com/group/rubyonrails-security/browse_t... posted 10hrs ago this comment was made. it contains a bugfix, instructions and a disclosure i'm copying here: Disclosure Notes ==== Due to communication difficulties and a mis-understanding between the reporter and the security team. This vulnerability has been publicly disclosed on several websites, users are advised to update their applications immediately. Steps are being taken to ensure that the security email is more reliable in the future. We regret the nature of this disclosure and will endeavor to update processes and documentation to ensure it doesn’t happen again in the future.

83 points by nate 17y ago

Yep, looks real enough to me. Confusion between "nil" and "false" in Ruby, where an assumed true/false return value actually takes on a third value that also matches a nil password.

wglb 17y ago
Not being a ruby programmer, is there a defensive coding idiom that would have prevented this confusion?
- rcoder 17y ago
  You can always test for nil explicitly. Compare: unless foo # ... foo can be false or nil to reach this branch end unless foo == false # ... this branch is reached unless foo == false end Since terseness is considered such a cardinal virtue amongst Ruby programmers (including myself), however, this kind of issue is often ignored, and difficult to detect unless your tests explicitly check for "truthy" and "non-truthy" values in addition to the true/false literals.
  - wglb 17y ago
    Thanks.
  - danishgolkonda 17y ago
    tell me in detail
- ionfish 17y ago
  if object.nil? puts "Object is nil!" else puts "Object is not nil." end
- lsb 17y ago
  Haskell obviously isn't Ruby, but its Maybe monad, where you have Just the login, or Nothing, looks pretty appealing.

bryanwoods 17y ago

Interesting, I'm not getting this flaw at all. Which version of 2.3 is this addressing? Rails 2.3.1 only? I just tried this on a 2.3.2 app and got HTTP Basic: Access denied.

nate 17y ago
Rails 2.3.x. The app you tested this on, what's the password procedure like? One part of problem is doing a password procedure like "Users[name]" where Users is a hash of usernames/passwords. Also I believe this just effects folks doing digest auth (authenticate_or_request_with_http_digest) not basic auth (authenticate_or_request_with_http_basic)

kneath 17y ago

I'm curious, did you submit a bug report for this patch? I can't find it on http://rails.lighthouseapp.com http://rails.lighthouseapp.com. It seems odd to me that you would use Tumblr and Hacker News as a sounding board for a security flaw rather than submitting a bug report or notifying the Rails team through existing mailing lists.

nate 17y ago
"Security vulnerabilities should be reported via an email to security@rubyonrails.org, do not use lighthouse for reporting security vulnerabilities. All content in lighthouse is publicly available as soon as it is posted." Yes, as I mention in the blog post, I've attempted to contact this security list and a couple members on the core team through their individual email accounts over a week ago. I've only received one response last Thursday that someone would look into it, but the issue seemed to die there. Now that enough time has been given for the security list to look into the problem (and hopefully not ignore it), the best practice I thought would be to tell as many people as possible about it so the fix can be applied and publicized. I felt I'd get a lot bigger audience here at Hacker news than the rails bug tracker. The bigger the audience the more people that can get their Rails 2.3 instances fixed if they are effected and avoid a problem. I was also planning on posting it there, but feel free to do it as well.
- nate 17y ago
  Here's a ticket now though. https://rails.lighthouseapp.com/projects/8994/tickets/2755-security-hole-found-in-rails-23s-http_authenticationrb https://rails.lighthouseapp.com/projects/8994/tickets/2755-s... And I've also put it up on rubyflow.
- tptacek 17y ago
  This is an absolutely classic indie/startup security fuckup. (1) Have a security contact, (2) publish a GPG key and accept GPG mail, (3) respond promptly with a "security flaw ID". You literally don't have to do anything more than this --- mechanically --- to get good-faith people not to publish flaws publicly.

bk 17y ago

Two thoughts: 1. It shows that tests are only as good as the thinking that goes into them, i.e., it's up the programmer to think through all the use cases, possibility for breakage, and use the appropriate tools to try a wide range of inputs. 2. This lack of responsiveness to the vulnerability reminds me of the complaints Zed (Shaw) made about the Rails Core back when he announced he was leaving the Ruby community. Preemptive disclaimer: These are not supposed to be snide remarks in any way, they're just two quick associations of a tired brain. So please no flames.

tlrobinson 17y ago
Indeed, back in January I came across a directory traversal vulnerability in Rack (http://blade.nagaokaut.ac.jp/cgi-bin/scat.rb/ruby/ruby-talk/324389 http://blade.nagaokaut.ac.jp/cgi-bin/scat.rb/ruby/ruby-talk/...). They had lots of unit tests, but none testing that particular case. Unit testing is certainly no magic bullet. On the other hand, the Rack team was incredibly responsive and put out an update within hours.
ankhmoop 17y ago
It's also an interesting example of where an Option/Maybe monad would save you. Coupled with a type system, the issue would even be caught at compile time (or edit-time, in the case of a type-aware editor/IDE).

oomkiller 17y ago

IMO, you should write your own code, instead of relying on someone else's possibly buggy code. If you are going to use somebody else's code, you should at LEAST look at it to make sure its correct, especially something as terse as this.

csbartus 17y ago

a sad story, comparing to the recent arc/hn security hacking: http://news.ycombinator.com/item?id=639976 http://news.ycombinator.com/item?id=639976 reading that post was delighting me: a serious bug, a fix added, a community supporting & being thankful. here at rails, a framework i would like to love it, a serious bug, no fix, a community throwing the shit over the fence and each other. this immediately reminds me of the recent pr0n story in rails, where some members of the community were offended, by whatever reasons, by other community members. the only good, non-technical advancement in rails in the last period i saw was the documentation project supported by the community and backed by rails eminents. i'm expecting a firm and strong support here from rails, either would it come from dhh, or anybody else. otherwise even if i would like to work with rails and build my life on it i can't afford a serious security hole being treated like this. ruby and almost in the same way rails are close to my heart. i'm always defending them in such cases, even making myself being too subjective. i can accept rails edginess, mac-alike metrosexualism (no ofense to anyone!), handsomeness and awesomeness, funky geekness BUT in some cases, like this, we should act normally, promptly, professionaly as any other community does. EDIT: the official rails core answer: http://groups.google.com/group/rubyonrails-security/browse_thread/thread/20e17a978d2ccbd3?hl=en http://groups.google.com/group/rubyonrails-security/browse_t... posted 10hrs ago this comment was made. it contains a bugfix, instructions and a disclosure i'm copying here: Disclosure Notes ==== Due to communication difficulties and a mis-understanding between the reporter and the security team. This vulnerability has been publicly disclosed on several websites, users are advised to update their applications immediately. Steps are being taken to ensure that the security email is more reliable in the future. We regret the nature of this disclosure and will endeavor to update processes and documentation to ensure it doesn’t happen again in the future.