From e5aa099a4e1b33d9f9699673c6f2f3be4dbc0f24 Mon Sep 17 00:00:00 2001 From: Jonathan Hefner Date: Tue, 8 Sep 2020 08:46:09 -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 | 13 ++++++++++++- actionview/test/template/translation_helper_test.rb | 7 +++++++ 2 files changed, 19 insertions(+), 1 deletion(-) diff --git a/actionview/lib/action_view/helpers/translation_helper.rb b/actionview/lib/action_view/helpers/translation_helper.rb index 1860bc4732..27534abb7d 100644 --- a/actionview/lib/action_view/helpers/translation_helper.rb +++ b/actionview/lib/action_view/helpers/translation_helper.rb @@ -79,14 +79,22 @@ 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)) - translation.respond_to?(:html_safe) ? translation.html_safe : translation + if translation.equal?(MISSING_TRANSLATION) + options[:default].first + else + translation.respond_to?(:html_safe) ? translation.html_safe : translation + end else I18n.translate(scope_key_by_partial(key), options.merge(raise: i18n_raise)) end @@ -121,6 +129,9 @@ def localize(*args) alias :l :localize private + MISSING_TRANSLATION = Object.new + private_constant :MISSING_TRANSLATION + def scope_key_by_partial(key) if key.to_s.first == "." if @virtual_path diff --git a/actionview/test/template/translation_helper_test.rb b/actionview/test/template/translation_helper_test.rb index f40595bf4d..788afc339c 100644 --- a/actionview/test/template/translation_helper_test.rb +++ b/actionview/test/template/translation_helper_test.rb @@ -205,6 +205,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