From 0a3487c7a06a60569817266ffdd39ef0409839d4 Mon Sep 17 00:00:00 2001
From: Andrew Carpenter <andrew@criticaljuncture.org>
Date: Thu, 28 Jul 2016 16:12:21 -0700
Subject: [PATCH] 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
---
 actionview/lib/action_view/helpers/tag_helper.rb |  2 +-
 actionview/test/template/tag_helper_test.rb      | 12 +++++++++++-
 2 files changed, 12 insertions(+), 2 deletions(-)

diff --git a/actionview/lib/action_view/helpers/tag_helper.rb b/actionview/lib/action_view/helpers/tag_helper.rb
index 42e7358..ac26c29 100644
--- a/actionview/lib/action_view/helpers/tag_helper.rb
+++ b/actionview/lib/action_view/helpers/tag_helper.rb
@@ -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(/"/, '&quot;'.freeze)}")
         end
     end
   end
diff --git a/actionview/test/template/tag_helper_test.rb b/actionview/test/template/tag_helper_test.rb
index f3956a3..fe5ec03 100644
--- a/actionview/test/template/tag_helper_test.rb
+++ b/actionview/test/template/tag_helper_test.rb
@@ -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)
@@ -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
-- 
2.8.1