Permalink
Browse files

fix for possible injection with mongo

Signed-off-by: José Valim <jose.valim@gmail.com>
  • Loading branch information...
1 parent 569d205 commit 1982ad9f572bf4d1da9eb685a418459d4dfecfba gilles committed with josevalim Mar 7, 2011
@@ -100,6 +100,12 @@ def http_authenticatable?(strategy)
# end
#
def find_for_authentication(conditions)
+ #the to_s is here to avoid mongodb injection where 'field => value' becomes 'field => {$ne => value}' thourgh the magic of rails
+ #still this does not prevent the leak if user1.token == '$ne' + user2.token (the chance of that is poor though)
+ #this might not be the best place or the best method, please change
+ conditions.each do |k, v|
+ conditions[k] = v.to_s
+ end
case_insensitive_keys.each { |k| conditions[k].try(:downcase!) }
to_adapter.find_first(conditions)
end
@@ -89,6 +89,18 @@ class TokenAuthenticationTest < ActionController::IntegrationTest
end
end
+ test 'should not be subject to injection' do
+ swap Devise, :token_authentication_key => :secret_token do
+ user1 = create_user_with_authentication_token()
+ user2 = create_another_user_with_authentication_token(:auth_token => "ANOTHERTOKEN")
+
+ visit users_path(Devise.token_authentication_key.to_s + '[$ne]' => user1.authentication_token)
+
+ assert warden.user(:user) == nil
+
+ end
+ end
+
private
def sign_in_as_new_user_with_token(options = {})
@@ -107,14 +119,32 @@ def sign_in_as_new_user_with_token(options = {})
user
end
- def create_user_with_authentication_token(options)
+ def create_user_with_authentication_token(options = {})
user = create_user(options)
- user.authentication_token = VALID_AUTHENTICATION_TOKEN
+ user.authentication_token = options[:auth_token] || VALID_AUTHENTICATION_TOKEN
user.save
user
end
+ def create_another_user_with_authentication_token(options = {})
+ @anotheruser ||= begin
+ user = User.create!(
+ :username => 'anotherusertest',
+ :email => options[:email] || 'anotheruser@test.com',
+ :password => options[:password] || '123456',
+ :password_confirmation => options[:password] || '123456',
+ :created_at => Time.now.utc
+ )
+ user.confirm! unless options[:confirm] == false
+ user.lock_access! if options[:locked] == true
+ user.authentication_token = options[:auth_token] || VALID_AUTHENTICATION_TOKEN
+ user.save
+ user
+ end
+ end
+
def get_users_path_as_existing_user(user)
sign_in_as_new_user_with_token(:user => user)
end
+
end
@@ -28,7 +28,7 @@ class TokenAuthenticatableTest < ActiveSupport::TestCase
test 'should return nil when authenticating an invalid user by authentication token' do
if DEVISE_ORM == :mongoid
- raise 'There is an incompatibility between Devise and Mongoid' <<
+ raise 'There is an incompatibility between Devise and Mongoid' <<
' that makes this test break. For more information, check' <<
' this issue: https://github.com/mongoid/mongoid/issues/725'
end
@@ -40,4 +40,26 @@ class TokenAuthenticatableTest < ActiveSupport::TestCase
assert_nil authenticated_user
end
+ test 'should not be subject to injection' do
+
+ if DEVISE_ORM != :mongoid
+ assert_nil(nil)
+ else
+
+ user1 = create_user
+ user1.ensure_authentication_token!
+ user1.confirm!
+
+ user2 = create_user
+ user2.ensure_authentication_token!
+ user2.confirm!
+
+ #now trick it
+ user = User.find_for_token_authentication(:auth_token => {'$ne' => user1.authentication_token})
+
+ assert_nil user
+ end
+
+ end
+
end

0 comments on commit 1982ad9

Please sign in to comment.