Hi,
There is a bug in the patch for the 3.2 series. I've attached a
combined patch here. The attached patch is a combination of these two
patches:
https://github.com/rails/rails/commit/4bcccf5ecd81a6272479537911b7d9760c5be164
https://github.com/rails/rails/commit/5aabcf25caefbe84f656256a9d3e7fc0c9e14ecc
Sorry for the problems.
> From cbdb7d367c4f15ecb85c308a0d78f61d629a74c1 Mon Sep 17 00:00:00 2001
> From: Andrew Carpenter <
and...@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
> ---
> 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(/"/, '"'.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=""">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=""">content</p>',
> + content_tag('p', "content", data: { title: '"'.html_safe })
> + end
> +
> def test_skip_invalid_escaped_attributes
> ['&1;', 'dfa3;', '& #123;'].each do |escaped|
> assert_equal %(<a href="#{escaped.gsub(/&/, '&')}" />), tag('a', :href => escaped)
> --
> 2.8.1
>
> From e4abbc8636e1300d14b1fd7e3f05e4e25bc8289e Mon Sep 17 00:00:00 2001
> From: Andrew Carpenter <
and...@criticaljuncture.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(/"/, '"'.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>" />', str
> end
>
> + def test_tag_does_not_honor_html_safe_double_quotes_as_attributes
> + assert_dom_equal '<p title=""">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=""">content</p>',
> + content_tag('p', "content", data: { title: '"'.html_safe })
> + end
> +
> def test_skip_invalid_escaped_attributes
> ['&1;', 'dfa3;', '& #123;'].each do |escaped|
> assert_equal %(<a href="#{escaped.gsub(/&/, '&')}" />), tag('a', :href => escaped)
> --
> 2.8.1
>
> From 0a3487c7a06a60569817266ffdd39ef0409839d4 Mon Sep 17 00:00:00 2001
> From: Andrew Carpenter <
and...@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(/"/, '"'.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>" />', str
> end
>
> + def test_tag_does_not_honor_html_safe_double_quotes_as_attributes
> + assert_dom_equal '<p title=""">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=""">content</p>',
> + content_tag('p', "content", data: { title: '"'.html_safe })
> + end
> +
> def test_skip_invalid_escaped_attributes
> ['&1;', 'dfa3;', '& #123;'].each do |escaped|
> assert_equal %(<a href="#{escaped.gsub(/&/, '&')}" />), 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