Fix race condition vulnerability, by ensuring the unconfirmed_email is always saved

lib/devise/models/confirmable.rb

@@ -258,9 +258,11 @@ 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
           self.email = self.devise_email_in_database
           self.confirmation_token = nil

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

test/integration/confirmable_test.rb

@@ -354,4 +354,32 @@ 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 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)
+
+    # 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 the `attacker` request's model is in memory
+    Admin.where(id: attacker.id).update_all(
+      unconfirmed_email: victim_email,
+      confirmation_token: "different token"
+    )
+
+    # 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)
+
+    attacker.reload
+    assert attacker.confirmed?
+    assert_equal attacker_email, attacker.email
+  end
 end