From b6dd03dc29542cfcbca226f802766927cf08da3a Mon Sep 17 00:00:00 2001 From: Jonathan Hefner Date: Tue, 8 Sep 2020 08:26:45 -0400 Subject: [PATCH] Fix XSS vulnerability in `translate` helper Prior to this commit, when a translation key indicated that the translation text was HTML, the value returned by `I18n.translate` would always be marked as `html_safe`. However, the value returned by `I18n.translate` could be an untrusted value directly from `options[:default]`. This commit ensures values directly from `options[:default]` are not marked as `html_safe`. --- .../lib/action_view/helpers/translation_helper.rb | 12 +++++++++++- actionview/test/template/translation_helper_test.rb | 7 +++++++ 2 files changed, 18 insertions(+), 1 deletion(-) diff --git a/actionview/lib/action_view/helpers/translation_helper.rb b/actionview/lib/action_view/helpers/translation_helper.rb index f75e713b47..960c3f65c4 100644 --- a/actionview/lib/action_view/helpers/translation_helper.rb +++ b/actionview/lib/action_view/helpers/translation_helper.rb @@ -76,13 +76,20 @@ def translate(key, **options) if html_safe_translation_key?(key) html_safe_options = options.dup + options.except(*I18n::RESERVED_KEYS).each do |name, value| unless name == :count && value.is_a?(Numeric) html_safe_options[name] = ERB::Util.html_escape(value.to_s) end end + + html_safe_options[:default] = MISSING_TRANSLATION unless html_safe_options[:default].blank? + translation = I18n.translate(scope_key_by_partial(key), **html_safe_options.merge(raise: i18n_raise)) - if translation.respond_to?(:map) + + if translation.equal?(MISSING_TRANSLATION) + options[:default].first + elsif translation.respond_to?(:map) translation.map { |element| element.respond_to?(:html_safe) ? element.html_safe : element } else translation.respond_to?(:html_safe) ? translation.html_safe : translation @@ -121,6 +128,9 @@ def localize(object, **options) alias :l :localize private + MISSING_TRANSLATION = Object.new + private_constant :MISSING_TRANSLATION + def scope_key_by_partial(key) stringified_key = key.to_s if stringified_key.first == "." diff --git a/actionview/test/template/translation_helper_test.rb b/actionview/test/template/translation_helper_test.rb index 4e6c6df285..47e1e1871b 100644 --- a/actionview/test/template/translation_helper_test.rb +++ b/actionview/test/template/translation_helper_test.rb @@ -217,6 +217,13 @@ def test_translate_with_last_default_not_named_html assert_equal false, translation.html_safe? end + def test_translate_does_not_mark_unsourced_string_default_as_html_safe + untrusted_string = "" + translation = translate(:"translations.missing", default: [:"translations.missing_html", untrusted_string]) + assert_equal untrusted_string, translation + assert_not_predicate translation, :html_safe? + end + def test_translate_with_string_default translation = translate(:'translations.missing', default: "A Generic String") assert_equal "A Generic String", translation -- 2.27.0