[prev in list] [next in list] [prev in thread] [next in thread]
List: oss-security
Subject: [oss-security] [CVE-2015-7579] XSS vulnerability in rails-html-sanitizer
From: Aaron Patterson <tenderlove () ruby-lang ! org>
Date: 2016-01-25 19:35:37
Message-ID: 20160125193537.GE14069 () TC ! local
[Download RAW message or body]
[Attachment #2 (multipart/mixed)]
XSS vulnerability in rails-html-sanitizer
There is a XSS vulnerability in `Rails::Html::FullSanitizer` used by Action View's `strip_tags`.
This vulnerability has been assigned the CVE identifier CVE-2015-7579.
Versions Affected: 1.0.2
Not affected: 1.0.0, 1.0.1
Fixed Versions: 1.0.3
Impact
------
Due to the way that `Rails::Html::FullSanitizer` is implemented, if an attacker
passes an already escaped HTML entity to the input of Action View's `strip_tags`
these entities will be unescaped what may cause a XSS attack if used in combination
with `raw` or `html_safe`.
For example:
strip_tags("<script>alert('XSS')</script>")
Would generate:
<script>alert('XSS')</script>
After the fix it will generate:
<script>alert('XSS')</script>
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
-----------
If you can't upgrade, please use the following monkey patch in an initializer
that is loaded before your application:
```
$ cat config/initializers/strip_tags_fix.rb
class ActionView::Base
def strip_tags(html)
self.class.full_sanitizer.sanitize(html)
end
end
```
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.
* Do-not-unescape-already-escaped-HTML-entities.patch
Credits
-------
Thank you to Arthur Neves from GitHub and Spyros Livathinos from Zendesk for
reporting the problem and working with us to fix it.
--
Aaron Patterson
http://tenderlovemaking.com/
["Do-not-unescape-already-escaped-HTML-entities.patch" (text/plain)]
From b1bc872f92f079c9d0f6b79d32d6b0bc6cfcc3dc Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Rafael=20Mendon=C3=A7a=20Fran=C3=A7a?=
<rafaelmfranca@gmail.com>
Date: Mon, 10 Aug 2015 16:22:15 -0300
Subject: [PATCH] Do not unescape already escaped HTML entities
The full sanitizer was using Loofah's #text method that automatically
escapes HTML entities. That behavior caused some problems where strings
that were not escaped in the older sanitizer started to be escaped. To
fix these problems we used the #text's `encode_special_chars` option as
`false` that not just skipped the HTML entities escaping but unescaped
already escaped entities.
This introduced a security bug because an attacker can pass escaped HTML
tags that will not be sanitized and will be returned as unescaped HTML
tags.
To fix it properly we introduced a new scrubber that will remove all
tags and keep just the text nodes of these tags without changing how
to escape the string.
CVE-2015-7579
---
lib/rails/html/sanitizer.rb | 17 ++++++++++-------
lib/rails/html/scrubbers.rb | 20 ++++++++++++++++++++
test/sanitizer_test.rb | 7 +++++--
test/scrubbers_test.rb | 16 +++++++++++++++-
4 files changed, 50 insertions(+), 10 deletions(-)
diff --git a/lib/rails/html/sanitizer.rb b/lib/rails/html/sanitizer.rb
index 8be10d5..8b9c7c8 100644
--- a/lib/rails/html/sanitizer.rb
+++ b/lib/rails/html/sanitizer.rb
@@ -13,6 +13,10 @@ module Rails
node.xpath(*xpaths).remove
node
end
+
+ def properly_encode(fragment, options)
+ fragment.xml? ? fragment.to_xml(options) : fragment.to_html(options)
+ end
end
# === Rails::Html::FullSanitizer
@@ -26,9 +30,12 @@ module Rails
return unless html
return html if html.empty?
- Loofah.fragment(html).tap do |fragment|
- remove_xpaths(fragment, XPATHS_TO_REMOVE)
- end.text(options)
+ loofah_fragment = Loofah.fragment(html)
+
+ remove_xpaths(loofah_fragment, XPATHS_TO_REMOVE)
+ loofah_fragment.scrub!(TextOnlyScrubber.new)
+
+ properly_encode(loofah_fragment, encoding: 'UTF-8')
end
end
@@ -136,10 +143,6 @@ module Rails
def allowed_attributes(options)
options[:attributes] || self.class.allowed_attributes
end
-
- def properly_encode(fragment, options)
- fragment.xml? ? fragment.to_xml(options) : fragment.to_html(options)
- end
end
end
end
diff --git a/lib/rails/html/scrubbers.rb b/lib/rails/html/scrubbers.rb
index 1384a2f..de497f9 100644
--- a/lib/rails/html/scrubbers.rb
+++ b/lib/rails/html/scrubbers.rb
@@ -144,5 +144,25 @@ module Rails
!super
end
end
+
+ # === Rails::Html::TextOnlyScrubber
+ #
+ # Rails::Html::TextOnlyScrubber allows you to permit text nodes.
+ #
+ # Unallowed elements will be stripped, i.e. element is removed but its subtree kept.
+ class TextOnlyScrubber < Loofah::Scrubber
+ def initialize
+ @direction = :bottom_up
+ end
+
+ def scrub(node)
+ if node.text?
+ CONTINUE
+ else
+ node.before node.children
+ node.remove
+ end
+ end
+ end
end
end
diff --git a/test/sanitizer_test.rb b/test/sanitizer_test.rb
index 06d70e4..d6ab7cd 100644
--- a/test/sanitizer_test.rb
+++ b/test/sanitizer_test.rb
@@ -104,9 +104,12 @@ class SanitizersTest < Minitest::Test
assert_equal "Frozen string with no tags", full_sanitize("Frozen string with no tags".freeze)
end
- def test_full_sanitize_allows_turning_off_encoding_special_chars
+ def test_full_sanitize_respect_html_escaping_of_the_given_string
+ assert_equal 'test\r\nstring', full_sanitize('test\r\nstring')
assert_equal '&', full_sanitize('&')
- assert_equal '&', full_sanitize('&', encode_special_chars: false)
+ assert_equal '&', full_sanitize('&')
+ assert_equal '&amp;', full_sanitize('&amp;')
+ assert_equal 'omg <script>BOM</script>', full_sanitize('omg <script>BOM</script>')
end
def test_strip_links_with_tags_in_tags
diff --git a/test/scrubbers_test.rb b/test/scrubbers_test.rb
index 2a2838a..4b84263 100644
--- a/test/scrubbers_test.rb
+++ b/test/scrubbers_test.rb
@@ -143,6 +143,20 @@ class TargetScrubberTest < ScrubberTest
end
end
+class TextOnlyScrubberTest < ScrubberTest
+ def setup
+ @scrubber = Rails::Html::TextOnlyScrubber.new
+ end
+
+ def test_removes_all_tags_and_keep_the_content
+ assert_scrubbed '<tag>hello</tag>', 'hello'
+ end
+
+ def test_skips_text_nodes
+ assert_node_skipped('some text')
+ end
+end
+
class ReturningStopFromScrubNodeTest < ScrubberTest
class ScrubStopper < Rails::Html::PermitScrubber
def scrub_node(node)
@@ -157,4 +171,4 @@ class ReturningStopFromScrubNodeTest < ScrubberTest
def test_returns_stop_from_scrub_if_scrub_node_does
assert_scrub_stopped '<script>remove me</script>'
end
-end
\ No newline at end of file
+end
--
2.6.3
[Attachment #6 (application/pgp-signature)]
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic