Skip to content

Fix email enumeration vulnerabilities in password reset and registration flows #5790

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
natevick wants to merge 3 commits into
base: main
Choose a base branch
from
+141 −33
Open

Fix email enumeration vulnerabilities in password reset and registration flows #5790

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
0825706
Fix email enumeration vulnerability in password reset
natevick Aug 14, 2025
bf27e2f
Fix email enumeration in registration flow
natevick Aug 14, 2025
c85eb3e
Add timing attack mitigation for password reset
natevick Aug 14, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 3 additions & 5 deletions app/controllers/devise/passwords_controller.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,11 +15,9 @@ def create
self.resource = resource_class.send_reset_password_instructions(resource_params)
yield resource if block_given?

if successfully_sent?(resource)
respond_with({}, location: after_sending_reset_password_instructions_path_for(resource_name))
else
respond_with(resource)
end
# Always show success message to prevent email enumeration
set_flash_message! :notice, :send_instructions
respond_with({}, location: after_sending_reset_password_instructions_path_for(resource_name))
end

# GET /resource/password/edit?reset_password_token=abcdef
Expand Down
30 changes: 30 additions & 0 deletions app/controllers/devise/registrations_controller.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,32 @@ def new
def create
build_resource(sign_up_params)

# Check if email already exists when in paranoid mode
if Devise.paranoid
# Apply same transformations as the model would
email_key = resource_class.authentication_keys.first
email_value = resource.send(email_key)

# Apply case insensitive search if configured
if resource_class.case_insensitive_keys.include?(email_key)
existing_user = resource_class.where("lower(#{email_key}) = ?", email_value.downcase).first
else
existing_user = resource_class.find_by(email_key => email_value)
end

if existing_user
# Send "already registered" email instead of showing error
devise_mailer.email_already_registered(existing_user).deliver

# Show same success message as normal registration with confirmation
set_flash_message! :notice, :signed_up_but_unconfirmed
expire_data_after_sign_in!
redirect_to new_session_path(resource_name)
return
end
end

# Normal registration flow
resource.save
yield resource if block_given?
if resource.persisted?
Expand Down Expand Up @@ -82,6 +108,10 @@ def cancel

protected

def devise_mailer
Devise.mailer
end

def update_needs_confirmation?(resource, previous)
resource.respond_to?(:pending_reconfirmation?) &&
resource.pending_reconfirmation? &&
Expand Down
4 changes: 4 additions & 0 deletions app/mailers/devise/mailer.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,5 +26,9 @@ def email_changed(record, opts = {})
def password_change(record, opts = {})
devise_mail(record, :password_change, opts)
end

def email_already_registered(record, opts = {})
devise_mail(record, :email_already_registered, opts)
end
end
end
11 changes: 11 additions & 0 deletions app/views/devise/mailer/email_already_registered.html.erb
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
<p>Hello <%= @resource.email %>!</p>

<p>Someone tried to create an account using your email address, but an account already exists.</p>

<p>If this was you, you can:</p>
<ul>
<li><%= link_to "Sign in to your account", new_session_url(@resource, @scope_name) %></li>
<li><%= link_to "Reset your password", new_password_url(@resource, @scope_name) %> if you've forgotten it</li>
</ul>

<p>If this wasn't you, you can safely ignore this email. Your account is secure.</p>
4 changes: 3 additions & 1 deletion config/locales/en.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,12 +27,14 @@ en:
subject: "Email Changed"
password_change:
subject: "Password Changed"
email_already_registered:
subject: "Account already exists"
omniauth_callbacks:
failure: "Could not authenticate you from %{kind} because \"%{reason}\"."
success: "Successfully authenticated from %{kind} account."
passwords:
no_token: "You can't access this page without coming from a password reset email. If you do come from a password reset email, please make sure you used the full URL provided."
send_instructions: "You will receive an email with instructions on how to reset your password in a few minutes."
send_instructions: "If your email address exists in our database, you will receive a password recovery link at your email address in a few minutes."
send_paranoid_instructions: "If your email address exists in our database, you will receive a password recovery link at your email address in a few minutes."
updated: "Your password has been changed successfully. You are now signed in."
updated_not_active: "Your password has been changed successfully."
Expand Down
16 changes: 14 additions & 2 deletions lib/devise/models/recoverable.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -118,11 +118,23 @@ def with_reset_password_token(token)

