CVE-2022-24782: Exposure of Sensitive Information to an Unauthorized Actor (CWE-200)
value: Discourse is an open source discussion platform. Versions 2.8.2 and prior in the `stable` branch, 2.9.0.beta3 and prior in the `beta` branch, and 2.9.0.beta3 and prior in the `tests-passed` branch are vulnerable to a data leak. Users can request an export of their own activity. Sometimes, due to category settings, they may have category membership for a secure category. The name of this secure category is shown to the user in the export. The same thing occurs when the user's post has been moved to a secure category. A patch for this issue is available in the `main` branch of Discourse's GitHub repository and is anticipated to be part of future releases.
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: 4.3
Published: 2022-03-24T21:15Z
Repository: https://github.com/discourse/discourse
Language: Ruby
Files changed: app/jobs/regular/export_user_archive.rb, spec/jobs/export_user_archive_spec.rb
Commit date: 2022-03-24 15:38:44 +1000
Commit message: SECURITY: Hide private categories in user activity export (#16273)

In some of the user's own activity export data,
we sometimes showed a secure category's name or
exposed the existence of a secure category.
Diff: diff --git a/app/jobs/regular/export_user_archive.rb b/app/jobs/regular/export_user_archive.rb
index ba2442df7745..489dac28b23d 100644
--- a/app/jobs/regular/export_user_archive.rb
+++ b/app/jobs/regular/export_user_archive.rb
@@ -259,19 +259,20 @@ module Jobs
     def category_preferences_export
       return enum_for(:category_preferences_export) unless block_given?
 
       CategoryUser
         .where(user_id: @current_user.id)
-        .select(:category_id, :notification_level, :last_seen_at)
+        .includes(:category)
+        .merge(Category.secured(guardian))
         .each do |cu|
-        yield [
-          cu.category_id,
-          piped_category_name(cu.category_id),
-          NotificationLevels.all[cu.notification_level],
-          cu.last_seen_at
-        ]
-      end
+          yield [
+            cu.category_id,
+            piped_category_name(cu.category_id, cu.category),
+            NotificationLevels.all[cu.notification_level],
+            cu.last_seen_at
+          ]
+        end
     end
 
     def flags_export
       return enum_for(:flags_export) unless block_given?
 
@@ -406,14 +407,13 @@ module Jobs
 
     def guardian
       @guardian ||= Guardian.new(@current_user)
     end
 
-    def piped_category_name(category_id)
-      return "-" unless category_id
-      category = Category.find_by(id: category_id)
-      return "#{category_id}" unless category
+    def piped_category_name(category_id, category)
+      return "#{category_id}" if category_id && !category
+      return "-" if !guardian.can_see_category?(category)
       categories = [category.name]
       while category.parent_category_id && category = category.parent_category
         categories << category.name
       end
       categories.reverse.join("|")
@@ -431,14 +431,14 @@ module Jobs
 
     def get_user_archive_fields(user_archive)
       user_archive_array = []
       topic_data = user_archive.topic
       user_archive = user_archive.as_json
-      topic_data = Topic.with_deleted.find_by(id: user_archive['topic_id']) if topic_data.nil?
+      topic_data = Topic.with_deleted.includes(:category).find_by(id: user_archive['topic_id']) if topic_data.nil?
       return user_archive_array if topic_data.nil?
 
-      categories = piped_category_name(topic_data.category_id)
+      categories = piped_category_name(topic_data.category_id, topic_data.category)
       is_pm = topic_data.archetype == "private_message" ? I18n.t("csv_export.boolean_yes") : I18n.t("csv_export.boolean_no")
       url = "#{Discourse.base_url}/t/#{topic_data.slug}/#{topic_data.id}/#{user_archive['post_number']}"
 
       topic_hash = { "post" => user_archive['raw'], "topic_title" => topic_data.title, "categories" => categories, "is_pm" => is_pm, "url" => url }
       user_archive.merge!(topic_hash)
diff --git a/spec/jobs/export_user_archive_spec.rb b/spec/jobs/export_user_archive_spec.rb
index ea10fdf94520..b9e8514d73c5 100644
--- a/spec/jobs/export_user_archive_spec.rb
+++ b/spec/jobs/export_user_archive_spec.rb
@@ -13,11 +13,11 @@ describe Jobs::ExportUserArchive do
     j
   }
   let(:component) { raise 'component not set' }
 
   fab!(:admin) { Fabricate(:admin) }
