CVE-2023-38685: Insufficient Information (NVD-CWE-noinfo)
value: Discourse is an open source discussion platform. Prior to version 3.0.6 of the `stable` branch and version 3.1.0.beta7 of the `beta` and `tests-passed` branches, information about restricted-visibility topic tags could be obtained by unauthorized users. The issue is patched in version 3.0.6 of the `stable` branch and version 3.1.0.beta7 of the `beta` and `tests-passed` branches.
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-07-28T16:15Z
Repository: https://github.com/discourse/discourse
Language: Ruby
Files changed: app/models/topic.rb, app/serializers/concerns/topic_tags_mixin.rb, app/views/list/list.erb, spec/requests/list_controller_spec.rb
Commit date: 2023-07-28 12:53:50 +0100
Commit message: SECURITY: Hide restricted tags in noscript view

The hidden tags are usually filtered out by the serializer, but the
noscript view uses the topic objects instead of the serialized objects.

Diff: diff --git a/app/models/topic.rb b/app/models/topic.rb
index 0d824b5576f1..d8f458f97281 100644
--- a/app/models/topic.rb
+++ b/app/models/topic.rb
@@ -2043,10 +2043,14 @@ class Topic < ActiveRecord::Base
 
   def group_pm?
     private_message? && all_allowed_users.count > 2
   end
 
+  def visible_tags(guardian)
+    tags.reject { |tag| guardian.hidden_tag_names.include?(tag[:name]) }
+  end
+
   private
 
   def invite_to_private_message(invited_by, target_user, guardian)
     if !guardian.can_send_private_message?(target_user)
       raise UserExists.new(I18n.t("activerecord.errors.models.topic.attributes.base.cant_send_pm"))
diff --git a/app/serializers/concerns/topic_tags_mixin.rb b/app/serializers/concerns/topic_tags_mixin.rb
index 0c001d46334d..493ec0f6ccee 100644
--- a/app/serializers/concerns/topic_tags_mixin.rb
+++ b/app/serializers/concerns/topic_tags_mixin.rb
@@ -24,19 +24,22 @@ module TopicTagsMixin
 
   private
 
   def all_tags
     return @tags if defined?(@tags)
+
+    tags = topic.visible_tags(scope)
+
     # Calling method `pluck` or `order` along with `includes` causing N+1 queries
     tags =
       (
         if SiteSetting.tags_sort_alphabetically
-          topic.tags.sort_by(&:name)
+          tags.sort_by(&:name)
         else
           topic_count_column = Tag.topic_count_column(scope)
-          topic.tags.sort_by { |tag| tag.public_send(topic_count_column) }.reverse
+          tags.sort_by { |tag| tag.public_send(topic_count_column) }.reverse
         end
       )
-    tags = tags.reject { |tag| scope.hidden_tag_names.include?(tag[:name]) } if !scope.is_staff?
+
     @tags = tags
   end
 end
diff --git a/app/views/list/list.erb b/app/views/list/list.erb
index e2b6a3471a3b..e322560d2335 100644
--- a/app/views/list/list.erb
+++ b/app/views/list/list.erb
@@ -68,15 +68,15 @@
                   <span class='badge-category clear-badge'>
                     <span class='category-name'><%= t.category.name %></span>
                   </span>
                 </a>
               <% end %>
-              <% if t.tags %>
+              <% if tags = t.visible_tags(guardian) %>
                 <div class="discourse-tags">
-                  <% t.tags.each_with_index do |tag, index| %>
+                  <% tags.each_with_index do |tag, index| %>
                     <a href='<%= tag.full_url %>' class='discourse-tag'><%= tag.name %></a>
-                    <% if index < t.tags.size - 1 %>,&nbsp;<% end %>
+                    <% if index < tags.size - 1 %>,&nbsp;<% end %>
                   <% end %>
                 </div>
               <% end %>
             </div>
             <% if t.pinned_until && (t.pinned_until > Time.zone.now) && (t.pinned_globally || @list.category) && t.excerpt %>
diff --git a/spec/requests/list_controller_spec.rb b/spec/requests/list_controller_spec.rb
index 7a7682dc3fff..e332bdfa85f7 100644
--- a/spec/requests/list_controller_spec.rb
+++ b/spec/requests/list_controller_spec.rb
@@ -193,10 +193,32 @@ RSpec.describe ListController do
           ).to contain_exactly(user.id, user2.id, user3.id)
         end.count
 
       expect(new_sql_queries_count).to be <= initial_sql_queries_count
     end
+
+    context "with topics with tags" do
+      let(:tag_group) { Fabricate.build(:tag_group) }
+      let(:tag_group_permission) { Fabricate.build(:tag_group_permission, tag_group: tag_group) }
+      let(:restricted_tag) { Fabricate(:tag) }
+      let(:public_tag) { Fabricate(:tag) }
+
+      before do
+        tag_group.tag_group_permissions << tag_group_permission
+        tag_group.save!
+        tag_group_permission.tag_group.tags << restricted_tag
+        topic.tags << [public_tag, restricted_tag]
+      end
+
+      it "does not show hidden tags" do
+        get "/latest"
+
+        expect(response.status).to eq(200)
+        expect(response.body).to include(public_tag.name)
+        expect(response.body).not_to include(restricted_tag.name)
+      end
+    end
   end
 
   describe "categories and X" do
     let(:category) { Fabricate(:category_with_definition) }
     let(:sub_category) { Fabricate(:category_with_definition, parent_category: category) }

Vulnerable code:           topic.tags.sort_by(&:name)
          topic.tags.sort_by { |tag| tag.public_send(topic_count_column) }.reverse
    tags = tags.reject { |tag| scope.hidden_tag_names.include?(tag[:name]) } if !scope.is_staff?
              <% if t.tags %>
                  <% t.tags.each_with_index do |tag, index| %>
                    <% if index < t.tags.size - 1 %>,&nbsp;<% end %>
Fixed code:   def visible_tags(guardian)
    tags.reject { |tag| guardian.hidden_tag_names.include?(tag[:name]) }
  end


    tags = topic.visible_tags(scope)

          tags.sort_by(&:name)
          tags.sort_by { |tag| tag.public_send(topic_count_column) }.reverse

              <% if tags = t.visible_tags(guardian) %>
                  <% tags.each_with_index do |tag, index| %>
                    <% if index < tags.size - 1 %>,&nbsp;<% end %>

    context "with topics with tags" do
      let(:tag_group) { Fabricate.build(:tag_group) }
      let(:tag_group_permission) { Fabricate.build(:tag_group_permission, tag_group: tag_group) }
      let(:restricted_tag) { Fabricate(:tag) }
      let(:public_tag) { Fabricate(:tag) }

      before do
        tag_group.tag_group_permissions << tag_group_permission
        tag_group.save!
        tag_group_permission.tag_group.tags << restricted_tag
        topic.tags << [public_tag, restricted_tag]
      end

      it "does not show hidden tags" do
        get "/latest"

        expect(response.status).to eq(200)
        expect(response.body).to include(public_tag.name)
        expect(response.body).not_to include(restricted_tag.name)
      end
    end