From 0b5c3f5a037561943cb0510c224f390f55ae600a Mon Sep 17 00:00:00 2001 From: Sam Date: Tue, 9 Aug 2016 10:02:18 +1000 Subject: [PATCH] SECURITY: do cookie auth rate limiting earlier --- lib/auth/default_current_user_provider.rb | 8 ++++++-- .../auth/default_current_user_provider_spec.rb | 10 ++++++++++ 2 files changed, 16 insertions(+), 2 deletions(-) diff --git a/lib/auth/default_current_user_provider.rb b/lib/auth/default_current_user_provider.rb index 066f55d2c1c..d726be453aa 100644 --- a/lib/auth/default_current_user_provider.rb +++ b/lib/auth/default_current_user_provider.rb @@ -38,14 +38,18 @@ class Auth::DefaultCurrentUserProvider current_user = nil if auth_token && auth_token.length == 32 - current_user = User.where(auth_token: auth_token) + limiter = RateLimiter.new(nil, "cookie_auth_#{request.ip}", COOKIE_ATTEMPTS_PER_MIN ,60) + + if limiter.can_perform? + current_user = User.where(auth_token: auth_token) .where('auth_token_updated_at IS NULL OR auth_token_updated_at > ?', SiteSetting.maximum_session_age.hours.ago) .first + end unless current_user begin - RateLimiter.new(nil, "cookie_auth_#{request.ip}", COOKIE_ATTEMPTS_PER_MIN ,60).performed! + limiter.performed! rescue RateLimiter::LimitExceeded raise Discourse::InvalidAccess end diff --git a/spec/components/auth/default_current_user_provider_spec.rb b/spec/components/auth/default_current_user_provider_spec.rb index 3fecd09e565..dd69a375b35 100644 --- a/spec/components/auth/default_current_user_provider_spec.rb +++ b/spec/components/auth/default_current_user_provider_spec.rb @@ -86,6 +86,10 @@ describe Auth::DefaultCurrentUserProvider do end it "can only try 10 bad cookies a minute" do + + user = Fabricate(:user) + provider('/').log_on_user(user, {}, {}) + RateLimiter.stubs(:disabled?).returns(false) RateLimiter.new(nil, "cookie_auth_10.0.0.1", 10, 60).clear! @@ -97,10 +101,16 @@ describe Auth::DefaultCurrentUserProvider do 10.times do provider('/', env).current_user end + expect { provider('/', env).current_user }.to raise_error(Discourse::InvalidAccess) + expect { + env["HTTP_COOKIE"] = "_t=#{user.auth_token}" + provider("/", env).current_user + }.to raise_error(Discourse::InvalidAccess) + env["REMOTE_ADDR"] = "10.0.0.2" provider('/', env).current_user end