SECURITY: Prefix session key and validate token format.

app/controllers/users_controller.rb

@@ -205,14 +205,18 @@ class UsersController < ApplicationController
   def password_reset
     expires_now()
 
+    if EmailToken.valid_token_format?(params[:token])
       @user = EmailToken.confirm(params[:token])
 
       if @user
-      session[params[:token]] = @user.id
+        session["password-#{params[:token]}"] = @user.id
       else
-      user_id = session[params[:token]]
+        user_id = session["password-#{params[:token]}"]
         @user = User.find(user_id) if user_id
       end
+    else
+      @invalid_token = true
+    end
 
     if !@user
       flash[:error] = I18n.t('password_reset.no_token')

app/models/email_token.rb

@@ -39,9 +39,12 @@ class EmailToken < ActiveRecord::Base
     SecureRandom.hex(EmailToken.token_length)
   end
 
+  def self.valid_token_format?(token)
+    return token.present? && token =~ /[a-f0-9]{#{token.length/2}}/i
+  end
+
   def self.confirm(token)
-    return unless token.present?
-    return unless token.length/2 == EmailToken.token_length
+    return unless valid_token_format?(token)
 
     email_token = EmailToken.where("token = ? and expired = FALSE AND ((NOT confirmed AND created_at >= ?) OR (confirmed AND created_at >= ?))", token, EmailToken.valid_after, EmailToken.confirm_valid_after).includes(:user).first
     return if email_token.blank?

spec/controllers/users_controller_spec.rb

@@ -243,7 +243,7 @@ describe UsersController do
       end
     end
 
-    context 'invalid token' do
+    context 'missing token' do
       before do
         get :password_reset, token: SecureRandom.hex
       end
@@ -251,9 +251,22 @@ describe UsersController do
       it 'disallows login' do
         flash[:error].should be_present
         session[:current_user_id].should be_blank
+        assigns[:invalid_token].should be_nil
         response.should be_success
       end
+    end
 
+    context 'invalid token' do
+      before do
+        get :password_reset, token: "evil_trout!"
+      end
+
+      it 'disallows login' do
+        flash[:error].should be_present
+        session[:current_user_id].should be_blank
+        assigns[:invalid_token].should be_true
+        response.should be_success
+      end
     end
 
     context 'valid token' do
@@ -269,18 +282,19 @@ describe UsersController do
     end
 
     context 'submit change' do
+      let(:token) { EmailToken.generate_token }
       before do
-        EmailToken.expects(:confirm).with('asdfasdf').returns(user)
+        EmailToken.expects(:confirm).with(token).returns(user)
       end
 
       it "logs in the user" do
-        put :password_reset, token: 'asdfasdf', password: 'newpassword'
+        put :password_reset, token: token, password: 'newpassword'
         session[:current_user_id].should be_present
       end
 
       it "doesn't log in the user when not approved" do
         SiteSetting.expects(:must_approve_users?).returns(true)
-        put :password_reset, token: 'asdfasdf', password: 'newpassword'
+        put :password_reset, token: token, password: 'newpassword'
         session[:current_user_id].should be_blank
       end
     end