# Attempt to find a user by its email. If a record is found, send new
# password instructions to it. If user is not found, returns a new user
# with an email not found error.
# with no errors to prevent email enumeration attacks.
# Attributes must contain the user's email
def send_reset_password_instructions(attributes = {})
recoverable = find_or_initialize_with_errors(reset_password_keys, attributes, :not_found)
recoverable.send_reset_password_instructions if recoverable.persisted?

if recoverable.persisted?
recoverable.send_reset_password_instructions
else
# Perform similar work to mitigate timing attacks
Copy link

@chaadow chaadow Dec 20, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i like the idea and i was about to do the same. But to properly simulate, we need to also simulate sending an email

I had the same idea as @gregmolnar in his comment about the sign up strategy. Basically here is the flow:

  • I take whatever params[:email] the user sends.
  • then I don't even check for its existence, I just do a MaybeSendResetPasswordInstructions.perform_later(params[:email])
  • And I redirect the user with a basic notice message "if your email exists in our database you ll get an email"

then obviously my job delegates to devise's Recoveral send_reset_password_instructions API.

The issue here is that the email sending is done in a synchronous way, so it's either a POST request to an email provider such as Sendgrid, or an SMTP call ( basically a network call )

So here i dont know maybe do sleep 0.5 or something. Or add a config in Recoverable which is config.send_emails_in_background or similar. if it's true, then we queue a job, if not we sleep for a fixed amount to simulate a network call

Copy link

@chaadow chaadow Dec 20, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let me know what you think, but this timing issue was reported by a pentester as well, and i was shocked about the difference between 40ms when the user does not exist, and 600ms to 1000ms when it exists.

I directly remembered that Devise NEVER sends their email in a background job ( I believe there is an open issue about that )

Copy link

@chaadow chaadow Dec 20, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

probably this could help if it gets merged soon: #5610

Copy link
Author

@natevick natevick Dec 24, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm open to making changes, my biggest concern is that the work will never get merged.

# Generate a token even though we won't use it
Devise.friendly_token(20)

Copy link

@chaadow chaadow Dec 20, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think what's missing is to simulate up to 50ms for an UPDATE statement that is done here:
https://github.com/heartcombo/devise/pull/5790/changes#diff-32eda6752ef9bcf49e99b0bc6be379576f4ba872944770a3ef8c66abd2d84b0bR92-R94

# Always return a new user with no errors to prevent email enumeration
recoverable = new
recoverable.errors.clear
end

recoverable
end

Expand Down
2 changes: 2 additions & 0 deletions mise.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
[tools]
ruby = "3.1.6"
42 changes: 20 additions & 22 deletions test/integration/recoverable_test.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ def reset_password(options = {}, &block)
end

assert_current_url '/users/sign_in'
assert_contain 'You will receive an email with instructions on how to reset your password in a few minutes.'
assert_contain 'If your email address exists in our database'
end

test 'reset password with email should send an email from a custom mailer' do
Expand All @@ -68,18 +68,17 @@ def reset_password(options = {}, &block)
assert_match edit_user_password_path(reset_password_token: 'abcdef'), mail.body.encoded
end

test 'reset password with email of different case should fail when email is NOT the list of case insensitive keys' do
test 'reset password with email of different case should show success message when email is NOT the list of case insensitive keys' do
swap Devise, case_insensitive_keys: [] do
create_user(email: 'Foo@Bar.com')

request_forgot_password do
fill_in 'email', with: 'foo@bar.com'
end

assert_response :success
assert_current_url '/users/password'
assert_have_selector "input[type=email][value='foo@bar.com']"
assert_contain 'not found'
# Now always shows success to prevent email enumeration
assert_current_url '/users/sign_in'
assert_contain 'If your email address exists in our database'
end
end

Expand All @@ -91,21 +90,20 @@ def reset_password(options = {}, &block)
end

assert_current_url '/users/sign_in'
assert_contain 'You will receive an email with instructions on how to reset your password in a few minutes.'
assert_contain 'If your email address exists in our database'
end

test 'reset password with email with extra whitespace should fail when email is NOT the list of strip whitespace keys' do
test 'reset password with email with extra whitespace should show success message when email is NOT the list of strip whitespace keys' do
swap Devise, strip_whitespace_keys: [] do
create_user(email: 'foo@bar.com')

request_forgot_password do
fill_in 'email', with: ' foo@bar.com '
end

assert_response :success
assert_current_url '/users/password'
assert_have_selector "input[type=email][value=' foo@bar.com ']"
assert_contain 'not found'
# Now always shows success to prevent email enumeration
assert_current_url '/users/sign_in'
assert_contain 'If your email address exists in our database'
end
end