-  fab!(:category) { Fabricate(:category_with_definition) }
+  fab!(:category) { Fabricate(:category_with_definition, name: "User Archive Category") }
   fab!(:subcategory) { Fabricate(:category_with_definition, parent_category_id: category.id) }
   fab!(:topic) { Fabricate(:topic, category: category) }
   let(:post) { Fabricate(:post, user: user, topic: topic) }
 
   def make_component_csv
@@ -166,10 +166,21 @@ describe Jobs::ExportUserArchive do
       cat2.destroy!
 
       _, csv_out = make_component_csv
       expect(csv_out).to match cat2_id.to_s
     end
+
+    it "can export a post from a secure category, obscuring the category name" do
+      cat2 = Fabricate(:private_category, group: Fabricate(:group), name: "Secret Cat")
+      topic2 = Fabricate(:topic, category: cat2, user: user, title: "This is a test secure topic")
+      _post2 = Fabricate(:post, topic: topic2, user: user)
+      data, csv_out = make_component_csv
+      expect(csv_out).not_to match "Secret Cat"
+      expect(data.length).to eq(1)
+      expect(data[0][:topic_title]).to eq("This is a test secure topic")
+      expect(data[0][:categories]).to eq("-")
+    end
   end
 
   context 'preferences' do
     let(:component) { 'preferences' }
 
@@ -312,13 +323,15 @@ describe Jobs::ExportUserArchive do
   end
 
   context 'category_preferences' do
     let(:component) { 'category_preferences' }
 
-    let(:subsubcategory) { Fabricate(:category_with_definition, parent_category_id: subcategory.id) }
-    let(:announcements) { Fabricate(:category_with_definition) }
-    let(:deleted_category) { Fabricate(:category) }
+    let(:subsubcategory) { Fabricate(:category_with_definition, parent_category_id: subcategory.id, name: "User Archive Subcategory") }
+    let(:announcements) { Fabricate(:category_with_definition, name: "Announcements") }
+    let(:deleted_category) { Fabricate(:category, name: "Deleted Category") }
+    let(:secure_category_group) { Fabricate(:group) }
+    let(:secure_category) { Fabricate(:private_category, group: secure_category_group, name: "Super Secret Category") }
 
     let(:reset_at) { DateTime.parse('2017-03-01 12:00') }
 
     before do
       SiteSetting.max_category_nesting = 3
@@ -339,15 +352,16 @@ describe Jobs::ExportUserArchive do
       CategoryUser.set_notification_level_for_category(user, NotificationLevels.all[:muted], deleted_category.id)
 
       deleted_category.destroy!
     end
 
-    it 'correctly exports the CategoryUser table' do
+    it 'correctly exports the CategoryUser table, excluding deleted categories' do
       data, _csv_out = make_component_csv
 
-      expect(data.find { |r| r['category_id'] == category.id }).to be_nil
-      expect(data.length).to eq(4)
+      expect(data.find { |r| r['category_id'] == category.id.to_s }).to be_nil
+      expect(data.find { |r| r['category_id'] == deleted_category.id.to_s }).to be_nil
+      expect(data.length).to eq(3)
       data.sort! { |a, b| a['category_id'].to_i <=> b['category_id'].to_i }
 
       expect(data[0][:category_id]).to eq(subcategory.id.to_s)
       expect(data[0][:notification_level].to_s).to eq('tracking')
       expect(DateTime.parse(data[0][:dismiss_new_timestamp])).to eq(reset_at)
@@ -359,12 +373,27 @@ describe Jobs::ExportUserArchive do
 
       expect(data[2][:category_id]).to eq(announcements.id.to_s)
       expect(data[2][:category_names]).to eq(announcements.name)
       expect(data[2][:notification_level]).to eq('watching_first_post')
       expect(data[2][:dismiss_new_timestamp]).to eq('')
+    end
 
