CVE-2024-36113 (Ruby, published 2024-07-03T19:15Z)
Description: value: Discourse is an open-source discussion platform. Prior to version 3.2.3 on the `stable` branch, version 3.3.0.beta3 on the `beta` branch, and version 3.3.0.beta4-dev on the `tests-passed` branch, a rogue staff user could suspend other staff users preventing them from logging in to the site. The issue is patched in version 3.2.3 on the `stable` branch, version 3.3.0.beta3 on the `beta` branch, and version 3.3.0.beta4-dev on the `tests-passed` branch. No known workarounds are available.
Severity: nan (CVSS v3 base score: )
Weakness: NVD-CWE-noinfo \u2014 Insufficient Information
There is insufficient information about the issue to classify it; details are unkown or unspecified.
Repository: https://github.com/discourse/discourse
Files changed:
app/controllers/admin/users_controller.rb
app/services/user_suspender.rb
spec/requests/admin/users_controller_spec.rb
spec/services/user_suspender_spec.rb
Commit message: SECURITY: Don't allow suspending staff users via other_user_ids param


Vulnerable code:
    similar_users = User.real.where.not(id: @user.id).where(ip_address: @user.ip_address)
    all_users.each do |user|
      user.suspended_till = params[:suspend_until]
      user.suspended_at = DateTime.now

      message = params[:message]

      User.transaction do
        user.save!

        user_history =
          StaffActionLogger.new(current_user).log_user_suspend(
            user,
            params[:reason],
            message: message,
            post_id: params[:post_id],
          )
      end
      user.logged_out
      if message.present?
        Jobs.enqueue(
          :critical_user_email,
          type: "account_suspended",
          user_id: user.id,
          user_history_id: user_history.id,
      end

      DiscourseEvent.trigger(
        :user_suspended,
        user: user,
        reason: params[:reason],
        message: message,
        user_history: user_history,
        post_id: params[:post_id],
        suspended_till: params[:suspend_until],
        suspended_at: DateTime.now,
      )
        full_suspend_reason: user_history.try(:details),
    guardian.ensure_can_silence_user! @user
      it "returns similar users" do

Fixed code:
    similar_users =
      User
        .real
        .where.not(id: @user.id)
        .where(ip_address: @user.ip_address, admin: false, moderator: false)
    all_users.each { |user| raise Discourse::InvalidAccess.new if !guardian.can_suspend?(user) }
    all_users.each do |user|
      suspender =
        UserSuspender.new(
          user,
          suspended_till: params[:suspend_until],
          reason: params[:reason],
          by_user: current_user,
          message: params[:message],
          post_id: params[:post_id],
      suspender.suspend
      user_history = suspender.user_history
        full_suspend_reason: user_history&.details,
    all_users.each do |user|
      raise Discourse::InvalidAccess.new if !guardian.can_silence_user?(user)
    end

# frozen_string_literal: true

class UserSuspender
  attr_reader :user_history

  def initialize(user, suspended_till:, reason:, by_user:, message: nil, post_id: nil)
    @user = user
    @suspended_till = suspended_till
    @reason = reason
    @by_user = by_user
    @message = message
    @post_id = post_id
  end

  def suspend
    suspended_at = DateTime.now

    @user.suspended_till = @suspended_till
    @user.suspended_at = suspended_at

    @user.transaction do
      @user.save!

      @user_history =
        StaffActionLogger.new(@by_user).log_user_suspend(
          @user,
          @reason,
          message: @message,
          post_id: @post_id,
        )
    end
    @user.logged_out

    if @message.present?
      Jobs.enqueue(
        Jobs::CriticalUserEmail,
        type: "account_suspended",
        user_id: @user.id,
        user_history_id: @user_history.id,
      )
    end

    DiscourseEvent.trigger(
      :user_suspended,
      user: @user,
      reason: @reason,
      message: @message,
      user_history: @user_history,
      post_id: @post_id,
      suspended_till: @suspended_till,
      suspended_at: suspended_at,
    )
    nil
  end
end
  fab!(:another_admin) { Fabricate(:admin) }
      it "includes similar users who aren't admin or mods" do
        Fabricate(:admin, ip_address: user.ip_address)
        Fabricate(:moderator, ip_address: user.ip_address)
    shared_examples "suspension of staff users" do
      it "doesn't allow suspending a staff user" do
        put "/admin/users/#{another_admin.id}/suspend.json",
            params: {
              suspend_until: 5.hours.from_now,
              reason: "naughty boy",
            }

        expect(response.status).to eq(403)
        expect(another_admin.reload).not_to be_suspended
      end

      it "doesn't allow suspending a staff user via other_user_ids" do
        put "/admin/users/#{user.id}/suspend.json",
            params: {
              suspend_until: 5.hours.from_now,
              reason: "naughty boy",
              other_user_ids: [another_admin.id],
            }

        expect(response.status).to eq(403)
        expect(user.reload).not_to be_suspended
        expect(another_admin.reload).not_to be_suspended
      end
    end

      include_examples "suspension of staff users"
      include_examples "suspension of staff users"
      it "doesn't allow silencing another admin" do
        put "/admin/users/#{another_admin.id}/silence.json"
        expect(response.status).to eq(403)
        expect(another_admin.reload).to_not be_silenced
      end

      it "doesn't allow silencing another admin via other_user_ids" do
        put "/admin/users/#{reg_user.id}/silence.json",
            params: {
              other_user_ids: [another_admin.id],
            }
        expect(response.status).to eq(403)
        expect(another_admin.reload).to_not be_silenced
        expect(reg_user.reload).to_not be_silenced
      end


      it "doesn't allow silencing another admin" do
        put "/admin/users/#{another_admin.id}/silence.json"
        expect(response.status).to eq(403)
        expect(another_admin.reload).to_not be_silenced
      end

      it "doesn't allow silencing another admin via other_user_ids" do
        put "/admin/users/#{reg_user.id}/silence.json",
            params: {
              other_user_ids: [another_admin.id],
            }
        expect(response.status).to eq(403)
        expect(another_admin.reload).to_not be_silenced
        expect(reg_user.reload).to_not be_silenced
      end
# frozen_string_literal: true

RSpec.describe UserSuspender do
  fab!(:user) { Fabricate(:user, trust_level: 0) }
  fab!(:post) { Fabricate(:post, user: user) }
  fab!(:admin)

  describe "suspend" do
    subject(:suspend_user) { suspender.suspend }

    let(:suspender) do
      UserSuspender.new(
        user,
        suspended_till: 5.hours.from_now,
        reason: "because",
        by_user: admin,
        post_id: post.id,
        message: "you have been suspended",
      )
    end

    it "suspends the user correctly" do
      freeze_time
      suspend_user
      expect(user.reload).to be_suspended
      expect(user.suspended_till).to be_within_one_second_of(5.hours.from_now)
      expect(user.suspended_at).to be_within_one_second_of(Time.zone.now)
    end

    it "creates a staff action log" do
      expect do suspend_user end.to change {
        UserHistory.where(
          action: UserHistory.actions[:suspend_user],
          acting_user_id: admin.id,
          target_user_id: user.id,
        ).count
      }.from(0).to(1)
    end

    it "logs the user out" do
      messages = MessageBus.track_publish("/logout/#{user.id}") { suspend_user }
      expect(messages.size).to eq(1)
      expect(messages[0].user_ids).to eq([user.id])
      expect(messages[0].data).to eq(user.id)
    end

    it "fires a user_suspended event" do
      freeze_time
      events = DiscourseEvent.track_events(:user_suspended) { suspend_user }
      expect(events.size).to eq(1)

      params = events[0][:params].first
      expect(params[:user].id).to eq(user.id)
      expect(params[:reason]).to eq("because")
      expect(params[:message]).to eq("you have been suspended")
      expect(params[:suspended_till]).to be_within_one_second_of(5.hours.from_now)
      expect(params[:suspended_at]).to eq(Time.zone.now)
    end

    context "when a message is provided" do
      it "enqueues a critical user email job" do
        expect do suspend_user end.to change { Jobs::CriticalUserEmail.jobs.size }.from(0).to(1)

        job = Jobs::CriticalUserEmail.jobs.first
        expect(job["args"].first["user_id"]).to eq(user.id)
        expect(job["args"].first["user_history_id"]).to eq(suspender.user_history.id)
      end
    end

    context "when a message is not provided" do
      let(:suspender) do
        UserSuspender.new(
          user,
          suspended_till: 5.hours.from_now,
          reason: "because",
          by_user: admin,
          post_id: post.id,
          message: nil,
        )
      end

      it "doesn't enqueue a critical user email job" do
        expect do suspend_user end.not_to change { Jobs::CriticalUserEmail.jobs.size }.from(0)
      end
    end
  end
end