1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
|
From aaa7ab1320330b3c4fa8f0fbda716dcfa21e3d65 Mon Sep 17 00:00:00 2001
From: Jonathan Hefner <jonathan@hefner.pro>
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 1860bc47325d..27534abb7deb 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 f40595bf4dd4..788afc339cf7 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 = "<script>alert()</script>"
+ 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
|
