CVE-2021-41140: Exposure of Sensitive Information to an Unauthorized Actor (CWE-200)
value: Discourse-reactions is a plugin for the Discourse platform that allows user to add their reactions to the post. In affected versions reactions given by user to secure topics and private messages are visible. This issue is patched in version 0.2 of discourse-reaction. Users who are unable to update are advised to disable the Discourse-reactions plugin in admin panel.
CWE description: The product exposes sensitive information to an actor that is not explicitly authorized to have access to that information.
CVSS v3 base score: 5.3
Published: 2021-10-19T18:15Z
Repository: https://github.com/discourse/discourse-reactions
Language: Ruby
Files changed: app/controllers/discourse_reactions/custom_reactions_controller.rb, spec/requests/custom_reactions_controller_spec.rb
Commit date: 2021-10-11 13:03:38 +1100
Commit message: SECURITY: leaking PMs and secure categories topics (#110)

Ensure that user has access to private messages and categories of he topics
Diff: diff --git a/app/controllers/discourse_reactions/custom_reactions_controller.rb b/app/controllers/discourse_reactions/custom_reactions_controller.rb
index f635749..1a68064 100644
--- a/app/controllers/discourse_reactions/custom_reactions_controller.rb
+++ b/app/controllers/discourse_reactions/custom_reactions_controller.rb
@@ -30,15 +30,18 @@ module DiscourseReactions
       params.require(:username)
       user = fetch_user_from_params(include_inactive: current_user.try(:staff?) || (current_user && SiteSetting.show_inactive_accounts))
       raise Discourse::NotFound unless guardian.can_see_profile?(user)
 
       reaction_users = DiscourseReactions::ReactionUser
-        .joins(:reaction, :post)
+        .joins(:reaction, post: :topic)
+        .joins("LEFT JOIN categories ON categories.id = topics.category_id")
         .includes(:user, :post, :reaction)
         .where(user_id: user.id)
         .where('discourse_reactions_reactions.reaction_users_count IS NOT NULL')
 
+      reaction_users = secure_reaction_users!(reaction_users)
+
       if params[:before_reaction_user_id]
         reaction_users = reaction_users
           .where('discourse_reactions_reaction_users.id < ?', params[:before_reaction_user_id].to_i)
       end
 
@@ -185,7 +188,42 @@ module DiscourseReactions
         reactions: reactions
       }
 
       MessageBus.publish("/topic/#{post.topic.id}/reactions", message)
     end
+
+    def secure_reaction_users!(reaction_users)
+      if !guardian.can_see_private_messages?(current_user.id) || !guardian.user
+        reaction_users = reaction_users.where("topics.archetype <> :private_message", private_message: archetype::private_message)
+      else
+        unless guardian.is_admin?
+          sql = <<~SQL
+            topics.archetype <> :private_message OR
+            EXISTS (
+              SELECT 1 FROM topic_allowed_users tu WHERE tu.topic_id = topics.id AND tu.user_id = :current_user_id
+            ) OR
+            EXISTS (
+              SELECT 1 FROM topic_allowed_groups tg WHERE tg.topic_id = topics.id AND tg.group_id IN (
+                SELECT group_id FROM group_users gu WHERE gu.user_id = :current_user_id
+              )
+            )
+          SQL
+
+          reaction_users = reaction_users.where(sql, private_message: Archetype::private_message, current_user_id: guardian.user.id)
+        end
+      end
+
+      unless guardian.is_admin?
+        allowed = guardian.secure_category_ids
+        if allowed.present?
+          reaction_users = reaction_users.where("(categories.read_restricted IS NULL OR
+                         NOT categories.read_restricted OR
+                        (categories.read_restricted and categories.id in (:categories)) )", categories: guardian.secure_category_ids)
+        else
+          reaction_users = reaction_users.where("(categories.read_restricted IS NULL OR NOT categories.read_restricted)")
+        end
+      end
+
+      reaction_users
+    end
   end
 end
