From 6fc84bd346ca5a20c2acf04da7aafcf5948aad11 Mon Sep 17 00:00:00 2001 From: Grant Cox Date: Tue, 8 Jul 2025 09:21:55 +1000 Subject: [PATCH 1/4] Fix race condition vulnerability, by ensuring the unconfirmed_email is always saved even if the submitted value is the same as the in-memory model's state --- lib/devise/models/confirmable.rb | 1 + test/integration/confirmable_test.rb | 25 +++++++++++++++++++++++++ 2 files changed, 26 insertions(+) diff --git a/lib/devise/models/confirmable.rb b/lib/devise/models/confirmable.rb index 6ce22c30f0..c920e7e323 100644 --- a/lib/devise/models/confirmable.rb +++ b/lib/devise/models/confirmable.rb @@ -262,6 +262,7 @@ def generate_confirmation_token! def postpone_email_change_until_confirmation_and_regenerate_confirmation_token @reconfirmation_required = true self.unconfirmed_email = self.email + unconfirmed_email_will_change! self.email = self.devise_email_in_database self.confirmation_token = nil generate_confirmation_token diff --git a/test/integration/confirmable_test.rb b/test/integration/confirmable_test.rb index 8e6f68ef2d..0f2c1a83a9 100644 --- a/test/integration/confirmable_test.rb +++ b/test/integration/confirmable_test.rb @@ -354,4 +354,29 @@ def visit_admin_confirmation_with_token(confirmation_token) assert_contain(/Email.*already.*taken/) assert admin.reload.pending_reconfirmation? end + + test 'concurrent "update email" requests should not allow confirmation_token and unconfirmed_email to get out of sync' do + attacker_email = "attacker@example.com" + victim_email = "victim@example.com" + + attacker = create_admin + # update the email address of the attacker, but do not confirm it yet + attacker.update(email: attacker_email) + + # a concurrent request also updates the email address to the victim, while this request's model is in memory + Admin.where(id: attacker.id).update_all( + unconfirmed_email: victim_email, + confirmation_token: "different token" + ) + + # now we update to the same prior unconfirmed email address, and confirm + attacker.update(email: attacker_email) + attacker_token = attacker.raw_confirmation_token + visit_admin_confirmation_with_token(attacker_token) + + attacker.reload + assert attacker.confirmed? + assert_equal attacker_email, attacker.email + end + end From f12a6c3fe7c1dec2868a9499c9203573c8f6dd9e Mon Sep 17 00:00:00 2001 From: Grant Cox Date: Fri, 8 Aug 2025 04:23:58 +1000 Subject: [PATCH 2/4] Improve the Confirmable race condition test Co-authored-by: Rune Philosof <57357936+runephilosof-abtion@users.noreply.github.com> --- test/integration/confirmable_test.rb | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/test/integration/confirmable_test.rb b/test/integration/confirmable_test.rb index 0f2c1a83a9..a909dc6449 100644 --- a/test/integration/confirmable_test.rb +++ b/test/integration/confirmable_test.rb @@ -355,22 +355,26 @@ def visit_admin_confirmation_with_token(confirmation_token) assert admin.reload.pending_reconfirmation? end - test 'concurrent "update email" requests should not allow confirmation_token and unconfirmed_email to get out of sync' do + test 'concurrent "update email" requests should not allow confirming a victim email address' do attacker_email = "attacker@example.com" victim_email = "victim@example.com" attacker = create_admin # update the email address of the attacker, but do not confirm it yet - attacker.update(email: attacker_email) + attacker.update!(email: attacker_email) + + # A new request starts, to update the unconfirmed email again. + attacker = Admin.find_by(id: attacker.id) - # a concurrent request also updates the email address to the victim, while this request's model is in memory + # A concurrent request also updates the email address to the victim, while the `attacker` request's model is in memory Admin.where(id: attacker.id).update_all( unconfirmed_email: victim_email, confirmation_token: "different token" ) - # now we update to the same prior unconfirmed email address, and confirm - attacker.update(email: attacker_email) + # Now the attacker updates to the same prior unconfirmed email address, and confirm. + # This should update the `unconfirmed_email` in the database, even though it is unchanged from the models point of view. + attacker.update!(email: attacker_email) attacker_token = attacker.raw_confirmation_token visit_admin_confirmation_with_token(attacker_token) From 647521ac03ca29f21632cad9aaa4634a3acd0900 Mon Sep 17 00:00:00 2001 From: Carlos Antonio da Silva Date: Fri, 6 Mar 2026 11:52:21 -0300 Subject: [PATCH 3/4] Apply ORM-specific fix for mongoid tests to pass It appears Mongoid differs from AR in that even if you're calling `will_change!` on an attribute to "force" it to see (and persist) changes, and it gets set to `changed_attributes` so it knows it's technically "changed", the model's `changes` which are based on `attribute_changed?` don't include the field because it didn't actually "change" from its original value, so it doesn't get persisted by Mongoid. This was causing the new test for the confirmable race condition issue to fail, because `will_change!` wasn't forcing the "change" to be persisted in Mongoid. This is a workaround for it, by setting `changed_attributes` directly to `nil` (could be any value like `__devise-force-change__`) it makes Mongoid see the changes and persist them. --- lib/devise/models/confirmable.rb | 5 +++-- lib/devise/orm.rb | 11 +++++++++++ test/integration/confirmable_test.rb | 3 +-- 3 files changed, 15 insertions(+), 4 deletions(-) diff --git a/lib/devise/models/confirmable.rb b/lib/devise/models/confirmable.rb index c920e7e323..1930086aaa 100644 --- a/lib/devise/models/confirmable.rb +++ b/lib/devise/models/confirmable.rb @@ -258,11 +258,12 @@ def generate_confirmation_token! generate_confirmation_token && save(validate: false) end - def postpone_email_change_until_confirmation_and_regenerate_confirmation_token @reconfirmation_required = true + # Force unconfirmed_email to be updated, even if the value hasn't changed, to prevent a + # race condition which could allow an attacker to confirm an email they don't own. See #5783. + devise_unconfirmed_email_will_change! self.unconfirmed_email = self.email - unconfirmed_email_will_change! self.email = self.devise_email_in_database self.confirmation_token = nil generate_confirmation_token diff --git a/lib/devise/orm.rb b/lib/devise/orm.rb index 3f3ac86db7..f00f397f05 100644 --- a/lib/devise/orm.rb +++ b/lib/devise/orm.rb @@ -35,6 +35,10 @@ def devise_will_save_change_to_email? will_save_change_to_email? end + def devise_unconfirmed_email_will_change! + unconfirmed_email_will_change! + end + def devise_respond_to_and_will_save_change_to_attribute?(attribute) respond_to?("will_save_change_to_#{attribute}?") && send("will_save_change_to_#{attribute}?") end @@ -61,6 +65,13 @@ def devise_will_save_change_to_email? email_changed? end + def devise_unconfirmed_email_will_change! + # Mongoid's will_change! doesn't force unchanged attributes into updates, + # so we override changed_attributes to make it see a difference. + unconfirmed_email_will_change! + changed_attributes["unconfirmed_email"] = nil + end + def devise_respond_to_and_will_save_change_to_attribute?(attribute) respond_to?("#{attribute}_changed?") && send("#{attribute}_changed?") end diff --git a/test/integration/confirmable_test.rb b/test/integration/confirmable_test.rb index a909dc6449..f9185e87f3 100644 --- a/test/integration/confirmable_test.rb +++ b/test/integration/confirmable_test.rb @@ -362,7 +362,7 @@ def visit_admin_confirmation_with_token(confirmation_token) attacker = create_admin # update the email address of the attacker, but do not confirm it yet attacker.update!(email: attacker_email) - + # A new request starts, to update the unconfirmed email again. attacker = Admin.find_by(id: attacker.id) @@ -382,5 +382,4 @@ def visit_admin_confirmation_with_token(confirmation_token) assert attacker.confirmed? assert_equal attacker_email, attacker.email end - end From 3be4fa3158bac06789514629db686734b44994a6 Mon Sep 17 00:00:00 2001 From: Carlos Antonio da Silva Date: Mon, 16 Mar 2026 17:36:01 -0300 Subject: [PATCH 4/4] Add changelog [ci skip] --- CHANGELOG.md | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 14fc8bbd4c..4f8ea035fd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,8 @@ +### Unreleased + +* security fixes + * Fix race condition vulnerability on confirmable "change email" which would allow confirming an email they don't own [#5783](https://github.com/heartcombo/devise/pull/5783) [#5784](https://github.com/heartcombo/devise/pull/5784) + ### 5.0.2 - 2026-02-18 * enhancements