Expand All @@ -124,18 +122,17 @@ def reset_password(options = {}, &block)
request_forgot_password

assert_current_url '/users/sign_in'
assert_contain 'You will receive an email with instructions on how to reset your password in a few minutes.'
assert_contain 'If your email address exists in our database'
end

test 'not authenticated user with invalid email should receive an error message' do
test 'not authenticated user with invalid email should still see success message' do
request_forgot_password do
fill_in 'email', with: 'invalid.test@test.com'
end

assert_response :success
assert_current_url '/users/password'
assert_have_selector "input[type=email][value='invalid.test@test.com']"
assert_contain 'not found'
# Now always shows success to prevent email enumeration
assert_current_url '/users/sign_in'
assert_contain 'If your email address exists in our database'
end

test 'authenticated user should not be able to visit edit password page' do
Expand Down Expand Up @@ -293,14 +290,15 @@ def reset_password(options = {}, &block)
assert_equal({}.to_json, response.body)
end

test 'reset password request with invalid e-mail in JSON format should return valid response' do
test 'reset password request with invalid e-mail in JSON format should return success response' do
create_user
post user_password_path(format: 'json'), params: { user: {email: "invalid.test@test.com"} }
assert_response :unprocessable_entity
assert_includes response.body, '{"errors":{'
# Now always returns success to prevent email enumeration
assert_response :success
assert_equal({}.to_json, response.body)
end

test 'reset password request with invalid e-mail in JSON format should return empty and valid response in paranoid mode' do
test 'reset password request with invalid e-mail in JSON format also returns success in paranoid mode' do
swap Devise, paranoid: true do
create_user
post user_password_path(format: 'json'), params: { user: {email: "invalid@test.com"} }
Expand Down
25 changes: 25 additions & 0 deletions test/integration/registerable_test.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -138,6 +138,31 @@ def user_sign_up
assert_not warden.authenticated?(:user)
end

test 'in paranoid mode, duplicate registration shows success message and sends email to existing user' do
swap Devise, paranoid: true do
create_user
get new_user_registration_path

fill_in 'email', with: 'user@test.com'
fill_in 'password', with: '123456'
fill_in 'password confirmation', with: '123456'

assert_email_sent 'user@test.com' do
click_button 'Sign up'
end

# Should redirect to sign in page with success message
assert_current_url '/users/sign_in'
assert_contain 'A message with a confirmation link has been sent to your email address'
assert_not warden.authenticated?(:user)

# Check that the email sent is the "already registered" email
mail = ActionMailer::Base.deliveries.last
assert_equal 'Account already exists', mail.subject
assert_match 'Someone tried to create an account using your email address', mail.body.encoded
end
end

test 'a guest should not be able to change account' do
get edit_user_registration_path
assert_redirected_to new_user_session_path
Expand Down
32 changes: 29 additions & 3 deletions test/models/recoverable_test.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -119,10 +119,11 @@ def setup
assert_equal user, reset_password_user
end

test 'should return a new record with errors if user was not found by e-mail' do
test 'should return a new record with no errors if user was not found by e-mail' do
reset_password_user = User.send_reset_password_instructions(email: "invalid@example.com")
assert_not reset_password_user.persisted?
assert_equal "not found", reset_password_user.errors[:email].join
# No longer exposes email not found to prevent enumeration
assert reset_password_user.errors.empty?
end

test 'should find a user to send instructions by authentication_keys' do
Expand All @@ -138,7 +139,8 @@ def setup
user = create_user
reset_password_user = User.send_reset_password_instructions(email: user.email)
assert_not reset_password_user.persisted?
assert reset_password_user.errors.added?(:username, :blank)
# No longer shows specific errors to prevent enumeration
assert reset_password_user.errors.empty?
end
end

Expand Down Expand Up @@ -260,4 +262,28 @@ def setup
assert_nil User.with_reset_password_token('random-token')
end

test 'should not expose email existence by default' do
user = User.send_reset_password_instructions(email: 'nonexistent@example.com')
assert_not user.persisted?
assert user.errors.empty?
end

test 'should send reset instructions only for existing users' do
existing_user = create_user

# Should send email for existing user
assert_email_sent do
user = User.send_reset_password_instructions(email: existing_user.email)
assert user.persisted?
assert user.errors.empty?
end

# Should not send email for non-existing user
assert_email_not_sent do
user = User.send_reset_password_instructions(email: 'nonexistent@example.com')
assert_not user.persisted?
assert user.errors.empty?
end
end

end