diff --git a/spec/requests/custom_reactions_controller_spec.rb b/spec/requests/custom_reactions_controller_spec.rb
index b6d45cc..bec23aa 100644
--- a/spec/requests/custom_reactions_controller_spec.rb
+++ b/spec/requests/custom_reactions_controller_spec.rb
@@ -1,8 +1,10 @@
 # frozen_string_literal: true
 
 require 'rails_helper'
+require_relative '../fabricators/reaction_fabricator.rb'
+require_relative '../fabricators/reaction_user_fabricator.rb'
 
 describe DiscourseReactions::CustomReactionsController do
   fab!(:post_1) { Fabricate(:post) }
   fab!(:user_1) { Fabricate(:user) }
   fab!(:user_2) { Fabricate(:user) }
@@ -107,10 +109,21 @@ describe DiscourseReactions::CustomReactionsController do
       expect(response.status).to eq(422)
     end
   end
 
   context '#reactions_given' do
+    fab!(:private_topic) { Fabricate(:private_message_topic, user: user_2) }
+    fab!(:private_post) { Fabricate(:post, topic: private_topic) }
+    fab!(:secure_group) { Fabricate(:group) }
+    fab!(:secure_category) { Fabricate(:private_category, group: secure_group) }
+    fab!(:secure_topic) { Fabricate(:topic, category: secure_category) }
+    fab!(:secure_post) { Fabricate(:post, topic: secure_topic) }
+    fab!(:private_reaction) { Fabricate(:reaction, post: private_post, reaction_value: "hugs") }
+    fab!(:secure_reaction) { Fabricate(:reaction, post: secure_post, reaction_value: "hugs") }
+    fab!(:private_topic_reaction_user) { Fabricate(:reaction_user, reaction: private_reaction, user: user_2, post: private_post) }
+    fab!(:secure_topic_reaction_user) { Fabricate(:reaction_user, reaction: secure_reaction, user: user_2, post: secure_post) }
+
     it 'returns reactions given by a user' do
       sign_in(user_1)
 
       get "/discourse-reactions/posts/reactions.json", params: { username: user_2.username }
       parsed = response.parsed_body
@@ -119,10 +132,44 @@ describe DiscourseReactions::CustomReactionsController do
       expect(parsed[0]['post_id']).to eq(post_2.id)
       expect(parsed[0]['post']['user']['id']).to eq(user_1.id)
       expect(parsed[0]['reaction']['id']).to eq(reaction_1.id)
     end
 
+    it 'does not return reactions for private messages' do
+      sign_in(user_1)
+
+      get "/discourse-reactions/posts/reactions.json", params: { username: user_2.username }
+      parsed = response.parsed_body
+      expect(response.parsed_body.map { |reaction| reaction["post_id"] }).not_to include(private_post.id)
+
+      sign_in(user_2)
+
+      get "/discourse-reactions/posts/reactions.json", params: { username: user_2.username }
+      parsed = response.parsed_body
+      expect(response.parsed_body.map { |reaction| reaction["post_id"] }).to include(private_post.id)
+    end
+
+    it 'does not return reactions for secure categories' do
+      secure_group.add(user_2)
+      sign_in(user_1)
+
+      get "/discourse-reactions/posts/reactions.json", params: { username: user_2.username }
+      parsed = response.parsed_body
+      expect(response.parsed_body.map { |reaction| reaction["post_id"] }).not_to include(secure_post.id)
+
+      secure_group.add(user_1)
+      get "/discourse-reactions/posts/reactions.json", params: { username: user_2.username }
+      parsed = response.parsed_body
+      expect(response.parsed_body.map { |reaction| reaction["post_id"] }).to include(secure_post.id)
+
+      sign_in(user_2)
+
+      get "/discourse-reactions/posts/reactions.json", params: { username: user_2.username }
+      parsed = response.parsed_body
+      expect(response.parsed_body.map { |reaction| reaction["post_id"] }).to include(secure_post.id)
+    end
+
     context 'a post with one of your reactions has been deleted' do
       fab!(:deleted_post) { Fabricate(:post) }
       fab!(:kept_post) { Fabricate(:post) }
       fab!(:user) { Fabricate(:user) }
       fab!(:reaction_on_deleted_post) { Fabricate(:reaction, post: deleted_post, reaction_value: "laughing") }

