CVE-2024-24748: Insufficient Information (NVD-CWE-noinfo)
value: Discourse is an open source platform for community discussion. In affected versions an attacker can learn that a secret subcategory exists under a public category which has no public subcategories. The issue is patched in the latest stable, beta and tests-passed version of Discourse. Users are advised to upgrade. There are no known workarounds for this vulnerability.
CWE description: There is insufficient information about the issue to classify it; details are unkown or unspecified.
Published: 2024-03-15T20:15Z
Repository: https://github.com/discourse/discourse
Language: Ruby
Files changed: app/models/category_list.rb, spec/models/category_list_spec.rb
Commit date: 2024-03-15 14:23:55 +0800
Commit message: SECURITY: Don't disclose the existence of secret subcategories

Diff: diff --git a/app/models/category_list.rb b/app/models/category_list.rb
index aba935d8fb66..8004fd257022 100644
--- a/app/models/category_list.rb
+++ b/app/models/category_list.rb
@@ -207,16 +207,23 @@ class CategoryList
       @categories.delete_if { |c| to_delete.include?(c) }
     end
 
     allowed_topic_create = Set.new(Category.topic_create_allowed(@guardian).pluck(:id))
 
+    parent_ids =
+      Category
+        .secured(@guardian)
+        .where(parent_category_id: categories_with_descendants.map(&:id))
+        .pluck("DISTINCT parent_category_id")
+        .to_set
+
     categories_with_descendants.each do |category|
       category.notification_level = notification_levels[category.id] || default_notification_level
       category.permission = CategoryGroup.permission_types[:full] if allowed_topic_create.include?(
         category.id,
       )
-      category.has_children = category.subcategories.present?
+      category.has_children = parent_ids.include?(category.id)
     end
   end
 
   def prune_empty
     return if SiteSetting.allow_uncategorized_topics
diff --git a/spec/models/category_list_spec.rb b/spec/models/category_list_spec.rb
index c5495f6c2b8b..ff08f054b3f3 100644
--- a/spec/models/category_list_spec.rb
+++ b/spec/models/category_list_spec.rb
@@ -10,10 +10,33 @@ RSpec.describe CategoryList do
 
   fab!(:user)
   fab!(:admin)
   let(:category_list) { CategoryList.new(Guardian.new(user), include_topics: true) }
 
+  context "when a category has a secret subcategory" do
+    fab!(:category)
+
+    fab!(:secret_subcategory) do
+      cat = Fabricate(:category, parent_category: category)
+      cat.set_permissions(admins: :full)
+      cat.save!
+      cat
+    end
+
+    let(:admin_category_list) { CategoryList.new(Guardian.new(admin), include_topics: true) }
+
+    it "doesn't set has_children when an unpriveleged user is querying" do
+      found = category_list.categories.find { |c| c.id == category.id }
+      expect(found.has_children).to eq(false)
+    end
+
+    it "sets has_children when an admin is querying" do
+      found = admin_category_list.categories.find { |c| c.id == category.id }
+      expect(found.has_children).to eq(true)
+    end
+  end
+
   describe "security" do
     it "properly hide secure categories" do
       cat = Fabricate(:category_with_definition)
       Fabricate(:topic, category: cat)
       cat.set_permissions(admins: :full)

Vulnerable code:       category.has_children = category.subcategories.present?
Fixed code:     parent_ids =
      Category
        .secured(@guardian)
        .where(parent_category_id: categories_with_descendants.map(&:id))
        .pluck("DISTINCT parent_category_id")
        .to_set

      category.has_children = parent_ids.include?(category.id)
  context "when a category has a secret subcategory" do
    fab!(:category)

    fab!(:secret_subcategory) do
      cat = Fabricate(:category, parent_category: category)
      cat.set_permissions(admins: :full)
      cat.save!
      cat
    end

    let(:admin_category_list) { CategoryList.new(Guardian.new(admin), include_topics: true) }

    it "doesn't set has_children when an unpriveleged user is querying" do
      found = category_list.categories.find { |c| c.id == category.id }
      expect(found.has_children).to eq(false)
    end

    it "sets has_children when an admin is querying" do
      found = admin_category_list.categories.find { |c| c.id == category.id }
      expect(found.has_children).to eq(true)
    end
  end