This project is archived and is in readonly mode.

#2755 ✓resolved
nate (at inklingmarkets)

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:

http://authbroken.heroku.com/

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

    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

    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>

Pages