CVE-2023-23935: Insufficient Information (NVD-CWE-noinfo)
value: Discourse is an open-source messaging platform. In versions 3.0.1 and prior on the `stable` branch and versions 3.1.0.beta2 and prior on the `beta` and `tests-passed` branches, the count of personal messages displayed for a tag is a count of all personal messages regardless of whether the personal message is visible to a given user. As a result, any users can technically poll a sensitive tag to determine if a new personal message is created even if the user does not have access to the personal message.

In the patched versions, the count of personal messages tagged with a given tag is hidden by default. To revert to the old behaviour of displaying the count of personal messages for a given tag, an admin may enable the `display_personal_messages_tag_counts` site setting.
CWE description: There is insufficient information about the issue to classify it; details are unkown or unspecified.
CVSS v3 base score: 4.3
Published: 2023-03-16T21:15Z
Repository: https://github.com/discourse/discourse
Language: JavaScript
Files changed: app/assets/javascripts/discourse/app/models/tag.js, app/assets/javascripts/discourse/tests/helpers/create-pretender.js, app/assets/javascripts/discourse/tests/integration/components/select-kit/tag-drop-test.js, app/assets/javascripts/discourse/tests/unit/models/tag-test.js, app/controllers/tags_controller.rb, config/locales/server.en.yml, config/site_settings.yml, spec/requests/tags_controller_spec.rb
Commit date: 2023-01-31 12:08:23 +0800
Commit message: SECURITY: Hide PM count for tags by default (#20061)

Currently `Topic#pm_topic_count` is a count of all personal messages tagged for a given tag. As a result, any user with access to PM tags can poll a sensitive tag to determine if a new personal message has been created using that tag even if the user does not have access to the personal message. We classify this as a minor leak in sensitive information.

With this commit, `Topic#pm_topic_count` is hidden from users by default unless the `display_personal_messages_tag_counts` site setting is enabled. 
Diff: diff --git a/app/assets/javascripts/discourse/app/models/tag.js b/app/assets/javascripts/discourse/app/models/tag.js
index 5f514f8db109..54f3733065a3 100644
--- a/app/assets/javascripts/discourse/app/models/tag.js
+++ b/app/assets/javascripts/discourse/app/models/tag.js
@@ -1,17 +1,15 @@
 import RestModel from "discourse/models/rest";
 import discourseComputed from "discourse-common/utils/decorators";
+import { readOnly } from "@ember/object/computed";
 
 export default RestModel.extend({
-  @discourseComputed("count", "pm_count")
-  totalCount(count, pmCount) {
-    return count + pmCount;
-  },
+  pmOnly: readOnly("pm_only"),
 
   @discourseComputed("count", "pm_count")
-  pmOnly(count, pmCount) {
-    return count === 0 && pmCount > 0;
+  totalCount(count, pmCount) {
+    return pmCount ? count + pmCount : count;
   },
 
   @discourseComputed("id")
   searchContext(id) {
     return { type: "tag", id, tag: this, name: id };
diff --git a/app/assets/javascripts/discourse/tests/helpers/create-pretender.js b/app/assets/javascripts/discourse/tests/helpers/create-pretender.js
index d694826cf524..25364f0ff0e6 100644
--- a/app/assets/javascripts/discourse/tests/helpers/create-pretender.js
+++ b/app/assets/javascripts/discourse/tests/helpers/create-pretender.js
@@ -98,37 +98,37 @@ export function applyDefaultHandlers(pretender) {
 
   pretender.get("/tags", () => {
     return response({
       tags: [
         { id: "eviltrout", count: 1 },
-        { id: "planned", text: "planned", count: 7, pm_count: 0 },
-        { id: "private", text: "private", count: 0, pm_count: 7 },
+        { id: "planned", text: "planned", count: 7, pm_only: false },
+        { id: "private", text: "private", count: 0, pm_only: true },
       ],
       extras: {
         tag_groups: [
           {
             id: 2,
             name: "Ford Cars",
             tags: [
-              { id: "Escort", text: "Escort", count: 1, pm_count: 0 },
-              { id: "focus", text: "focus", count: 3, pm_count: 0 },
+              { id: "Escort", text: "Escort", count: 1, pm_only: false },
+              { id: "focus", text: "focus", count: 3, pm_only: false },
             ],
           },
           {
             id: 1,
             name: "Honda Cars",
             tags: [
-              { id: "civic", text: "civic", count: 4, pm_count: 0 },
-              { id: "accord", text: "accord", count: 2, pm_count: 0 },
+              { id: "civic", text: "civic", count: 4, pm_only: false },
+              { id: "accord", text: "accord", count: 2, pm_only: false },
             ],
           },
           {
             id: 1,
             name: "Makes",
             tags: [
-              { id: "ford", text: "ford", count: 5, pm_count: 0 },
-              { id: "honda", text: "honda", count: 6, pm_count: 0 },
+              { id: "ford", text: "ford", count: 5, pm_only: false },
+              { id: "honda", text: "honda", count: 6, pm_only: false },
             ],
           },
         ],
       },
     });
diff --git a/app/assets/javascripts/discourse/tests/integration/components/select-kit/tag-drop-test.js b/app/assets/javascripts/discourse/tests/integration/components/select-kit/tag-drop-test.js
index 385c94991e72..e8ff49de4c3d 100644
--- a/app/assets/javascripts/discourse/tests/integration/components/select-kit/tag-drop-test.js
+++ b/app/assets/javascripts/discourse/tests/integration/components/select-kit/tag-drop-test.js
@@ -26,11 +26,11 @@ module("Integration | Component | select-kit/tag-drop", function (hooks) {
     this.site.set("top_tags", ["jeff", "neil", "arpit", "régis"]);
 
     pretender.get("/tags/filter/search", (params) => {
       if (params.queryParams.q === "dav") {
         return response({
-          results: [{ id: "David", name: "David", count: 2, pm_count: 0 }],
+          results: [{ id: "David", name: "David", count: 2, pm_only: false }],
         });
       }
     });
   });
 
diff --git a/app/assets/javascripts/discourse/tests/unit/models/tag-test.js b/app/assets/javascripts/discourse/tests/unit/models/tag-test.js
new file mode 100644
index 000000000000..c42364fa07ad
--- /dev/null
+++ b/app/assets/javascripts/discourse/tests/unit/models/tag-test.js
@@ -0,0 +1,31 @@
+import { module, test } from "qunit";
+import { getOwner } from "discourse-common/lib/get-owner";
+import { setupTest } from "ember-qunit";
+
+module("Unit | Model | tag", function (hooks) {
+  setupTest(hooks);
+
+  hooks.beforeEach(function () {
+    this.store = getOwner(this).lookup("service:store");
+  });
+
+  test("totalCount when pm_count is not present", function (assert) {
+    const tag = this.store.createRecord("tag", { count: 5 });
+    assert.strictEqual(tag.totalCount, 5);
+  });
+
+  test("totalCount when pm_count is present", function (assert) {
+    const tag = this.store.createRecord("tag", { count: 5, pm_count: 8 });
+    assert.strictEqual(tag.totalCount, 13);
+  });
+
+  test("pmOnly", function (assert) {
+    const tag = this.store.createRecord("tag", { pm_only: false });
+
+    assert.notOk(tag.pmOnly);
+
+    tag.set("pm_only", true);
+
+    assert.ok(tag.pmOnly);
+  });
+});
diff --git a/app/controllers/tags_controller.rb b/app/controllers/tags_controller.rb
index 6f9025414b42..f4b75c2330dd 100644
--- a/app/controllers/tags_controller.rb
+++ b/app/controllers/tags_controller.rb
@@ -399,20 +399,26 @@ class TagsController < ::ApplicationController
       .map do |t|
         topic_count = t.public_send(Tag.topic_count_column(guardian))
 
         next if topic_count == 0 && t.pm_topic_count > 0 && !show_pm_tags
 
-        {
+        attrs = {
           id: t.name,
           text: t.name,
           name: t.name,
           description: t.description,
           count: topic_count,
-          pm_count: show_pm_tags ? t.pm_topic_count : 0,
+          pm_only: topic_count == 0 && t.pm_topic_count > 0,
           target_tag:
             t.target_tag_id ? target_tags.find { |x| x.id == t.target_tag_id }&.name : nil,
         }
+
+        if show_pm_tags && SiteSetting.display_personal_messages_tag_counts
+          attrs[:pm_count] = t.pm_topic_count
+        end
+
+        attrs
       end
       .compact
   end
 
   def set_category
diff --git a/config/locales/server.en.yml b/config/locales/server.en.yml
index 695d67151ab2..bee38346f194 100644
--- a/config/locales/server.en.yml
+++ b/config/locales/server.en.yml
@@ -1650,10 +1650,11 @@ en:
     content_security_policy_collect_reports: "Enable CSP violation report collection at /csp_reports"
     content_security_policy_frame_ancestors: "Restrict who can embed this site in iframes via CSP. Control allowed hosts on <a href='%{base_path}/admin/customize/embedding'>Embedding</a>"
     content_security_policy_script_src: "Additional allowlisted script sources. The current host and CDN are included by default. See <a href='https://meta.discourse.org/t/mitigate-xss-attacks-with-content-security-policy/104243' target='_blank'>Mitigate XSS Attacks with Content Security Policy.</a>"
     invalidate_inactive_admin_email_after_days: "Admin accounts that have not visited the site in this number of days will need to re-validate their email address before logging in. Set to 0 to disable."
     include_secure_categories_in_tag_counts: "When enabled, count of topics for a tag will include topics that are in read restricted categories for all users. When disabled, normal users are only shown a count of topics for a tag where all the topics are in public categories."
+    display_personal_messages_tag_counts: "When enabled, count of personal messages tagged with a given tag will be displayed."
     top_menu: "Determine which items appear in the homepage navigation, and in what order. Example latest|new|unread|categories|top|read|posted|bookmarks"
     post_menu: "Determine which items appear on the post menu, and in what order. Example like|edit|flag|delete|share|bookmark|reply"
     post_menu_hidden_items: "The menu items to hide by default in the post menu unless an expansion ellipsis is clicked on."
     share_links: "Determine which items appear on the share dialog, and in what order."
     allow_username_in_share_links: "Allow usernames to be included in share links. This is useful to reward badges based on unique visitors."
diff --git a/config/site_settings.yml b/config/site_settings.yml
index 5b5acafaf297..1963eef26206 100644
--- a/config/site_settings.yml
+++ b/config/site_settings.yml
@@ -1804,10 +1804,12 @@ security:
   suppress_secured_categories_from_admin:
     default: false
     hidden: true
   include_secure_categories_in_tag_counts:
     default: false
+  display_personal_messages_tag_counts:
+    default: false
 
 onebox:
   post_onebox_maxlength:
     default: 500
     locale_default:
diff --git a/spec/requests/tags_controller_spec.rb b/spec/requests/tags_controller_spec.rb
index e3622bdaeae4..63b60c5c3157 100644
--- a/spec/requests/tags_controller_spec.rb
+++ b/spec/requests/tags_controller_spec.rb
@@ -10,13 +10,25 @@ RSpec.describe TagsController do
 
   before { SiteSetting.tagging_enabled = true }
 
   describe "#index" do
     fab!(:test_tag) { Fabricate(:tag, name: "test", description: "some description") }
+
     fab!(:topic_tag) do
-      Fabricate(:tag, name: "topic-test", public_topic_count: 1, staff_topic_count: 1)
+      Fabricate(
+        :tag,
+        name: "topic-test",
+        public_topic_count: 1,
+        staff_topic_count: 1,
+        pm_topic_count: 5,
+      )
+    end
+
+    fab!(:pm_only_tag) do
+      Fabricate(:tag, public_topic_count: 0, staff_topic_count: 0, pm_topic_count: 1)
     end
+
     fab!(:synonym) { Fabricate(:tag, name: "synonym", target_tag: topic_tag) }
 
     shared_examples "retrieves the right tags" do
       it "retrieves all tags as a staff user" do
         sign_in(admin)
@@ -25,17 +37,67 @@ RSpec.describe TagsController do
 
         expect(response.status).to eq(200)
 
         tags = response.parsed_body["tags"]
 
+        serialized_tag = tags.find { |t| t["id"] == test_tag.name }
+
+        expect(serialized_tag["count"]).to eq(0)
+        expect(serialized_tag["pm_count"]).to eq(nil)
+        expect(serialized_tag["pm_only"]).to eq(false)
+
+        serialized_tag = tags.find { |t| t["id"] == topic_tag.name }
+
+        expect(serialized_tag["count"]).to eq(1)
+        expect(serialized_tag["pm_count"]).to eq(nil)
+        expect(serialized_tag["pm_only"]).to eq(false)
+      end
+
+      it "does not include pm_count attribute when user cannot tag PM topics even if display_personal_messages_tag_counts site setting has been enabled" do
+        SiteSetting.display_personal_messages_tag_counts = true
+
+        sign_in(admin)
+
+        get "/tags.json"
+
+        expect(response.status).to eq(200)
+
+        tags = response.parsed_body["tags"]
+
         expect(tags[0]["name"]).to eq(test_tag.name)
-        expect(tags[0]["count"]).to eq(0)
-        expect(tags[0]["pm_count"]).to eq(0)
+        expect(tags[0]["pm_count"]).to eq(nil)
 
         expect(tags[1]["name"]).to eq(topic_tag.name)
-        expect(tags[1]["count"]).to eq(1)
-        expect(tags[1]["pm_count"]).to eq(0)
+        expect(tags[1]["pm_count"]).to eq(nil)
+      end
+
+      it "includes pm_count attribute when user can tag PM topics and display_personal_messages_tag_counts site setting has been enabled" do
+        SiteSetting.display_personal_messages_tag_counts = true
+        SiteSetting.pm_tags_allowed_for_groups = Group::AUTO_GROUPS[:admins]
+
+        sign_in(admin)
+
+        get "/tags.json"
+
+        expect(response.status).to eq(200)
+
+        tags = response.parsed_body["tags"]
+
+        serialized_tag = tags.find { |t| t["id"] == test_tag.name }
+
+        expect(serialized_tag["pm_count"]).to eq(0)
+        expect(serialized_tag["pm_only"]).to eq(false)
+
+        serialized_tag = tags.find { |t| t["id"] == topic_tag.name }
+
+        expect(serialized_tag["pm_count"]).to eq(5)
+        expect(serialized_tag["pm_only"]).to eq(false)
+
+        serialized_tag = tags.find { |t| t["id"] == pm_only_tag.name }
+
+        expect(serialized_tag["pm_count"]).to eq(1)
+        expect(serialized_tag["pm_only"]).to eq(true)
       end
 
       it "only retrieve tags that have been used in public topics for non-staff user" do
         sign_in(user)
 
@@ -46,11 +108,11 @@ RSpec.describe TagsController do
         tags = response.parsed_body["tags"]
         expect(tags.length).to eq(1)
 
         expect(tags[0]["name"]).to eq(topic_tag.name)
         expect(tags[0]["count"]).to eq(1)
-        expect(tags[0]["pm_count"]).to eq(0)
+        expect(tags[0]["pm_count"]).to eq(nil)
       end
     end
 
     context "with pm_tags_allowed_for_groups" do
       fab!(:admin) { Fabricate(:admin) }
@@ -64,21 +126,21 @@ RSpec.describe TagsController do
       end
 
       context "when enabled" do
         before do
           SiteSetting.pm_tags_allowed_for_groups = "1|2|3"
+          SiteSetting.display_personal_messages_tag_counts = true
           sign_in(admin)
         end
 
         it "shows topic tags and pm tags" do
           get "/tags.json"
           tags = response.parsed_body["tags"]
-          expect(tags.length).to eq(2)
 
           serialized_tag = tags.find { |t| t["id"] == topic_tag.name }
           expect(serialized_tag["count"]).to eq(2)
-          expect(serialized_tag["pm_count"]).to eq(0)
+          expect(serialized_tag["pm_count"]).to eq(5)
 
           serialized_tag = tags.find { |t| t["id"] == test_tag.name }
           expect(serialized_tag["count"]).to eq(0)
           expect(serialized_tag["pm_count"]).to eq(1)
         end
@@ -178,11 +240,11 @@ RSpec.describe TagsController do
 
         expect(tags[0]["name"]).to eq(test_tag.name)
         expect(tags[0]["text"]).to eq(test_tag.name)
         expect(tags[0]["description"]).to eq(test_tag.description)
         expect(tags[0]["count"]).to eq(0)
-        expect(tags[0]["pm_count"]).to eq(0)
+        expect(tags[0]["pm_count"]).to eq(nil)
         expect(tags[0]["target_tag"]).to eq(nil)
 
         expect(tags[1]["name"]).to eq(topic_tag.name)
 
         categories = response.parsed_body["extras"]["categories"]
@@ -191,11 +253,11 @@ RSpec.describe TagsController do
         expect(categories[0]["tags"].length).to eq(1)
         expect(categories[0]["tags"][0]["name"]).to eq(test_tag.name)
         expect(categories[0]["tags"][0]["text"]).to eq(test_tag.name)
         expect(categories[0]["tags"][0]["description"]).to eq(test_tag.description)
         expect(categories[0]["tags"][0]["count"]).to eq(0)
-        expect(categories[0]["tags"][0]["pm_count"]).to eq(0)
+        expect(categories[0]["tags"][0]["pm_count"]).to eq(nil)
         expect(categories[0]["tags"][0]["target_tag"]).to eq(nil)
       end
 
       it "does not result in N+1 queries when there are multiple categories configured with tags for an admin user" do
         category.update!(tags: [test_tag])

Vulnerable code:   @discourseComputed("count", "pm_count")
  totalCount(count, pmCount) {
    return count + pmCount;
  },
  pmOnly(count, pmCount) {
    return count === 0 && pmCount > 0;
        { id: "planned", text: "planned", count: 7, pm_count: 0 },
        { id: "private", text: "private", count: 0, pm_count: 7 },
              { id: "Escort", text: "Escort", count: 1, pm_count: 0 },
              { id: "focus", text: "focus", count: 3, pm_count: 0 },
              { id: "civic", text: "civic", count: 4, pm_count: 0 },
              { id: "accord", text: "accord", count: 2, pm_count: 0 },
              { id: "ford", text: "ford", count: 5, pm_count: 0 },
              { id: "honda", text: "honda", count: 6, pm_count: 0 },
          results: [{ id: "David", name: "David", count: 2, pm_count: 0 }],
        {
          pm_count: show_pm_tags ? t.pm_topic_count : 0,
      Fabricate(:tag, name: "topic-test", public_topic_count: 1, staff_topic_count: 1)
        expect(tags[0]["count"]).to eq(0)
        expect(tags[0]["pm_count"]).to eq(0)
        expect(tags[1]["count"]).to eq(1)
        expect(tags[1]["pm_count"]).to eq(0)
        expect(tags[0]["pm_count"]).to eq(0)
          expect(tags.length).to eq(2)
          expect(serialized_tag["pm_count"]).to eq(0)
        expect(tags[0]["pm_count"]).to eq(0)
        expect(categories[0]["tags"][0]["pm_count"]).to eq(0)
Fixed code: import { readOnly } from "@ember/object/computed";
  pmOnly: readOnly("pm_only"),
  totalCount(count, pmCount) {
    return pmCount ? count + pmCount : count;
        { id: "planned", text: "planned", count: 7, pm_only: false },
        { id: "private", text: "private", count: 0, pm_only: true },
              { id: "Escort", text: "Escort", count: 1, pm_only: false },
              { id: "focus", text: "focus", count: 3, pm_only: false },
              { id: "civic", text: "civic", count: 4, pm_only: false },
              { id: "accord", text: "accord", count: 2, pm_only: false },
              { id: "ford", text: "ford", count: 5, pm_only: false },
              { id: "honda", text: "honda", count: 6, pm_only: false },
          results: [{ id: "David", name: "David", count: 2, pm_only: false }],
import { module, test } from "qunit";
import { getOwner } from "discourse-common/lib/get-owner";
import { setupTest } from "ember-qunit";

module("Unit | Model | tag", function (hooks) {
  setupTest(hooks);

  hooks.beforeEach(function () {
    this.store = getOwner(this).lookup("service:store");
  });

  test("totalCount when pm_count is not present", function (assert) {
    const tag = this.store.createRecord("tag", { count: 5 });
    assert.strictEqual(tag.totalCount, 5);
  });

  test("totalCount when pm_count is present", function (assert) {
    const tag = this.store.createRecord("tag", { count: 5, pm_count: 8 });
    assert.strictEqual(tag.totalCount, 13);
  });

  test("pmOnly", function (assert) {
    const tag = this.store.createRecord("tag", { pm_only: false });

    assert.notOk(tag.pmOnly);

    tag.set("pm_only", true);

    assert.ok(tag.pmOnly);
  });
});
        attrs = {
          pm_only: topic_count == 0 && t.pm_topic_count > 0,

        if show_pm_tags && SiteSetting.display_personal_messages_tag_counts
          attrs[:pm_count] = t.pm_topic_count
        end

        attrs
    display_personal_messages_tag_counts: "When enabled, count of personal messages tagged with a given tag will be displayed."
  display_personal_messages_tag_counts:
    default: false

      Fabricate(
        :tag,
        name: "topic-test",
        public_topic_count: 1,
        staff_topic_count: 1,
        pm_topic_count: 5,
      )
    end

    fab!(:pm_only_tag) do
      Fabricate(:tag, public_topic_count: 0, staff_topic_count: 0, pm_topic_count: 1)

        serialized_tag = tags.find { |t| t["id"] == test_tag.name }

        expect(serialized_tag["count"]).to eq(0)
        expect(serialized_tag["pm_count"]).to eq(nil)
        expect(serialized_tag["pm_only"]).to eq(false)

        serialized_tag = tags.find { |t| t["id"] == topic_tag.name }

        expect(serialized_tag["count"]).to eq(1)
        expect(serialized_tag["pm_count"]).to eq(nil)
        expect(serialized_tag["pm_only"]).to eq(false)
      end

      it "does not include pm_count attribute when user cannot tag PM topics even if display_personal_messages_tag_counts site setting has been enabled" do
        SiteSetting.display_personal_messages_tag_counts = true

        sign_in(admin)

        get "/tags.json"

        expect(response.status).to eq(200)

        tags = response.parsed_body["tags"]

        expect(tags[0]["pm_count"]).to eq(nil)
        expect(tags[1]["pm_count"]).to eq(nil)
      end

      it "includes pm_count attribute when user can tag PM topics and display_personal_messages_tag_counts site setting has been enabled" do
        SiteSetting.display_personal_messages_tag_counts = true
        SiteSetting.pm_tags_allowed_for_groups = Group::AUTO_GROUPS[:admins]

        sign_in(admin)

        get "/tags.json"

        expect(response.status).to eq(200)

        tags = response.parsed_body["tags"]

        serialized_tag = tags.find { |t| t["id"] == test_tag.name }

        expect(serialized_tag["pm_count"]).to eq(0)
        expect(serialized_tag["pm_only"]).to eq(false)

        serialized_tag = tags.find { |t| t["id"] == topic_tag.name }

        expect(serialized_tag["pm_count"]).to eq(5)
        expect(serialized_tag["pm_only"]).to eq(false)

        serialized_tag = tags.find { |t| t["id"] == pm_only_tag.name }

        expect(serialized_tag["pm_count"]).to eq(1)
        expect(serialized_tag["pm_only"]).to eq(true)
        expect(tags[0]["pm_count"]).to eq(nil)
          SiteSetting.display_personal_messages_tag_counts = true
          expect(serialized_tag["pm_count"]).to eq(5)
        expect(tags[0]["pm_count"]).to eq(nil)
        expect(categories[0]["tags"][0]["pm_count"]).to eq(nil)