Skip to content

Commit 4394e90

Browse files
andrewcarpentertenderlove
authored andcommitted
ensure tag/content_tag escapes " in attribute vals
Many helpers mark content as HTML-safe without escaping double quotes -- including `sanitize`. Regardless of whether or not the attribute values are HTML-escaped, we want to be sure they don't include double quotes, as that can cause XSS issues. For example: `content_tag(:div, "foo", title: sanitize('" onmouseover="alert(1);//'))` CVE-2016-6316
1 parent b9f71e4 commit 4394e90

File tree

2 files changed

+11
-1
lines changed

2 files changed

+11
-1
lines changed

actionview/lib/action_view/helpers/tag_helper.rb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -90,7 +90,7 @@ def tag_option(key, value, escape)
9090
else
9191
value = escape ? ERB::Util.unwrapped_html_escape(value) : value
9292
end
93-
%(#{key}="#{value}")
93+
%(#{key}="#{value.gsub(/"/, '"'.freeze)}")
9494
end
9595

9696
private

actionview/test/template/tag_helper_test.rb

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -274,6 +274,16 @@ def test_tag_builder_honors_html_safe_with_escaped_array_class
274274
assert_equal '<p class="song> play&gt;"></p>', tag.p(class: [raw("song>"), "play>"])
275275
end
276276

277+
def test_tag_does_not_honor_html_safe_double_quotes_as_attributes
278+
assert_dom_equal '<p title="&quot;">content</p>',
279+
content_tag('p', "content", title: '"'.html_safe)
280+
end
281+
282+
def test_data_tag_does_not_honor_html_safe_double_quotes_as_attributes
283+
assert_dom_equal '<p data-title="&quot;">content</p>',
284+
content_tag('p', "content", data: { title: '"'.html_safe })
285+
end
286+
277287
def test_skip_invalid_escaped_attributes
278288
["&1;", "&#1dfa3;", "& #123;"].each do |escaped|
279289
assert_equal %(<a href="#{escaped.gsub(/&/, '&amp;')}" />), tag("a", href: escaped)

0 commit comments

Comments
 (0)