SECURITY: limit bad cookie auth attempts

- Also cleans up the _t cookie if it is invalid

lib/auth/default_current_user_provider.rb

@@ -1,4 +1,5 @@
 require_dependency "auth/current_user_provider"
+require_dependency "rate_limiter"
 
 class Auth::DefaultCurrentUserProvider
 
@@ -7,6 +8,7 @@ class Auth::DefaultCurrentUserProvider
   API_KEY_ENV ||= "_DISCOURSE_API".freeze
   TOKEN_COOKIE ||= "_t".freeze
   PATH_INFO ||= "PATH_INFO".freeze
+  COOKIE_ATTEMPTS_PER_MIN ||= 10
 
   # do all current user initialization here
   def initialize(env)
@@ -40,6 +42,14 @@ class Auth::DefaultCurrentUserProvider
                          .where('auth_token_updated_at IS NULL OR auth_token_updated_at > ?',
                                   SiteSetting.maximum_session_age.hours.ago)
                          .first
+
+      unless current_user
+        begin
+          RateLimiter.new(nil, "cookie_auth_#{request.ip}", COOKIE_ATTEMPTS_PER_MIN ,60).performed!
+        rescue RateLimiter::LimitExceeded
+          raise Discourse::InvalidAccess
+        end
+      end
     end
 
     if current_user && (current_user.suspended? || !current_user.active)
@@ -69,6 +79,9 @@ class Auth::DefaultCurrentUserProvider
       user.update_column(:auth_token_updated_at, Time.zone.now)
       cookies[TOKEN_COOKIE] = { value: user.auth_token, httponly: true, expires: SiteSetting.maximum_session_age.hours.from_now }
     end
+    if !user && cookies.key?(TOKEN_COOKIE)
+      cookies.delete(TOKEN_COOKIE)
+    end
   end
 
   def log_on_user(user, session, cookies)
@@ -107,12 +120,12 @@ class Auth::DefaultCurrentUserProvider
 
       if user.admin && defined?(Rack::MiniProfiler)
         # clear the profiling cookie to keep stuff tidy
-        cookies["__profilin"] = nil
+        cookies.delete("__profilin")
       end
 
       user.logged_out
     end
-    cookies[TOKEN_COOKIE] = nil
+    cookies.delete(TOKEN_COOKIE)
   end
 
 

spec/components/auth/default_current_user_provider_spec.rb

@@ -85,6 +85,34 @@ describe Auth::DefaultCurrentUserProvider do
     expect(user.auth_token_updated_at - Time.now).to eq(0)
   end
 
+  it "can only try 10 bad cookies a minute" do
+    RateLimiter.stubs(:disabled?).returns(false)
+
+    RateLimiter.new(nil, "cookie_auth_10.0.0.1", 10, 60).clear!
+    RateLimiter.new(nil, "cookie_auth_10.0.0.2", 10, 60).clear!
+
+    ip = "10.0.0.1"
+    env = { "HTTP_COOKIE" => "_t=#{SecureRandom.hex}", "REMOTE_ADDR" => ip }
+
+    10.times do
+      provider('/', env).current_user
+    end
+    expect {
+      provider('/', env).current_user
+    }.to raise_error(Discourse::InvalidAccess)
+
+    env["REMOTE_ADDR"] = "10.0.0.2"
+    provider('/', env).current_user
+  end
+
+  it "correctly removes invalid cookies" do
+
+    cookies = {"_t" => "BAAAD"}
+    provider('/').refresh_session(nil, {}, cookies)
+
+    expect(cookies.key?("_t")).to eq(false)
+  end
+
   it "recycles existing auth_token correctly" do
     SiteSetting.maximum_session_age = 3
     user = Fabricate(:user)