CVE-2023-36818: Insufficient Information (NVD-CWE-noinfo)
value: Discourse is an open source discussion platform. In affected versions a request to create or update custom sidebar section can cause a denial of service. This issue has been patched in commit `52b003d915`. 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.
CVSS v3 base score: 7.5
Published: 2023-07-14T22:15Z
Repository: https://github.com/discourse/discourse
Language: Ruby
Files changed: app/controllers/sidebar_sections_controller.rb, app/models/sidebar_section.rb, config/site_settings.yml, spec/requests/sidebar_sections_controller_spec.rb
Commit date: 2023-07-11 15:25:01 -0600
Commit message: SECURITY: limit amount of links in custom sidebar section (#22543)

Custom sidebar section can have maximum of 50 links

Co-authored-by: Krzysztof Kotlarek <kotlarek.krzysztof@gmail.com>
Diff: diff --git a/app/controllers/sidebar_sections_controller.rb b/app/controllers/sidebar_sections_controller.rb
index 0c9a919d18f3..5e84793d841d 100644
--- a/app/controllers/sidebar_sections_controller.rb
+++ b/app/controllers/sidebar_sections_controller.rb
@@ -27,10 +27,12 @@ class SidebarSectionsController < ApplicationController
     end
 
     render_serialized(sidebar_section, SidebarSectionSerializer)
   rescue ActiveRecord::RecordInvalid => e
     render_json_error(e.record.errors.full_messages.first)
+  rescue ActiveRecord::NestedAttributes::TooManyRecords => e
+    render_json_error(e.message)
   end
 
   def update
     sidebar_section = SidebarSection.find_by(id: section_params["id"])
     @guardian.ensure_can_edit!(sidebar_section)
@@ -60,10 +62,12 @@ class SidebarSectionsController < ApplicationController
     end
 
     render_serialized(sidebar_section.reload, SidebarSectionSerializer)
   rescue ActiveRecord::RecordInvalid => e
     render_json_error(e.record.errors.full_messages.first)
+  rescue ActiveRecord::NestedAttributes::TooManyRecords => e
+    render_json_error(e.message)
   rescue Discourse::InvalidAccess
     render json: failed_json, status: 403
   end
 
   def reset
diff --git a/app/models/sidebar_section.rb b/app/models/sidebar_section.rb
index 0162ca5fe4b9..3b564c3faf00 100644
--- a/app/models/sidebar_section.rb
+++ b/app/models/sidebar_section.rb
@@ -9,11 +9,13 @@ class SidebarSection < ActiveRecord::Base
   has_many :sidebar_urls,
            through: :sidebar_section_links,
            source: :linkable,
            source_type: "SidebarUrl"
 
-  accepts_nested_attributes_for :sidebar_urls, allow_destroy: true
+  accepts_nested_attributes_for :sidebar_urls,
+                                allow_destroy: true,
+                                limit: -> { SiteSetting.max_sidebar_section_links }
 
   before_save :set_system_user_for_public_section
 
   validates :title,
             presence: true,
diff --git a/config/site_settings.yml b/config/site_settings.yml
index 52f857e45ad7..e1d65ce5cea4 100644
--- a/config/site_settings.yml
+++ b/config/site_settings.yml
@@ -2135,10 +2135,13 @@ developer:
     type: group_list
     list_type: compact
     default: ""
     allow_any: false
     refresh: true
+  max_sidebar_section_links:
+    default: 50
+    hidden: true
 
 navigation:
   navigation_menu:
     client: true
     default: "sidebar"
diff --git a/spec/requests/sidebar_sections_controller_spec.rb b/spec/requests/sidebar_sections_controller_spec.rb
index 61124d3e7e45..a8a0bb3764a3 100644
--- a/spec/requests/sidebar_sections_controller_spec.rb
+++ b/spec/requests/sidebar_sections_controller_spec.rb
@@ -85,10 +85,28 @@ RSpec.describe SidebarSectionsController do
       expect(sidebar_section.sidebar_urls.fourth.name).to eq("My preferences")
       expect(sidebar_section.sidebar_urls.fourth.value).to eq("/my/preferences")
       expect(sidebar_section.sidebar_urls.fourth.external).to be false
     end
 
+    it "validates max number of links" do
+      SiteSetting.max_sidebar_section_links = 5
+
+      sign_in(user)
+
+      links =
+        6.times.map do
+          { icon: "external-link-alt", name: "My preferences", value: "/my/preferences" }
+        end
+
+      post "/sidebar_sections.json", params: { title: "custom section", links: links }
+
+      expect(response.status).to eq(422)
+      expect(response.parsed_body["errors"]).to eq(
+        ["Maximum 5 records are allowed. Got 6 records instead."],
+      )
+    end
+
     it "does not allow regular user to create public section" do
       sign_in(user)
 
       post "/sidebar_sections.json",
            params: {
@@ -234,10 +252,32 @@ RSpec.describe SidebarSectionsController do
       expect(user_history.details).to eq(
         "links: latest - /latest, meta - https://meta.discourse.org, homepage - https://discourse.org",
       )
     end
 
+    it "validates limit of links" do
+      SiteSetting.max_sidebar_section_links = 5
+
+      sign_in(user)
+
+      links =
+        6.times.map do
+          { icon: "external-link-alt", name: "My preferences", value: "/my/preferences" }
+        end
+
+      put "/sidebar_sections/#{sidebar_section.id}.json",
+          params: {
+            title: "custom section",
+            links: links,
+          }
+
+      expect(response.status).to eq(422)
+      expect(response.parsed_body["errors"]).to eq(
+        ["Maximum 5 records are allowed. Got 6 records instead."],
+      )
+    end
+
     it "doesn't allow to edit other's sections" do
       sidebar_section_2 = Fabricate(:sidebar_section)
       sidebar_url_3 = Fabricate(:sidebar_url, name: "other_tags", value: "/tags")
       Fabricate(:sidebar_section_link, sidebar_section: sidebar_section_2, linkable: sidebar_url_3)
       sign_in(user)

Vulnerable code:   accepts_nested_attributes_for :sidebar_urls, allow_destroy: true
Fixed code:   rescue ActiveRecord::NestedAttributes::TooManyRecords => e
    render_json_error(e.message)
  rescue ActiveRecord::NestedAttributes::TooManyRecords => e
    render_json_error(e.message)
  accepts_nested_attributes_for :sidebar_urls,
                                allow_destroy: true,
                                limit: -> { SiteSetting.max_sidebar_section_links }
  max_sidebar_section_links:
    default: 50
    hidden: true
    it "validates max number of links" do
      SiteSetting.max_sidebar_section_links = 5

      sign_in(user)

      links =
        6.times.map do
          { icon: "external-link-alt", name: "My preferences", value: "/my/preferences" }
        end

      post "/sidebar_sections.json", params: { title: "custom section", links: links }

      expect(response.status).to eq(422)
      expect(response.parsed_body["errors"]).to eq(
        ["Maximum 5 records are allowed. Got 6 records instead."],
      )
    end

    it "validates limit of links" do
      SiteSetting.max_sidebar_section_links = 5

      sign_in(user)

      links =
        6.times.map do
          { icon: "external-link-alt", name: "My preferences", value: "/my/preferences" }
        end

      put "/sidebar_sections/#{sidebar_section.id}.json",
          params: {
            title: "custom section",
            links: links,
          }

      expect(response.status).to eq(422)
      expect(response.parsed_body["errors"]).to eq(
        ["Maximum 5 records are allowed. Got 6 records instead."],
      )
    end