This project is archived and is in readonly mode.
Security hole found in Rails 2.3's http_authentication.rb
Reported by nate (at inklingmarkets) | June 3rd, 2009 @ 09:22 PM | in 2.x
A hole that I believe could be a MAJOR deal for anyone using digest authentication in Rails 2.3 with the new http_authentication.rb code and who followed the simple Digest example from the rdoc, or the blog entry introducing it.
http://weblog.rubyonrails.org/2009/1/2/this-week-in-edge-rails
The hole would allow anyone to get to a protected page simply by sending a nil username/password with the request or an “incorrect” username and no password.
In other words, whatever you were trying to protect using this is completely exposed.
If you are effected, you should immediately patch (patch
included below) your Rails 2.3 installation or change your password
procedure.
To provide a concrete example:
Don’t even type a username and password. Just hit enter. You shouldn’t have gotten to that protected page, but you did.
———————
This is based on the example from the rails blog,
(which is based on a password_procedure also found in the rdoc and tests for this):
class PostsController < ApplicationController
Users = {“dhh” => “secret”}
before_filter :authenticate
def index
render :text => “You needed a password to see this…”
end
private
def authenticate
realm = “Application”
authenticate_or_request_with_http_digest(realm) do |name|
Users[name]
end
end
end
———————
Another concrete example is the test case I’ve included in the patch:
test “authentication request with nil credentials” do
@request.env[‘HTTP_AUTHORIZATION’] = encode_credentials(:username => nil, :password => nil)
get :index
assert_response :unauthorized
assert_equal “HTTP Digest: Access denied.\n”, @response.body, “Authentication didn’t fail for request”
assert_not_equal ‘Hello Secret’, @response.body, “Authentication didn’t fail for request”
end
This test does not pass against Rails 2.3.2. The response is a 200, which means Rails 2.3 lets a user with a nil username and password to access the protected :index action.
———————-
The problem stems from the example in the documentation:
def authenticate
authenticate_or_request_with_http_digest(REALM) do |username|
USERS[username]
end
end
If a request comes in with a nil username (for example from http_digest_authentication_test.rb: @request.env[‘HTTP_AUTHORIZATION’] = encode_credentials(:username => nil, :password => nil))
USERS[nil] is going to return nil.
The documentation after this example in the rdoc, states: “Returning +nil+ will cause authentication to fail.” But that isn’t what happens. Nil is returned from the password procedure (USERS[nil]) and then proceeds to match the nil password that was sent in with the request. So the validate method succeeds even though no user was actually matched against.
————————————
This patch also contains the fix I believe I recommend based on the behavior the documentation stated would happen. In other words, since the documentation states that the password validation should fail if the password procedure returns nil, I’ve changed the validate_digest_response to return false if the password_procedure returns nil.
https://gist.github.com/d5fa1ce8be9dfe0d9d19
For the record, I’ve attempted to contact the Ruby on Rails security list as well as a couple members on the core team, but no action has yet been taken to resolve the issue. I felt this was big enough deal that people should be alerted without further delay.
Comments and changes to this ticket
-
Gregg Kellogg June 6th, 2009 @ 06:04 PM
Note that authenticate_or_request_http_digest can take either the clear-text password or the HA1 digest hash of the username and password: MD5(username:realm:password) (see icket #2209: https://rails.lighthouseapp.com/projects/8994/tickets/2209-patch-3-... ).
There were some email discussions at the time to obsolete the clear-text password altogther, but it was too late in the release processes to make this change.
My opinion was that HA1 should be exposed as an api and at least used as a recommended practice. Better still would be to obsolete the use of the clear-text password altogether.
-
CancelProfileIsBroken August 7th, 2009 @ 01:53 PM
- State changed from new to resolved
fixed in 056ddbdcfb07f0b5c7e6ed8a35f6c3b55b4ab489
Create your profile
Help contribute to this project by taking a few moments to create your personal profile. Create your profile »
<h2 style="font-size: 14px">Tickets have moved to Github</h2>
The new ticket tracker is available at <a href="https://github.com/rails/rails/issues">https://github.com/rails/rails/issues</a>