Do not mark strip_tags result as html_safe

Thanks to Marek Labos & Nethemba

CVE-2012-3465

3 files changed, 10 insertions, 3 deletions

diff --git a/actionpack/CHANGELOG.md b/actionpack/CHANGELOG.md
index 9c12e09392..aceef9ae27 100644
--- a/actionpack/CHANGELOG.md
+++ b/actionpack/CHANGELOG.md
@@ -1,5 +1,12 @@
 ## Rails 3.2.8 ##
 
+*   There is an XSS vulnerability in the strip_tags helper in Ruby on Rails, the
+    helper doesn't correctly handle malformed html.  As a result an attacker can
+    execute arbitrary javascript through the use of specially crafted malformed
+    html.
+
+    *Marek from Nethemba (www.nethemba.com) & Santiago Pastorino*
+
 *   When a "prompt" value is supplied to the `select_tag` helper, the "prompt" value is not escaped.
     If untrusted data is not escaped, and is supplied as the prompt value, there is a potential for XSS attacks.
     Vulnerable code will look something like this:
diff --git a/actionpack/lib/action_view/helpers/sanitize_helper.rb b/actionpack/lib/action_view/helpers/sanitize_helper.rb
index 7768c8c151..0f6a5ed405 100644
--- a/actionpack/lib/action_view/helpers/sanitize_helper.rb
+++ b/actionpack/lib/action_view/helpers/sanitize_helper.rb
@@ -80,7 +80,7 @@ module ActionView
       #   strip_tags("<div id='top-bar'>Welcome to my website!</div>")
       #   # => Welcome to my website!
       def strip_tags(html)
-        self.class.full_sanitizer.sanitize(html).try(:html_safe)
+        self.class.full_sanitizer.sanitize(html)
       end
 
       # Strips all link tags from +text+ leaving just the link text.
diff --git a/actionpack/test/template/sanitize_helper_test.rb b/actionpack/test/template/sanitize_helper_test.rb
index 222d4dbf4c..cc93b53ea6 100644
--- a/actionpack/test/template/sanitize_helper_test.rb
+++ b/actionpack/test/template/sanitize_helper_test.rb
@@ -42,9 +42,9 @@ class SanitizeHelperTest < ActionView::TestCase
     [nil, '', '   '].each do |blank|
       stripped = strip_tags(blank)
       assert_equal blank, stripped
-      assert stripped.html_safe? unless blank.nil?
     end
-    assert strip_tags("<script>").html_safe?
+    assert_equal "", strip_tags("<script>")
+    assert_equal "something &lt;img onerror=alert(1337)", ERB::Util.html_escape(strip_tags("something <img onerror=alert(1337)"))
   end
 
   def test_sanitize_is_marked_safe