CVE-2023-37904: Concurrent Execution using Shared Resource with Improper Synchronization ('Race Condition') (CWE-362)
value: Discourse is an open source discussion platform. Prior to version 3.0.6 of the `stable` branch and version 3.1.0.beta7 of the `beta` and `tests-passed` branches, more users than permitted could be created from invite links. The issue is patched in version 3.0.6 of the `stable` branch and version 3.1.0.beta7 of the `beta` and `tests-passed` branches. As a workaround, use restrict to email address invites.
CWE description: The product contains a code sequence that can run concurrently with other code, and the code sequence requires temporary, exclusive access to a shared resource, but a timing window exists in which the shared resource can be modified by another code sequence that is operating concurrently.
CVSS v3 base score: 3.1
Published: 2023-07-28T16:15Z
Repository: https://github.com/discourse/discourse
Language: Ruby
Files changed: app/controllers/invites_controller.rb, app/models/invite_redeemer.rb, spec/requests/invites_controller_spec.rb
Commit date: 2023-07-28 12:53:48 +0100
Commit message: SECURITY: Handle concurrent invite accepts

Raise an error on concurrent invite accept attempts.

Diff: diff --git a/app/controllers/invites_controller.rb b/app/controllers/invites_controller.rb
index 08cfc423ef7d..843eed212133 100644
--- a/app/controllers/invites_controller.rb
+++ b/app/controllers/invites_controller.rb
@@ -276,11 +276,14 @@ class InvitesController < ApplicationController
             attrs[:email_token] = params[:email_token] if params[:email_token].present?
           end
         end
 
         user = invite.redeem(**attrs)
-      rescue ActiveRecord::RecordInvalid, ActiveRecord::RecordNotSaved, Invite::UserExists => e
+      rescue ActiveRecord::RecordInvalid,
+             ActiveRecord::RecordNotSaved,
+             ActiveRecord::LockWaitTimeout,
+             Invite::UserExists => e
         return render json: failed_json.merge(message: e.message), status: 412
       end
 
       if user.blank?
         return render json: failed_json.merge(message: I18n.t("invite.not_found_json")), status: 404
diff --git a/app/models/invite_redeemer.rb b/app/models/invite_redeemer.rb
index 3fa9ac7313f7..9c760415627a 100644
--- a/app/models/invite_redeemer.rb
+++ b/app/models/invite_redeemer.rb
@@ -241,11 +241,14 @@ class InviteRedeemer
 
   def mark_invite_redeemed
     @invited_user_record = InvitedUser.create!(invite_id: invite.id, redeemed_at: Time.zone.now)
 
     if @invited_user_record.present?
-      Invite.increment_counter(:redemption_count, invite.id)
+      invite.with_lock("FOR UPDATE NOWAIT") do
+        Invite.increment_counter(:redemption_count, invite.id)
+        invite.save!
+      end
       delete_duplicate_invites
     end
 
     @invited_user_record.present?
   end
diff --git a/spec/requests/invites_controller_spec.rb b/spec/requests/invites_controller_spec.rb
index 89c0f8bdcae1..fabf6c1d3046 100644
--- a/spec/requests/invites_controller_spec.rb
+++ b/spec/requests/invites_controller_spec.rb
@@ -982,10 +982,31 @@ RSpec.describe InvitesController do
     context "with an invite link" do
       fab!(:invite) do
         Fabricate(:invite, email: nil, emailed_status: Invite.emailed_status_types[:not_required])
       end
 
+      it "does not create multiple users for a single use invite" do
+        user_count = User.count
+
+        2
+          .times
+          .map do
+            Thread.new do
+              put "/invites/show/#{invite.invite_key}.json",
+                  params: {
+                    email: "test@example.com",
+                    password: "verystrongpassword",
+                  }
+            end
+          end
+          .each(&:join)
+
+        expect(invite.reload.max_redemptions_allowed).to eq(1)
+        expect(invite.reload.redemption_count).to eq(1)
+        expect(User.count).to eq(user_count + 1)
+      end
+
       it "sends an activation email and does not activate the user" do
         expect {
           put "/invites/show/#{invite.invite_key}.json",
               params: {
                 email: "test@example.com",

Vulnerable code:       rescue ActiveRecord::RecordInvalid, ActiveRecord::RecordNotSaved, Invite::UserExists => e
      Invite.increment_counter(:redemption_count, invite.id)
Fixed code:       rescue ActiveRecord::RecordInvalid,
             ActiveRecord::RecordNotSaved,
             ActiveRecord::LockWaitTimeout,
             Invite::UserExists => e
      invite.with_lock("FOR UPDATE NOWAIT") do
        Invite.increment_counter(:redemption_count, invite.id)
        invite.save!
      end
      it "does not create multiple users for a single use invite" do
        user_count = User.count

        2
          .times
          .map do
            Thread.new do
              put "/invites/show/#{invite.invite_key}.json",
                  params: {
                    email: "test@example.com",
                    password: "verystrongpassword",
                  }
            end
          end
          .each(&:join)

        expect(invite.reload.max_redemptions_allowed).to eq(1)
        expect(invite.reload.redemption_count).to eq(1)
        expect(User.count).to eq(user_count + 1)
      end