-      expect(data[3][:category_names]).to eq(data[3][:category_id])
+    it "does not include any secure categories the user does not have access to, even if the user has a CategoryUser record" do
+      CategoryUser.set_notification_level_for_category(user, NotificationLevels.all[:muted], secure_category.id)
+      data, _csv_out = make_component_csv
+
+      expect(data.any? { |r| r['category_id'] == secure_category.id.to_s }).to eq(false)
+      expect(data.length).to eq(3)
+    end
+
+    it "does include secure categories that the user has access to" do
+      CategoryUser.set_notification_level_for_category(user, NotificationLevels.all[:muted], secure_category.id)
+      GroupUser.create!(user: user, group: secure_category_group)
+      data, _csv_out = make_component_csv
+
+      expect(data.any? { |r| r['category_id'] == secure_category.id.to_s }).to eq(true)
+      expect(data.length).to eq(4)
     end
   end
 
   context 'flags' do
     let(:component) { 'flags' }

Vulnerable code:         .select(:category_id, :notification_level, :last_seen_at)
        yield [
          cu.category_id,
          piped_category_name(cu.category_id),
          NotificationLevels.all[cu.notification_level],
          cu.last_seen_at
        ]
      end
    def piped_category_name(category_id)
      return "-" unless category_id
      category = Category.find_by(id: category_id)
      return "#{category_id}" unless category
      topic_data = Topic.with_deleted.find_by(id: user_archive['topic_id']) if topic_data.nil?
      categories = piped_category_name(topic_data.category_id)
  fab!(:category) { Fabricate(:category_with_definition) }
    let(:subsubcategory) { Fabricate(:category_with_definition, parent_category_id: subcategory.id) }
    let(:announcements) { Fabricate(:category_with_definition) }
    let(:deleted_category) { Fabricate(:category) }
    it 'correctly exports the CategoryUser table' do
      expect(data.find { |r| r['category_id'] == category.id }).to be_nil
      expect(data.length).to eq(4)
      expect(data[3][:category_names]).to eq(data[3][:category_id])
Fixed code:         .includes(:category)
        .merge(Category.secured(guardian))
          yield [
            cu.category_id,
            piped_category_name(cu.category_id, cu.category),
            NotificationLevels.all[cu.notification_level],
            cu.last_seen_at
          ]
        end
    def piped_category_name(category_id, category)
      return "#{category_id}" if category_id && !category
      return "-" if !guardian.can_see_category?(category)
      topic_data = Topic.with_deleted.includes(:category).find_by(id: user_archive['topic_id']) if topic_data.nil?
      categories = piped_category_name(topic_data.category_id, topic_data.category)
  fab!(:category) { Fabricate(:category_with_definition, name: "User Archive Category") }

    it "can export a post from a secure category, obscuring the category name" do
      cat2 = Fabricate(:private_category, group: Fabricate(:group), name: "Secret Cat")
      topic2 = Fabricate(:topic, category: cat2, user: user, title: "This is a test secure topic")
      _post2 = Fabricate(:post, topic: topic2, user: user)
      data, csv_out = make_component_csv
      expect(csv_out).not_to match "Secret Cat"
      expect(data.length).to eq(1)
      expect(data[0][:topic_title]).to eq("This is a test secure topic")
      expect(data[0][:categories]).to eq("-")
    end
    let(:subsubcategory) { Fabricate(:category_with_definition, parent_category_id: subcategory.id, name: "User Archive Subcategory") }
    let(:announcements) { Fabricate(:category_with_definition, name: "Announcements") }
    let(:deleted_category) { Fabricate(:category, name: "Deleted Category") }
    let(:secure_category_group) { Fabricate(:group) }
    let(:secure_category) { Fabricate(:private_category, group: secure_category_group, name: "Super Secret Category") }
    it 'correctly exports the CategoryUser table, excluding deleted categories' do
      expect(data.find { |r| r['category_id'] == category.id.to_s }).to be_nil
      expect(data.find { |r| r['category_id'] == deleted_category.id.to_s }).to be_nil
      expect(data.length).to eq(3)
    end
    it "does not include any secure categories the user does not have access to, even if the user has a CategoryUser record" do
      CategoryUser.set_notification_level_for_category(user, NotificationLevels.all[:muted], secure_category.id)
      data, _csv_out = make_component_csv

      expect(data.any? { |r| r['category_id'] == secure_category.id.to_s }).to eq(false)
      expect(data.length).to eq(3)
    end

    it "does include secure categories that the user has access to" do
      CategoryUser.set_notification_level_for_category(user, NotificationLevels.all[:muted], secure_category.id)
      GroupUser.create!(user: user, group: secure_category_group)
      data, _csv_out = make_component_csv

      expect(data.any? { |r| r['category_id'] == secure_category.id.to_s }).to eq(true)
      expect(data.length).to eq(4)