Openwall GNU/*/Linux - a small security-enhanced Linux distro for servers
[<prev] [next>] [thread-next>] [day] [month] [year] [list]
Date: Thu, 11 Aug 2016 10:52:10 -0700
From: Aaron Patterson <tenderlove@...y-lang.org>
To: security@...e.de, rubyonrails-security@...glegroups.com,
	oss-security@...ts.openwall.com, ruby-security-ann@...glegroups.com
Subject: [CVE-2016-6316] Possible XSS Vulnerability in Action View

# Possible XSS Vulnerability in Action View

There is a possible XSS vulnerability in Action View.  Text declared as "HTML
safe" will not have quotes escaped when used as attribute values in tag
helpers.  This vulnerability has been assigned the CVE identifier
CVE-2016-6316.

Versions Affected:  >= 3.0.0.
Not affected:       < 3.0.0
Fixed Versions:     5.0.0.1, 4.2.7.1, 3.2.22.3

Impact
------
Text declared as "HTML safe" when passed as an attribute value to a tag helper
will not have quotes escaped which can lead to an XSS attack.  Impacted code
looks something like this:

```
content_tag(:div, "hi", title: user_input.html_safe)
```

Some helpers like the `sanitize` helper will automatically mark strings as
"HTML safe", so impacted code could also look something like this:

```
content_tag(:div, "hi", title: sanitize(user_input))
```

All users running an affected release should either upgrade or use one of the
workarounds immediately.

Releases
--------
The FIXED releases are available at the normal locations.

Workarounds
-----------
You can work around this issue by either *not* marking arbitrary user input as
safe, or by manually escaping quotes like this:

```
def escape_quotes(value)
  value.gsub(/"/, '&quot;'.freeze)
end

content_tag(:div, "hi", title: escape_quotes(sanitize(user_input)))
```

Patches
-------
To aid users who aren't able to upgrade immediately we have provided patches for
the two supported release series. They are in git-am format and consist of a
single changeset.

* 3-2-attribute-xss.patch - Patch for 3.2 series
* 4-2-attribute-xss.patch - Patch for 4.2 series
* 5-0-attribute-xss.patch - Patch for 5.0 series

Please note that only the 5.0.x and 4.2.x series are supported at present. Users
of earlier unsupported releases are advised to upgrade as soon as possible as we
cannot guarantee the continued availability of security fixes for unsupported
releases.

Credits
-------

Thanks to Andrew Carpenter of Critical Juncture for reporting this issue and
sending a patch to fix it!

-- 
Aaron Patterson
http://tenderlovemaking.com/

From cbdb7d367c4f15ecb85c308a0d78f61d629a74c1 Mon Sep 17 00:00:00 2001
From: Andrew Carpenter <andrew@...ticaljuncture.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
---
 actionpack/lib/action_view/helpers/tag_helper.rb | 15 +++++++++++----
 actionpack/test/template/tag_helper_test.rb      | 10 ++++++++++
 2 files changed, 21 insertions(+), 4 deletions(-)

diff --git a/actionpack/lib/action_view/helpers/tag_helper.rb b/actionpack/lib/action_view/helpers/tag_helper.rb
index 7f58a27..34741b8 100644
--- a/actionpack/lib/action_view/helpers/tag_helper.rb
+++ b/actionpack/lib/action_view/helpers/tag_helper.rb
@@ -141,20 +141,27 @@ module ActionView
                   unless v.is_a?(String) || v.is_a?(Symbol) || v.is_a?(BigDecimal)
                     v = v.to_json
                   end
-                  v = ERB::Util.html_escape(v) if escape
-                  attrs << %(data-#{k.to_s.dasherize}="#{v}")
+                  attrs << tag_option("data-#{k.to_s.dasherize}", v, escape)
                 end
               elsif BOOLEAN_ATTRIBUTES.include?(key)
                 attrs << %(#{key}="#{key}") if value
               elsif !value.nil?
                 final_value = value.is_a?(Array) ? value.join(" ") : value
-                final_value = ERB::Util.html_escape(final_value) if escape
-                attrs << %(#{key}="#{final_value}")
+                attrs << tag_option(key, value, escape)
               end
             end
             " #{attrs.sort * ' '}".html_safe unless attrs.empty?
           end
         end
+
+        def tag_option(key, value, escape)
+          if value.is_a?(Array)
+            value = escape ? safe_join(value, " ") : value.join(" ")
+          else
+            value = escape ? ERB::Util.html_escape(value) : value
+          end
+          %(#{key}="#{value.gsub(/"/, '&quot;'.freeze)}")
+        end
     end
   end
 end
diff --git a/actionpack/test/template/tag_helper_test.rb b/actionpack/test/template/tag_helper_test.rb
index e362955..9c3d636 100644
--- a/actionpack/test/template/tag_helper_test.rb
+++ b/actionpack/test/template/tag_helper_test.rb
@@ -101,6 +101,16 @@ class TagHelperTest < ActionView::TestCase
     end
   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)
-- 
2.8.1


From e4abbc8636e1300d14b1fd7e3f05e4e25bc8289e Mon Sep 17 00:00:00 2001
From: Andrew Carpenter <andrew@...ticaljuncture.org>
Date: Thu, 28 Jul 2016 16:12:21 -0700
Subject: [PATCH 1/2] 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      | 10 ++++++++++
 2 files changed, 11 insertions(+), 1 deletion(-)

diff --git a/actionview/lib/action_view/helpers/tag_helper.rb b/actionview/lib/action_view/helpers/tag_helper.rb
index b203857..f09595d 100644
--- a/actionview/lib/action_view/helpers/tag_helper.rb
+++ b/actionview/lib/action_view/helpers/tag_helper.rb
@@ -181,7 +181,7 @@ module ActionView
           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 ce89d57..8332dd0 100644
--- a/actionview/test/template/tag_helper_test.rb
+++ b/actionview/test/template/tag_helper_test.rb
@@ -140,6 +140,16 @@ class TagHelperTest < ActionView::TestCase
     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)
-- 
2.8.1


From 0a3487c7a06a60569817266ffdd39ef0409839d4 Mon Sep 17 00:00:00 2001
From: Andrew Carpenter <andrew@...ticaljuncture.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



[ CONTENT OF TYPE application/pgp-signature SKIPPED ]

Powered by blists - more mailing lists

Your e-mail address:

Please check out the Open Source Software Security Wiki, which is counterpart to this mailing list.

Powered by Openwall GNU/*/Linux - Powered by OpenVZ