CVE-2023-49098: Improper Access Control (CWE-284)
value: Discourse-reactions is a plugin that allows user to add their reactions to the post. Data about a user's reaction notifications could be exposed. This vulnerability was patched in commit 2c26939.
CWE description: The product does not restrict or incorrectly restricts access to a resource from an unauthorized actor.
CVSS v3 base score: 3.5
Published: 2024-01-12T21: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: 2024-01-08 17:26:54 -0500
Commit message: SECURITY: restrict custom reaction access (#263)

Co-authored-by: David Battersby <info@davidbattersby.com>
Diff: diff --git a/app/controllers/discourse_reactions/custom_reactions_controller.rb b/app/controllers/discourse_reactions/custom_reactions_controller.rb
index e8a241b..f232d86 100644
--- a/app/controllers/discourse_reactions/custom_reactions_controller.rb
+++ b/app/controllers/discourse_reactions/custom_reactions_controller.rb
@@ -43,11 +43,11 @@ class DiscourseReactions::CustomReactionsController < ApplicationController
     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)
+    raise Discourse::InvalidAccess unless guardian.can_see_notifications?(user)
 
     reaction_users =
       DiscourseReactions::ReactionUser
         .joins(:reaction, :post)
         .joins("INNER JOIN topics t ON t.id = posts.topic_id AND t.deleted_at IS NULL")
@@ -76,11 +76,11 @@ class DiscourseReactions::CustomReactionsController < ApplicationController
     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)
+    raise Discourse::InvalidAccess unless guardian.can_see_notifications?(user)
 
     posts = Post.joins(:topic).where(user_id: user.id)
     posts = guardian.filter_allowed_categories(posts)
     post_ids = posts.select(:id)
 
diff --git a/spec/requests/custom_reactions_controller_spec.rb b/spec/requests/custom_reactions_controller_spec.rb
index e93c16a..7054245 100644
--- a/spec/requests/custom_reactions_controller_spec.rb
+++ b/spec/requests/custom_reactions_controller_spec.rb
@@ -165,31 +165,31 @@ describe DiscourseReactions::CustomReactionsController do
     end
     fab!(:secure_topic_reaction_user) do
       Fabricate(:reaction_user, reaction: secure_reaction, user: user_2, post: secure_post)
     end
 
-    it "returns reactions given by a user" do
-      sign_in(user_1)
+    it "returns reactions given by a user when current user is admin" do
+      sign_in(admin)
 
       get "/discourse-reactions/posts/reactions.json", params: { username: user_2.username }
-      parsed = response.parsed_body
+      expect(response.status).to eq(200)
 
-      expect(parsed[0]["user"]["id"]).to eq(user_2.id)
-      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)
+      parsed = response.parsed_body
+      expect(parsed[2]["user"]["id"]).to eq(user_2.id)
+      expect(parsed[2]["post_id"]).to eq(post_2.id)
+      expect(parsed[2]["post"]["user"]["id"]).to eq(user_1.id)
+      expect(parsed[2]["reaction"]["id"]).to eq(reaction_1.id)
     end
 
-    it "does not return reactions for private messages" do
+    it "does not return reactions for private messages of other users" 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,
-      )
+      expect(response.status).to eq(403)
+    end
 
+    it "returns reactions for private messages of current user" do
       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(
@@ -200,19 +200,15 @@ describe DiscourseReactions::CustomReactionsController do
     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,
-      )
+      expect(response.status).to eq(403)
 
       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)
+      expect(response.status).to eq(403)
 
       sign_in(user_2)
 
       get "/discourse-reactions/posts/reactions.json", params: { username: user_2.username }
       parsed = response.parsed_body
@@ -279,12 +275,12 @@ describe DiscourseReactions::CustomReactionsController do
       end
     end
   end
 
   describe "#reactions_received" do
-    it "returns reactions received by a user" do
-      sign_in(user_2)
+    it "returns reactions received by a user when current user is admin" do
+      sign_in(admin)
 
       get "/discourse-reactions/posts/reactions-received.json",
           params: {
             username: user_1.username,
           }
@@ -294,10 +290,21 @@ 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_2.id)
     end
 
+    it "does not return reactions received by a user when current user is not an admin" do
+      sign_in(user_1)
+
+      get "/discourse-reactions/posts/reactions-received.json",
+          params: {
+            username: user_2.username,
+          }
+
+      expect(response.status).to eq(403)
+    end
+
     it "filters by acting username" do
       sign_in(user_1)
 
       get "/discourse-reactions/posts/reactions-received.json",
           params: {

Vulnerable code:     raise Discourse::NotFound unless guardian.can_see_profile?(user)
    raise Discourse::NotFound unless guardian.can_see_profile?(user)
    it "returns reactions given by a user" do
      sign_in(user_1)
      parsed = response.parsed_body
      expect(parsed[0]["user"]["id"]).to eq(user_2.id)
      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)
    it "does not return reactions for private messages" do
      parsed = response.parsed_body
      expect(response.parsed_body.map { |reaction| reaction["post_id"] }).not_to include(
        private_post.id,
      )
      parsed = response.parsed_body
      expect(response.parsed_body.map { |reaction| reaction["post_id"] }).not_to include(
        secure_post.id,
      )
      parsed = response.parsed_body
      expect(response.parsed_body.map { |reaction| reaction["post_id"] }).to include(secure_post.id)
    it "returns reactions received by a user" do
      sign_in(user_2)
Fixed code:     raise Discourse::InvalidAccess unless guardian.can_see_notifications?(user)
    raise Discourse::InvalidAccess unless guardian.can_see_notifications?(user)
    it "returns reactions given by a user when current user is admin" do
      sign_in(admin)
      expect(response.status).to eq(200)
      parsed = response.parsed_body
      expect(parsed[2]["user"]["id"]).to eq(user_2.id)
      expect(parsed[2]["post_id"]).to eq(post_2.id)
      expect(parsed[2]["post"]["user"]["id"]).to eq(user_1.id)
      expect(parsed[2]["reaction"]["id"]).to eq(reaction_1.id)
    it "does not return reactions for private messages of other users" do
      expect(response.status).to eq(403)
    end
    it "returns reactions for private messages of current user" do
      expect(response.status).to eq(403)
      expect(response.status).to eq(403)
    it "returns reactions received by a user when current user is admin" do
      sign_in(admin)
    it "does not return reactions received by a user when current user is not an admin" do
      sign_in(user_1)

      get "/discourse-reactions/posts/reactions-received.json",
          params: {
            username: user_2.username,
          }

      expect(response.status).to eq(403)
    end