From bbc7746cef61ccdca065069d5c0d93595d8be307 Mon Sep 17 00:00:00 2001 From: Ted Johansson Date: Tue, 18 Apr 2023 14:32:21 +0800 Subject: [PATCH] SECURITY: Ensure site setting being updated is a configurable site setting (#21132) --- app/controllers/admin/site_settings_controller.rb | 2 +- config/locales/server.en.yml | 1 + lib/site_setting_extension.rb | 8 ++++++-- .../requests/admin/site_settings_controller_spec.rb | 13 +++++++++++++ 4 files changed, 21 insertions(+), 3 deletions(-) diff --git a/app/controllers/admin/site_settings_controller.rb b/app/controllers/admin/site_settings_controller.rb index ad28e93af44..e14c1f45843 100644 --- a/app/controllers/admin/site_settings_controller.rb +++ b/app/controllers/admin/site_settings_controller.rb @@ -38,7 +38,7 @@ class Admin::SiteSettingsController < Admin::AdminController value = Upload.get_from_url(value) || "" if SiteSetting.type_supervisor.get_type(id) == :upload update_existing_users = params[:update_existing_user].present? - previous_value = value_or_default(SiteSetting.public_send(id)) if update_existing_users + previous_value = value_or_default(SiteSetting.get(id)) if update_existing_users SiteSetting.set_and_log(id, value, current_user) diff --git a/config/locales/server.en.yml b/config/locales/server.en.yml index 1661e9544ed..9eb6a6498aa 100644 --- a/config/locales/server.en.yml +++ b/config/locales/server.en.yml @@ -198,6 +198,7 @@ en: embed: load_from_remote: "There was an error loading that post." site_settings: + invalid_site_setting: "No setting named '%{name}' exists" invalid_category_id: "You specified a category that does not exist" invalid_choice: one: "You specified the invalid choice %{name}" diff --git a/lib/site_setting_extension.rb b/lib/site_setting_extension.rb index d89408a63db..c42230f30e1 100644 --- a/lib/site_setting_extension.rb +++ b/lib/site_setting_extension.rb @@ -438,7 +438,9 @@ module SiteSettingExtension value = prev_value = "[FILTERED]" if secret_settings.include?(name.to_sym) StaffActionLogger.new(user).log_site_setting_change(name, prev_value, value) else - raise Discourse::InvalidParameters.new("No setting named '#{name}' exists") + raise Discourse::InvalidParameters.new( + I18n.t("errors.site_settings.invalid_site_setting", name: name), + ) end end @@ -446,7 +448,9 @@ module SiteSettingExtension if has_setting?(name) self.public_send(name) else - raise Discourse::InvalidParameters.new("No setting named '#{name}' exists") + raise Discourse::InvalidParameters.new( + I18n.t("errors.site_settings.invalid_site_setting", name: name), + ) end end diff --git a/spec/requests/admin/site_settings_controller_spec.rb b/spec/requests/admin/site_settings_controller_spec.rb index 693a6612825..1aabf0deeb0 100644 --- a/spec/requests/admin/site_settings_controller_spec.rb +++ b/spec/requests/admin/site_settings_controller_spec.rb @@ -253,6 +253,19 @@ RSpec.describe Admin::SiteSettingsController do expect(SiteSetting.search_tokenize_chinese).to eq(true) end + it "throws an error when the parameter is not a configurable site setting" do + put "/admin/site_settings/clear_cache!.json", + params: { + clear_cache!: "", + update_existing_user: true, + } + + expect(response.status).to eq(422) + expect(response.parsed_body["errors"]).to contain_exactly( + "No setting named 'clear_cache!' exists", + ) + end + it "throws an error when trying to change a deprecated setting with override = false" do SiteSetting.personal_message_enabled_groups = Group::AUTO_GROUPS[:trust_level_4] put "/admin/site_settings/enable_personal_messages.json",