Vulnerable code:         .joins(:reaction, :post)
Fixed code:         .joins(:reaction, post: :topic)
        .joins("LEFT JOIN categories ON categories.id = topics.category_id")
      reaction_users = secure_reaction_users!(reaction_users)


    def secure_reaction_users!(reaction_users)
      if !guardian.can_see_private_messages?(current_user.id) || !guardian.user
        reaction_users = reaction_users.where("topics.archetype <> :private_message", private_message: archetype::private_message)
      else
        unless guardian.is_admin?
          sql = <<~SQL
            topics.archetype <> :private_message OR
            EXISTS (
              SELECT 1 FROM topic_allowed_users tu WHERE tu.topic_id = topics.id AND tu.user_id = :current_user_id
            ) OR
            EXISTS (
              SELECT 1 FROM topic_allowed_groups tg WHERE tg.topic_id = topics.id AND tg.group_id IN (
                SELECT group_id FROM group_users gu WHERE gu.user_id = :current_user_id
              )
            )
          SQL

          reaction_users = reaction_users.where(sql, private_message: Archetype::private_message, current_user_id: guardian.user.id)
        end
      end

      unless guardian.is_admin?
        allowed = guardian.secure_category_ids
        if allowed.present?
          reaction_users = reaction_users.where("(categories.read_restricted IS NULL OR
                         NOT categories.read_restricted OR
                        (categories.read_restricted and categories.id in (:categories)) )", categories: guardian.secure_category_ids)
        else
          reaction_users = reaction_users.where("(categories.read_restricted IS NULL OR NOT categories.read_restricted)")
        end
      end

      reaction_users
    end
require_relative '../fabricators/reaction_fabricator.rb'
require_relative '../fabricators/reaction_user_fabricator.rb'
    fab!(:private_topic) { Fabricate(:private_message_topic, user: user_2) }
    fab!(:private_post) { Fabricate(:post, topic: private_topic) }
    fab!(:secure_group) { Fabricate(:group) }
    fab!(:secure_category) { Fabricate(:private_category, group: secure_group) }
    fab!(:secure_topic) { Fabricate(:topic, category: secure_category) }
    fab!(:secure_post) { Fabricate(:post, topic: secure_topic) }
    fab!(:private_reaction) { Fabricate(:reaction, post: private_post, reaction_value: "hugs") }
    fab!(:secure_reaction) { Fabricate(:reaction, post: secure_post, reaction_value: "hugs") }
    fab!(:private_topic_reaction_user) { Fabricate(:reaction_user, reaction: private_reaction, user: user_2, post: private_post) }
    fab!(:secure_topic_reaction_user) { Fabricate(:reaction_user, reaction: secure_reaction, user: user_2, post: secure_post) }

    it 'does not return reactions for private messages' do
      sign_in(user_1)

      get "/discourse-reactions/posts/reactions.json", params: { username: user_2.username }
      parsed = response.parsed_body
      expect(response.parsed_body.map { |reaction| reaction["post_id"] }).not_to include(private_post.id)

      sign_in(user_2)

      get "/discourse-reactions/posts/reactions.json", params: { username: user_2.username }
      parsed = response.parsed_body
      expect(response.parsed_body.map { |reaction| reaction["post_id"] }).to include(private_post.id)
    end

    it 'does not return reactions for secure categories' do
      secure_group.add(user_2)
      sign_in(user_1)

      get "/discourse-reactions/posts/reactions.json", params: { username: user_2.username }
      parsed = response.parsed_body
      expect(response.parsed_body.map { |reaction| reaction["post_id"] }).not_to include(secure_post.id)

      secure_group.add(user_1)
      get "/discourse-reactions/posts/reactions.json", params: { username: user_2.username }
      parsed = response.parsed_body
      expect(response.parsed_body.map { |reaction| reaction["post_id"] }).to include(secure_post.id)

      sign_in(user_2)

      get "/discourse-reactions/posts/reactions.json", params: { username: user_2.username }
      parsed = response.parsed_body
      expect(response.parsed_body.map { |reaction| reaction["post_id"] }).to include(secure_post.id)
    end