CVE-2023-30611: Insufficient Information (NVD-CWE-noinfo)
value: Discourse-reactions is a plugin that allows user to add their reactions to the post in the Discourse messaging platform. In affected versions data about what reactions were performed on a post in a private topic could be leaked. This issue has been addressed in version 0.3. Users are advised to upgrade. Users unable to upgrade should disable the discourse-reactions plugin to fully mitigate the issue.
CWE description: There is insufficient information about the issue to classify it; details are unkown or unspecified.
CVSS v3 base score: 5.3
Published: 2023-04-19T18:15Z
Repository: https://github.com/discourse/discourse-reactions
Language: Ruby
Files changed: app/controllers/discourse_reactions/custom_reactions_controller.rb, plugin.rb, spec/requests/custom_reactions_controller_spec.rb
Commit date: 2023-04-03 11:34:28 -0600
Commit message: SECURITY: Publish reactions based on topic permissions (#218)

Only show post reactions to users that have permission to see the
post.
Diff: diff --git a/app/controllers/discourse_reactions/custom_reactions_controller.rb b/app/controllers/discourse_reactions/custom_reactions_controller.rb
index c356e2c..44e280b 100644
--- a/app/controllers/discourse_reactions/custom_reactions_controller.rb
+++ b/app/controllers/discourse_reactions/custom_reactions_controller.rb
@@ -136,11 +136,11 @@ class DiscourseReactions::CustomReactionsController < ApplicationController
   end
 
   def post_reactions_users
     id = params.require(:id).to_i
     reaction_value = params[:reaction_value]
-    post = Post.find_by(id: id)
+    post = fetch_post_from_params
 
     raise Discourse::InvalidParameters if !post
 
     reaction_users = []
 
@@ -235,21 +235,24 @@ class DiscourseReactions::CustomReactionsController < ApplicationController
   def format_likes_users(likes)
     likes.includes([:user]).limit(MAX_USERS_COUNT + 1).map { |like| format_like_user(like) }
   end
 
   def fetch_post_from_params
-    post = Post.find(params[:post_id])
+    post_id = params[:post_id] || params[:id]
+    post = Post.find(post_id)
     guardian.ensure_can_see!(post)
     post
   end
 
   def publish_change_to_clients!(post, reaction: nil, previous_reaction: nil)
-    MessageBus.publish(
-      "/topic/#{post.topic.id}/reactions",
-      post_id: post.id,
-      reactions: [reaction, previous_reaction].compact.uniq,
-    )
+    message = { post_id: post.id, reactions: [reaction, previous_reaction].compact.uniq }
+
+    opts = {}
+    secure_audience = post.topic.secure_audience_publish_messages
+    opts = secure_audience if secure_audience[:user_ids] != [] && secure_audience[:group_ids] != []
+
+    MessageBus.publish("/topic/#{post.topic.id}/reactions", message, opts)
   end
 
   def secure_reaction_users!(reaction_users)
     builder = DB.build("/*where*/")
     UserAction.filter_private_messages(builder, current_user.id, guardian)
diff --git a/plugin.rb b/plugin.rb
index 826d632..9eca19c 100644
--- a/plugin.rb
+++ b/plugin.rb
@@ -1,10 +1,10 @@
 # frozen_string_literal: true
 
 # name: discourse-reactions
 # about: Allows users to react with emojis to a post
-# version: 0.2
+# version: 0.3
 # author: Ahmed Gagan, Rafael dos Santos Silva, Kris Aubuchon, Joffrey Jaffeux, Kris Kotlarek, Jordan Vidrine
 # url: https://github.com/discourse/discourse-reactions
 # transpile_js: true
 
 enabled_site_setting :discourse_reactions_enabled
diff --git a/spec/requests/custom_reactions_controller_spec.rb b/spec/requests/custom_reactions_controller_spec.rb
index e2824f9..e93c16a 100644
--- a/spec/requests/custom_reactions_controller_spec.rb
+++ b/spec/requests/custom_reactions_controller_spec.rb
@@ -7,14 +7,18 @@ describe DiscourseReactions::CustomReactionsController do
   fab!(:user_1) { Fabricate(:user) }
   fab!(:user_2) { Fabricate(:user) }
   fab!(:user_3) { Fabricate(:user) }
   fab!(:user_4) { Fabricate(:user) }
   fab!(:user_5) { Fabricate(:user) }
+  fab!(:admin) { Fabricate(:admin) }
   fab!(:post_2) { Fabricate(:post, user: user_1) }
+  fab!(:private_topic) { Fabricate(:private_message_topic, user: user_2, recipient: admin) }
+  fab!(:private_post) { Fabricate(:post, topic: private_topic) }
   fab!(:reaction_1) { Fabricate(:reaction, post: post_2, reaction_value: "laughing") }
   fab!(:reaction_2) { Fabricate(:reaction, post: post_2, reaction_value: "open_mouth") }
   fab!(:reaction_3) { Fabricate(:reaction, post: post_2, reaction_value: "hugs") }
+  fab!(:reaction_4) { Fabricate(:reaction, post: private_post, reaction_value: "hugs") }
   fab!(:like) do
     Fabricate(
       :post_action,
       post: post_2,
       user: user_5,
@@ -31,20 +35,24 @@ describe DiscourseReactions::CustomReactionsController do
     Fabricate(:reaction_user, reaction: reaction_3, user: user_4, post: post_2)
   end
   fab!(:reaction_user_4) do
     Fabricate(:reaction_user, reaction: reaction_2, user: user_3, post: post_2)
   end
+  fab!(:reaction_user_5) do
+    Fabricate(:reaction_user, reaction: reaction_4, user: admin, post: private_post)
+  end
 
   before do
     SiteSetting.discourse_reactions_enabled = true
     SiteSetting.discourse_reactions_like_icon = "heart"
     SiteSetting.discourse_reactions_enabled_reactions =
       "laughing|open_mouth|cry|angry|thumbsup|hugs"
   end
 
   describe "#toggle" do
     let(:payload_with_user) { [{ "id" => "hugs", "type" => "emoji", "count" => 1 }] }
+    let(:api_key) { Fabricate(:api_key, user: admin, created_by: admin) }
 
     it "toggles reaction" do
       sign_in(user_1)
       expected_payload = [{ "id" => "hugs", "type" => "emoji", "count" => 1 }]
       expect do
@@ -116,10 +124,25 @@ describe DiscourseReactions::CustomReactionsController do
       expect(messages.map(&:channel).uniq.first).to eq("/topic/#{post_1.topic.id}/reactions")
       expect(messages[0].data[:reactions]).to contain_exactly("cry")
       expect(messages[1].data[:reactions]).to contain_exactly("cry", "angry")
     end
 
+    it "publishes MessageBus messages securely" do
+      sign_in(user_1)
+      messages =
+        MessageBus.track_publish("/topic/#{private_post.topic.id}/reactions") do
+          put "/discourse-reactions/posts/#{private_post.id}/custom-reactions/cry/toggle.json",
+              headers: {
+                "HTTP_API_KEY" => api_key.key,
+                "HTTP_API_USERNAME" => api_key.user.username,
+              }
+        end
+      user_1_messages = messages.find { |m| m.user_ids.include?(user_1.id) }
+      expect(messages.count).to eq(1)
+      expect(user_1_messages).to eq(nil)
+    end
+
     it "errors when reaction is invalid" do
       sign_in(user_1)
       expect do
         put "/discourse-reactions/posts/#{post_1.id}/custom-reactions/invalid-reaction/toggle.json"
       end.not_to change { DiscourseReactions::Reaction.count }
@@ -386,16 +409,16 @@ describe DiscourseReactions::CustomReactionsController do
       expect(parsed["reaction_users"][0]["users"][0]["avatar_template"]).to eq(
         user_1.avatar_template,
       )
     end
 
-    it "gives 400 ERROR when the post_id OR reaction_value is invalid" do
+    it "gives 404 ERROR when the post_id OR reaction_value is invalid" do
       get "/discourse-reactions/posts/1000000/reactions-users.json"
-      expect(response.status).to eq(400)
+      expect(response.status).to eq(404)
 
       get "/discourse-reactions/posts/1000000/reactions-users.json?reaction_value=test"
-      expect(response.status).to eq(400)
+      expect(response.status).to eq(404)
     end
 
     it "merges identic custom reaction into likes" do
       get "/discourse-reactions/posts/#{post_2.id}/reactions-users.json?reaction_value=#{DiscourseReactions::Reaction.main_reaction_id}"
       parsed = response.parsed_body
@@ -411,10 +434,21 @@ describe DiscourseReactions::CustomReactionsController do
 
       get "/discourse-reactions/posts/#{post_2.id}/reactions-users.json?reaction_value=#{DiscourseReactions::Reaction.main_reaction_id}"
       parsed = response.parsed_body
       expect(parsed["reaction_users"][0]["count"]).to eq(like_count + reaction_count)
     end
+
+    it "does not show reaction_users on PMs without permission" do
+      get "/discourse-reactions/posts/#{private_post.id}/reactions-users.json"
+      expect(response.status).to eq(403)
+    end
+
+    it "shows reaction_users on PMs with permission" do
+      sign_in(user_2)
+      get "/discourse-reactions/posts/#{private_post.id}/reactions-users.json"
+      expect(response.status).to eq(200)
+    end
   end
 
   describe "positive notifications" do
     before { PostActionNotifier.enable }
 

Vulnerable code:     post = Post.find_by(id: id)
    post = Post.find(params[:post_id])
    MessageBus.publish(
      "/topic/#{post.topic.id}/reactions",
      post_id: post.id,
      reactions: [reaction, previous_reaction].compact.uniq,
    )
# version: 0.2
    it "gives 400 ERROR when the post_id OR reaction_value is invalid" do
      expect(response.status).to eq(400)
      expect(response.status).to eq(400)
Fixed code:     post = fetch_post_from_params
    post_id = params[:post_id] || params[:id]
    post = Post.find(post_id)
    message = { post_id: post.id, reactions: [reaction, previous_reaction].compact.uniq }

    opts = {}
    secure_audience = post.topic.secure_audience_publish_messages
    opts = secure_audience if secure_audience[:user_ids] != [] && secure_audience[:group_ids] != []

    MessageBus.publish("/topic/#{post.topic.id}/reactions", message, opts)
# version: 0.3
  fab!(:admin) { Fabricate(:admin) }
  fab!(:private_topic) { Fabricate(:private_message_topic, user: user_2, recipient: admin) }
  fab!(:private_post) { Fabricate(:post, topic: private_topic) }
  fab!(:reaction_4) { Fabricate(:reaction, post: private_post, reaction_value: "hugs") }
  fab!(:reaction_user_5) do
    Fabricate(:reaction_user, reaction: reaction_4, user: admin, post: private_post)
  end
    let(:api_key) { Fabricate(:api_key, user: admin, created_by: admin) }
    it "publishes MessageBus messages securely" do
      sign_in(user_1)
      messages =
        MessageBus.track_publish("/topic/#{private_post.topic.id}/reactions") do
          put "/discourse-reactions/posts/#{private_post.id}/custom-reactions/cry/toggle.json",
              headers: {
                "HTTP_API_KEY" => api_key.key,
                "HTTP_API_USERNAME" => api_key.user.username,
              }
        end
      user_1_messages = messages.find { |m| m.user_ids.include?(user_1.id) }
      expect(messages.count).to eq(1)
      expect(user_1_messages).to eq(nil)
    end

    it "gives 404 ERROR when the post_id OR reaction_value is invalid" do
      expect(response.status).to eq(404)
      expect(response.status).to eq(404)

    it "does not show reaction_users on PMs without permission" do
      get "/discourse-reactions/posts/#{private_post.id}/reactions-users.json"
      expect(response.status).to eq(403)
    end

    it "shows reaction_users on PMs with permission" do
      sign_in(user_2)
      get "/discourse-reactions/posts/#{private_post.id}/reactions-users.json"
      expect(response.status).to eq(200)
    end