diff --git a/config/locales/server.en.yml b/config/locales/server.en.yml index 24d1489856c..aff2d7474ed 100644 --- a/config/locales/server.en.yml +++ b/config/locales/server.en.yml @@ -2709,6 +2709,7 @@ en: invalid_reply_by_email_address: "Value must contain '%{reply_key}' and be different from the notification email." invalid_alternative_reply_by_email_addresses: "All values must contain '%{reply_key}' and be different from the notification email." invalid_domain_hostname: "Must not include * or ? characters." + invalid_allowed_iframes_url: "Iframe urls must start with http:// or https:// and have at least one more additional '/'" invalid_csp_script_src: "Value must be either 'unsafe-eval' or 'wasm-unsafe-eval', or in the form '-' where supported hash algorithms are sha256, sha384 or sha512. Ensure that your input is wrapped in single quotation marks." pop3_polling_host_is_empty: "You must set a 'pop3 polling host' before enabling POP3 polling." pop3_polling_username_is_empty: "You must set a 'pop3 polling username' before enabling POP3 polling." diff --git a/config/site_settings.yml b/config/site_settings.yml index 87f0b132936..216ed3c0796 100644 --- a/config/site_settings.yml +++ b/config/site_settings.yml @@ -2020,6 +2020,7 @@ security: type: list list_type: simple client: true + validator: "AllowedIframesValidator" allowed_crawler_user_agents: type: list default: "" diff --git a/db/migrate/20240731190511_update_invalid_allowed_iframe_values.rb b/db/migrate/20240731190511_update_invalid_allowed_iframe_values.rb new file mode 100644 index 00000000000..8947b45e391 --- /dev/null +++ b/db/migrate/20240731190511_update_invalid_allowed_iframe_values.rb @@ -0,0 +1,42 @@ +# frozen_string_literal: true +class UpdateInvalidAllowedIframeValues < ActiveRecord::Migration[7.1] + def up + prev_value = + DB.query_single("SELECT value FROM site_settings WHERE name = 'allowed_iframes'").first + + return if prev_value.blank? + + # Url starts with http:// or https:// and has at least one more additional '/' + regex = %r{\Ahttps?://([^/]*/)+[^/]*\z}x + + new_value = + prev_value + .split("|") + .map do |substring| + if substring.match?(regex) + substring + else + "#{substring}/" + end + end + .uniq + .join("|") + + return if new_value == prev_value + + DB.exec(<<~SQL, new_value:) + UPDATE site_settings + SET value = :new_value + WHERE name = 'allowed_iframes' + SQL + + DB.exec(<<~SQL, prev_value:, new_value:) + INSERT INTO user_histories (action, subject, previous_value, new_value, admin_only, updated_at, created_at, acting_user_id) + VALUES (3, 'allowed_iframes', :prev_value, :new_value, true, NOW(), NOW(), -1) + SQL + end + + def down + raise ActiveRecord::IrreversibleMigration + end +end diff --git a/lib/validators/allowed_iframes_validator.rb b/lib/validators/allowed_iframes_validator.rb new file mode 100644 index 00000000000..63e155364fc --- /dev/null +++ b/lib/validators/allowed_iframes_validator.rb @@ -0,0 +1,18 @@ +# frozen_string_literal: true + +class AllowedIframesValidator + # Url starts with http:// or https:// and has at least one more additional '/' + VALID_ALLOWED_IFRAME_URL_REGEX = %r{\Ahttps?://([^/]*/)+[^/]*\z}x + + def initialize(opts = {}) + @opts = opts + end + + def valid_value?(values) + values.split("|").all? { _1.match? VALID_ALLOWED_IFRAME_URL_REGEX } + end + + def error_message + I18n.t("site_settings.errors.invalid_allowed_iframes_url") + end +end diff --git a/spec/lib/oneboxer_spec.rb b/spec/lib/oneboxer_spec.rb index cbf4ba55e4e..a2cde501cda 100644 --- a/spec/lib/oneboxer_spec.rb +++ b/spec/lib/oneboxer_spec.rb @@ -713,7 +713,7 @@ RSpec.describe Oneboxer do body: allowlisted_oembed.to_json, ) - SiteSetting.allowed_iframes = "discourse.org|https://ifram.es" + SiteSetting.allowed_iframes = "https://discourse.org/|https://ifram.es/" expect(Oneboxer.onebox("https://blocklist.ed/iframes", invalidate_oneboxes: true)).to be_empty expect(Oneboxer.onebox("https://allowlist.ed/iframes", invalidate_oneboxes: true)).to match(