Skip to content

Commit

Permalink
ensure tag/content_tag escapes " in attribute vals
Browse files Browse the repository at this point in the history
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
  • Loading branch information
andrewcarpenter authored and tenderlove committed Aug 10, 2016
1 parent 2efddad commit 8f544bc
Show file tree
Hide file tree
Showing 2 changed files with 12 additions and 2 deletions.
2 changes: 1 addition & 1 deletion actionview/lib/action_view/helpers/tag_helper.rb
Expand Up @@ -189,7 +189,7 @@ def tag_option(key, value, escape)
else
value = escape ? ERB::Util.unwrapped_html_escape(value) : value
end
%(#{key}="#{value}")
%(#{key}="#{value.gsub(/"/, '"'.freeze)}")
end
end
end
Expand Down
12 changes: 11 additions & 1 deletion actionview/test/template/tag_helper_test.rb
Expand Up @@ -150,6 +150,16 @@ def test_tag_honors_html_safe_with_escaped_array_class
assert_equal '<p class="song> play&gt;" />', str
end

def test_tag_does_not_honor_html_safe_double_quotes_as_attributes
assert_dom_equal '<p title="&quot;">content</p>',
content_tag('p', "content", title: '"'.html_safe)
end

def test_data_tag_does_not_honor_html_safe_double_quotes_as_attributes
assert_dom_equal '<p data-title="&quot;">content</p>',
content_tag('p', "content", data: { title: '"'.html_safe })
end

def test_skip_invalid_escaped_attributes
['&1;', '&#1dfa3;', '& #123;'].each do |escaped|
assert_equal %(<a href="#{escaped.gsub(/&/, '&amp;')}" />), tag('a', :href => escaped)
Expand Down Expand Up @@ -177,6 +187,6 @@ def test_aria_attributes
def test_link_to_data_nil_equal
div_type1 = content_tag(:div, 'test', { 'data-tooltip' => nil })
div_type2 = content_tag(:div, 'test', { data: {tooltip: nil} })
assert_dom_equal div_type1, div_type2
assert_dom_equal div_type1, div_type2
end
end

0 comments on commit 8f544bc

Please sign in to comment.