diff --git a/Gemfile b/Gemfile index f446e63b3e1..e4963029f72 100644 --- a/Gemfile +++ b/Gemfile @@ -96,6 +96,7 @@ gem 'discourse_image_optim', require: 'image_optim' gem 'multi_json' gem 'mustache' gem 'nokogiri' +gem 'loofah' gem 'css_parser', require: false gem 'omniauth' diff --git a/Gemfile.lock b/Gemfile.lock index c542933c73f..075b4e0142d 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -525,6 +525,7 @@ DEPENDENCIES logstash-event logstash-logger logster + loofah lru_redux lz4-ruby mail! diff --git a/app/assets/javascripts/admin/addon/templates/components/site-settings/html.hbs b/app/assets/javascripts/admin/addon/templates/components/site-settings/html.hbs new file mode 100644 index 00000000000..0da61a84e1f --- /dev/null +++ b/app/assets/javascripts/admin/addon/templates/components/site-settings/html.hbs @@ -0,0 +1,2 @@ +{{text-field value=(html-safe value) classNames="input-setting-string"}} +
{{html-safe setting.description}}
diff --git a/app/controllers/admin/site_settings_controller.rb b/app/controllers/admin/site_settings_controller.rb index 6a1b7bda3d3..b909adf06a7 100644 --- a/app/controllers/admin/site_settings_controller.rb +++ b/app/controllers/admin/site_settings_controller.rb @@ -6,7 +6,10 @@ class Admin::SiteSettingsController < Admin::AdminController end def index - render_json_dump(site_settings: SiteSetting.all_settings, diags: SiteSetting.diags) + render_json_dump( + site_settings: SiteSetting.all_settings(sanitize_plain_text_settings: true), + diags: SiteSetting.diags + ) end def update diff --git a/app/services/site_settings_task.rb b/app/services/site_settings_task.rb index f9d185cdf2f..64c80486ffe 100644 --- a/app/services/site_settings_task.rb +++ b/app/services/site_settings_task.rb @@ -2,7 +2,7 @@ class SiteSettingsTask def self.export_to_hash(include_defaults: false, include_hidden: false) - site_settings = SiteSetting.all_settings(include_hidden) + site_settings = SiteSetting.all_settings(include_hidden: include_hidden) h = {} site_settings.each do |site_setting| next if site_setting[:default] == site_setting[:value] if !include_defaults diff --git a/app/services/user_merger.rb b/app/services/user_merger.rb index c0a6b6b826b..c322d5f3638 100644 --- a/app/services/user_merger.rb +++ b/app/services/user_merger.rb @@ -148,7 +148,7 @@ class UserMerger def update_site_settings ::MessageBus.publish '/merge_user', { message: I18n.t("admin.user.merge_user.updating_site_settings") }, user_ids: [@acting_user.id] if @acting_user - SiteSetting.all_settings(true).each do |setting| + SiteSetting.all_settings(include_hidden: true).each do |setting| if setting[:type] == "username" && setting[:value] == @source_user.username SiteSetting.set_and_log(setting[:setting], @target_user.username) end diff --git a/config/site_settings.yml b/config/site_settings.yml index 04d7de530d6..327b1bee977 100644 --- a/config/site_settings.yml +++ b/config/site_settings.yml @@ -19,6 +19,7 @@ # type: username - Must match the username of an existing user. # type: list - A list of values, chosen from a set of valid values defined in the choices option. # type: enum - A single value, chosen from a set of valid values in the choices option. +# type: html - A single value. It could contain HTML. # # A type:list setting with the word 'colors' in its name will make color values have a bold line of the corresponding color # diff --git a/lib/site_setting_extension.rb b/lib/site_setting_extension.rb index 6b02292c6d3..3196e2a97b5 100644 --- a/lib/site_setting_extension.rb +++ b/lib/site_setting_extension.rb @@ -208,14 +208,20 @@ module SiteSettingExtension def client_settings_json_uncached MultiJson.dump(Hash[*@client_settings.map do |name| value = self.public_send(name) - value = value.to_s if type_supervisor.get_type(name) == :upload - value = value.map(&:to_s).join("|") if type_supervisor.get_type(name) == :uploaded_image_list + type = type_supervisor.get_type(name) + value = value.to_s if type == :upload + value = value.map(&:to_s).join("|") if type == :uploaded_image_list + + if should_sanitize?(value, type) + value = sanitize(value) + end + [name, value] end.flatten]) end # Retrieve all settings - def all_settings(include_hidden = false) + def all_settings(include_hidden: false, sanitize_plain_text_settings: false) locale_setting_hash = { @@ -244,6 +250,8 @@ module SiteSettingExtension default.to_i < Upload::SEEDED_ID_THRESHOLD default = default_uploads[default.to_i] + elsif sanitize_plain_text_settings && should_sanitize?(value, type_hash[:type].to_s) + value = sanitize(value) end opts = { @@ -574,6 +582,14 @@ module SiteSettingExtension end end + def should_sanitize?(value, type) + value.is_a?(String) && type.to_s != 'html' + end + + def sanitize(value) + CGI.unescapeHTML(Loofah.scrub_fragment(value, :strip).to_s) + end + def logger Rails.logger end diff --git a/lib/site_settings/type_supervisor.rb b/lib/site_settings/type_supervisor.rb index ee78697ebf2..b33030c713b 100644 --- a/lib/site_settings/type_supervisor.rb +++ b/lib/site_settings/type_supervisor.rb @@ -36,7 +36,8 @@ class SiteSettings::TypeSupervisor tag_list: 21, color: 22, simple_list: 23, - emoji_list: 24 + emoji_list: 24, + html: 25 ) end diff --git a/spec/components/site_setting_extension_spec.rb b/spec/components/site_setting_extension_spec.rb index 2db0f2c494c..0180122715e 100644 --- a/spec/components/site_setting_extension_spec.rb +++ b/spec/components/site_setting_extension_spec.rb @@ -631,7 +631,7 @@ describe SiteSettingExtension do end it "is present in all_settings when we ask for hidden" do - expect(settings.all_settings(true).find { |s| s[:setting] == :superman_identity }).to be_present + expect(settings.all_settings(include_hidden: true).find { |s| s[:setting] == :superman_identity }).to be_present end end @@ -842,6 +842,23 @@ describe SiteSettingExtension do expect(setting[:default]).to eq(system_upload.url) end end + + it 'should sanitize html in the site settings' do + settings.setting(:with_html, 'rest') + settings.refresh! + + setting = settings.all_settings(sanitize_plain_text_settings: true).last + + expect(setting[:value]).to eq('rest') + end + + it 'settings with html type are not sanitized' do + settings.setting(:with_html, 'rest', type: :html) + + setting = settings.all_settings(sanitize_plain_text_settings: true).last + + expect(setting[:value]).to eq('rest') + end end describe '.client_settings_json_uncached' do @@ -855,6 +872,27 @@ describe SiteSettingExtension do %Q|{"default_locale":"#{SiteSetting.default_locale}","upload_type":"#{upload.url}","string_type":"haha"}| ) end + + it 'should sanitize html in the site settings' do + settings.setting(:with_html, 'rest', client: true) + settings.setting(:with_symbols, '<>rest', client: true) + settings.setting(:with_unknown_tag, 'rest', client: true) + settings.refresh! + + client_settings = JSON.parse settings.client_settings_json_uncached + + expect(client_settings['with_html']).to eq('rest') + expect(client_settings['with_symbols']).to eq('<>rest') + expect(client_settings['with_unknown_tag']).to eq('rest') + end + + it 'settings with html type are not sanitized' do + settings.setting(:with_html, 'rest', type: :html, client: true) + + client_settings = JSON.parse settings.client_settings_json_uncached + + expect(client_settings['with_html']).to eq('rest') + end end describe '.setup_methods' do diff --git a/spec/components/site_settings/type_supervisor_spec.rb b/spec/components/site_settings/type_supervisor_spec.rb index 36c6f82a197..f565b86eb05 100644 --- a/spec/components/site_settings/type_supervisor_spec.rb +++ b/spec/components/site_settings/type_supervisor_spec.rb @@ -94,6 +94,9 @@ describe SiteSettings::TypeSupervisor do it "'emoji_list' should be at the right position" do expect(SiteSettings::TypeSupervisor.types[:emoji_list]).to eq(24) end + it "'html' should be at the right position" do + expect(SiteSettings::TypeSupervisor.types[:html]).to eq(25) + end end end