CVE-2024-36122: Insufficient Information (NVD-CWE-noinfo)
value: Discourse is an open-source discussion platform. Prior to version 3.2.3 on the `stable` branch and version 3.3.0.beta4 on the `beta` and `tests-passed` branches, moderators using the review queue to review users may see a users email address even when the Allow moderators to view email addresses setting is disabled. This issue is patched in version 3.2.3 on the `stable` branch and version 3.3.0.beta4 on the `beta` and `tests-passed` branches. As possible workarounds, either prevent moderators from accessing the review queue or disable the approve suspect users site setting and the must approve users site setting to prevent users from being added to the review queue.
CWE description: There is insufficient information about the issue to classify it; details are unkown or unspecified.
Published: 2024-07-03T20:15Z
Repository: https://github.com/discourse/discourse
Language: Ruby
Files changed: app/serializers/reviewable_user_serializer.rb, spec/serializers/reviewable_user_serializer_spec.rb
Commit date: 2024-07-03 20:49:19 +0800
Commit message: SECURITY: Update reviewable user serializer payload

Exclude email from reviewable user serializer based on user scope.

Diff: diff --git a/app/serializers/reviewable_user_serializer.rb b/app/serializers/reviewable_user_serializer.rb
index c517338201280..1033b6d8b2fe4 100644
--- a/app/serializers/reviewable_user_serializer.rb
+++ b/app/serializers/reviewable_user_serializer.rb
@@ -14,6 +14,16 @@ class ReviewableUserSerializer < ReviewableSerializer
   end
 
   def include_user_fields?
     object.target.present? && object.target.user_fields.present?
   end
+
+  def attributes(*args)
+    data = super
+    data[:payload]&.delete("email") if !include_email?
+    data
+  end
+
+  def include_email?
+    scope.can_check_emails?(scope.user)
+  end
 end
diff --git a/spec/serializers/reviewable_user_serializer_spec.rb b/spec/serializers/reviewable_user_serializer_spec.rb
index bb7e4ad3efce1..70b7a93322590 100644
--- a/spec/serializers/reviewable_user_serializer_spec.rb
+++ b/spec/serializers/reviewable_user_serializer_spec.rb
@@ -1,19 +1,44 @@
 # frozen_string_literal: true
 
 RSpec.describe ReviewableUserSerializer do
   let(:user) { Fabricate(:user) }
   let(:admin) { Fabricate(:admin) }
+  let(:moderator) { Fabricate(:moderator) }
+  let(:reviewable) { Reviewable.find_by(target: user) }
 
-  it "includes the user fields for review" do
+  before do
     SiteSetting.must_approve_users = true
     Jobs::CreateUserReviewable.new.execute(user_id: user.id)
-    reviewable = Reviewable.find_by(target: user)
+  end
 
+  it "includes the user fields for review" do
     json = ReviewableUserSerializer.new(reviewable, scope: Guardian.new(admin), root: nil).as_json
     expect(json[:user_id]).to eq(reviewable.target_id)
     expect(json[:payload]["username"]).to eq(user.username)
     expect(json[:payload]["email"]).to eq(user.email)
     expect(json[:payload]["name"]).to eq(user.name)
     expect(json[:topic_url]).to be_blank
   end
+
+  it "excludes the email user field for moderators" do
+    json =
+      ReviewableUserSerializer.new(reviewable, scope: Guardian.new(moderator), root: nil).as_json
+    expect(json[:user_id]).to eq(reviewable.target_id)
+    expect(json[:payload]["username"]).to eq(user.username)
+    expect(json[:payload]["email"]).to eq(nil)
+    expect(json[:payload]["name"]).to eq(user.name)
+    expect(json[:topic_url]).to be_blank
+  end
+
+  it "includes the email user field for moderators if enabled" do
+    SiteSetting.moderators_view_emails = true
+
+    json =
+      ReviewableUserSerializer.new(reviewable, scope: Guardian.new(moderator), root: nil).as_json
+    expect(json[:user_id]).to eq(reviewable.target_id)
+    expect(json[:payload]["username"]).to eq(user.username)
+    expect(json[:payload]["email"]).to eq(user.email)
+    expect(json[:payload]["name"]).to eq(user.name)
+    expect(json[:topic_url]).to be_blank
+  end
 end

Vulnerable code:   it "includes the user fields for review" do
    reviewable = Reviewable.find_by(target: user)
Fixed code: 
  def attributes(*args)
    data = super
    data[:payload]&.delete("email") if !include_email?
    data
  end

  def include_email?
    scope.can_check_emails?(scope.user)
  end
  let(:moderator) { Fabricate(:moderator) }
  let(:reviewable) { Reviewable.find_by(target: user) }
  before do
  end
  it "includes the user fields for review" do

  it "excludes the email user field for moderators" do
    json =
      ReviewableUserSerializer.new(reviewable, scope: Guardian.new(moderator), root: nil).as_json
    expect(json[:user_id]).to eq(reviewable.target_id)
    expect(json[:payload]["username"]).to eq(user.username)
    expect(json[:payload]["email"]).to eq(nil)
    expect(json[:payload]["name"]).to eq(user.name)
    expect(json[:topic_url]).to be_blank
  end

  it "includes the email user field for moderators if enabled" do
    SiteSetting.moderators_view_emails = true

    json =
      ReviewableUserSerializer.new(reviewable, scope: Guardian.new(moderator), root: nil).as_json
    expect(json[:user_id]).to eq(reviewable.target_id)
    expect(json[:payload]["username"]).to eq(user.username)
    expect(json[:payload]["email"]).to eq(user.email)
    expect(json[:payload]["name"]).to eq(user.name)
    expect(json[:topic_url]).to be_blank
  end