From 9415935902f120a9bac0bfce7129725a0db38ed3 Mon Sep 17 00:00:00 2001
From: Michael Koziarski <michael@koziarski.com>
Date: Thu, 8 Oct 2009 09:31:20 +1300
Subject: Switch to on-by-default XSS escaping for rails.

  This consists of:

  * String#html_safe! a method to mark a string as 'safe'
  * ActionView::SafeBuffer a string subclass which escapes anything unsafe which is concatenated to it
  * Calls to String#html_safe! throughout the rails helpers
  * a 'raw' helper which lets you concatenate trusted HTML from non-safety-aware sources (e.g. presantized strings in the DB)
  * New ERB implementation based on erubis which uses a SafeBuffer instead of a String

Hat tip to Django for the inspiration.
---
 actionpack/lib/action_view/helpers/active_model_helper.rb |  1 +
 actionpack/lib/action_view/helpers/asset_tag_helper.rb    |  6 +++---
 actionpack/lib/action_view/helpers/capture_helper.rb      |  2 +-
 actionpack/lib/action_view/helpers/date_helper.rb         |  6 +++---
 actionpack/lib/action_view/helpers/form_helper.rb         |  4 ++--
 actionpack/lib/action_view/helpers/form_options_helper.rb |  2 +-
 actionpack/lib/action_view/helpers/form_tag_helper.rb     |  6 +++---
 actionpack/lib/action_view/helpers/prototype_helper.rb    |  2 +-
 actionpack/lib/action_view/helpers/raw_output_helper.rb   |  9 +++++++++
 actionpack/lib/action_view/helpers/sanitize_helper.rb     | 12 ++++++++++--
 actionpack/lib/action_view/helpers/tag_helper.rb          |  8 ++++----
 actionpack/lib/action_view/helpers/url_helper.rb          | 10 +++++-----
 12 files changed, 43 insertions(+), 25 deletions(-)
 create mode 100644 actionpack/lib/action_view/helpers/raw_output_helper.rb

(limited to 'actionpack/lib/action_view/helpers')

diff --git a/actionpack/lib/action_view/helpers/active_model_helper.rb b/actionpack/lib/action_view/helpers/active_model_helper.rb
index 3e6e62237d..c9231225e1 100644
--- a/actionpack/lib/action_view/helpers/active_model_helper.rb
+++ b/actionpack/lib/action_view/helpers/active_model_helper.rb
@@ -91,6 +91,7 @@ module ActionView
         yield contents if block_given?
         contents << submit_tag(submit_value)
         contents << '</form>'
+        contents.html_safe!
       end
 
       # Returns a string containing the error message attached to the +method+ on the +object+ if one exists.
diff --git a/actionpack/lib/action_view/helpers/asset_tag_helper.rb b/actionpack/lib/action_view/helpers/asset_tag_helper.rb
index 95f00cda39..faa7f2e2e9 100644
--- a/actionpack/lib/action_view/helpers/asset_tag_helper.rb
+++ b/actionpack/lib/action_view/helpers/asset_tag_helper.rb
@@ -289,7 +289,7 @@ module ActionView
         else
           sources = expand_javascript_sources(sources, recursive)
           ensure_javascript_sources!(sources) if cache
-          sources.collect { |source| javascript_src_tag(source, options) }.join("\n")
+          sources.collect { |source| javascript_src_tag(source, options) }.join("\n").html_safe!
         end
       end
 
@@ -440,7 +440,7 @@ module ActionView
         else
           sources = expand_stylesheet_sources(sources, recursive)
           ensure_stylesheet_sources!(sources) if cache
-          sources.collect { |source| stylesheet_tag(source, options) }.join("\n")
+          sources.collect { |source| stylesheet_tag(source, options) }.join("\n").html_safe!
         end
       end
 
@@ -584,7 +584,7 @@ module ActionView
 
         if sources.is_a?(Array)
           content_tag("video", options) do
-            sources.map { |source| tag("source", :src => source) }.join
+            sources.map { |source| tag("source", :src => source) }.join.html_safe!
           end
         else
           options[:src] = path_to_video(sources)
diff --git a/actionpack/lib/action_view/helpers/capture_helper.rb b/actionpack/lib/action_view/helpers/capture_helper.rb
index c90acc5ac2..b62df75dbb 100644
--- a/actionpack/lib/action_view/helpers/capture_helper.rb
+++ b/actionpack/lib/action_view/helpers/capture_helper.rb
@@ -143,7 +143,7 @@ module ActionView
       # Defaults to a new empty string.
       def with_output_buffer(buf = nil) #:nodoc:
         unless buf
-          buf = ''
+          buf = ActionView::SafeBuffer.new
           buf.force_encoding(output_buffer.encoding) if buf.respond_to?(:force_encoding)
         end
         self.output_buffer, old_buffer = buf, output_buffer
diff --git a/actionpack/lib/action_view/helpers/date_helper.rb b/actionpack/lib/action_view/helpers/date_helper.rb
index 8a7a870b99..4b51dc7856 100644
--- a/actionpack/lib/action_view/helpers/date_helper.rb
+++ b/actionpack/lib/action_view/helpers/date_helper.rb
@@ -916,15 +916,15 @@ module ActionView
 
     class InstanceTag #:nodoc:
       def to_date_select_tag(options = {}, html_options = {})
-        datetime_selector(options, html_options).select_date
+        datetime_selector(options, html_options).select_date.html_safe!
       end
 
       def to_time_select_tag(options = {}, html_options = {})
-        datetime_selector(options, html_options).select_time
+        datetime_selector(options, html_options).select_time.html_safe!
       end
 
       def to_datetime_select_tag(options = {}, html_options = {})
-        datetime_selector(options, html_options).select_datetime
+        datetime_selector(options, html_options).select_datetime.html_safe!
       end
 
       private
diff --git a/actionpack/lib/action_view/helpers/form_helper.rb b/actionpack/lib/action_view/helpers/form_helper.rb
index 32b9c4a7dd..c46b39fc23 100644
--- a/actionpack/lib/action_view/helpers/form_helper.rb
+++ b/actionpack/lib/action_view/helpers/form_helper.rb
@@ -282,7 +282,7 @@ module ActionView
 
         concat(form_tag(options.delete(:url) || {}, options.delete(:html) || {}))
         fields_for(object_name, *(args << options), &proc)
-        concat('</form>')
+        concat('</form>'.html_safe!)
       end
 
       def apply_form_for_options!(object_or_array, options) #:nodoc:
@@ -809,7 +809,7 @@ module ActionView
         add_default_name_and_id(options)
         hidden = tag("input", "name" => options["name"], "type" => "hidden", "value" => options['disabled'] && checked ? checked_value : unchecked_value)
         checkbox = tag("input", options)
-        hidden + checkbox
+        (hidden + checkbox).html_safe!
       end
 
       def to_boolean_select_tag(options = {})
diff --git a/actionpack/lib/action_view/helpers/form_options_helper.rb b/actionpack/lib/action_view/helpers/form_options_helper.rb
index 3db5202e7d..935ab5f3e8 100644
--- a/actionpack/lib/action_view/helpers/form_options_helper.rb
+++ b/actionpack/lib/action_view/helpers/form_options_helper.rb
@@ -296,7 +296,7 @@ module ActionView
           options << %(<option value="#{html_escape(value.to_s)}"#{selected_attribute}#{disabled_attribute}>#{html_escape(text.to_s)}</option>)
         end
 
-        options_for_select.join("\n")
+        options_for_select.join("\n").html_safe!
       end
 
       # Returns a string of option tags that have been compiled by iterating over the +collection+ and assigning the
diff --git a/actionpack/lib/action_view/helpers/form_tag_helper.rb b/actionpack/lib/action_view/helpers/form_tag_helper.rb
index 1d851ecbd7..7688e786b1 100644
--- a/actionpack/lib/action_view/helpers/form_tag_helper.rb
+++ b/actionpack/lib/action_view/helpers/form_tag_helper.rb
@@ -440,7 +440,7 @@ module ActionView
         concat(tag(:fieldset, options, true))
         concat(content_tag(:legend, legend)) unless legend.blank?
         concat(content)
-        concat("</fieldset>")
+        concat("</fieldset>".html_safe!)
       end
 
       private
@@ -467,14 +467,14 @@ module ActionView
 
         def form_tag_html(html_options)
           extra_tags = extra_tags_for_form(html_options)
-          tag(:form, html_options, true) + extra_tags
+          (tag(:form, html_options, true) + extra_tags).html_safe!
         end
 
         def form_tag_in_block(html_options, &block)
           content = capture(&block)
           concat(form_tag_html(html_options))
           concat(content)
-          concat("</form>")
+          concat("</form>".html_safe!)
         end
 
         def token_tag
diff --git a/actionpack/lib/action_view/helpers/prototype_helper.rb b/actionpack/lib/action_view/helpers/prototype_helper.rb
index 03f1dabb4e..8c1f0ad81f 100644
--- a/actionpack/lib/action_view/helpers/prototype_helper.rb
+++ b/actionpack/lib/action_view/helpers/prototype_helper.rb
@@ -395,7 +395,7 @@ module ActionView
 
         concat(form_remote_tag(options))
         fields_for(object_name, *(args << options), &proc)
-        concat('</form>')
+        concat('</form>'.html_safe!)
       end
       alias_method :form_remote_for, :remote_form_for
 
diff --git a/actionpack/lib/action_view/helpers/raw_output_helper.rb b/actionpack/lib/action_view/helpers/raw_output_helper.rb
new file mode 100644
index 0000000000..79b0e4ee75
--- /dev/null
+++ b/actionpack/lib/action_view/helpers/raw_output_helper.rb
@@ -0,0 +1,9 @@
+module ActionView #:nodoc:
+  module Helpers #:nodoc:
+    module RawOutputHelper
+      def raw(stringish)
+        stringish.to_s.html_safe!
+      end
+    end
+  end
+end
\ No newline at end of file
diff --git a/actionpack/lib/action_view/helpers/sanitize_helper.rb b/actionpack/lib/action_view/helpers/sanitize_helper.rb
index d89b955317..69d0d0fb67 100644
--- a/actionpack/lib/action_view/helpers/sanitize_helper.rb
+++ b/actionpack/lib/action_view/helpers/sanitize_helper.rb
@@ -49,7 +49,11 @@ module ActionView
       # confuse browsers.
       #
       def sanitize(html, options = {})
-        self.class.white_list_sanitizer.sanitize(html, options)
+        returning self.class.white_list_sanitizer.sanitize(html, options) do |sanitized|
+          if sanitized
+            sanitized.html_safe!
+          end
+        end
       end
 
       # Sanitizes a block of CSS code. Used by +sanitize+ when it comes across a style attribute.
@@ -72,7 +76,11 @@ 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)
+        returning self.class.full_sanitizer.sanitize(html) do |sanitized|
+          if sanitized
+            sanitized.html_safe!
+          end
+        end
       end
 
       # Strips all link tags from +text+ leaving just the link text.
diff --git a/actionpack/lib/action_view/helpers/tag_helper.rb b/actionpack/lib/action_view/helpers/tag_helper.rb
index 7fae0f6b8d..ceddbd8cc1 100644
--- a/actionpack/lib/action_view/helpers/tag_helper.rb
+++ b/actionpack/lib/action_view/helpers/tag_helper.rb
@@ -41,7 +41,7 @@ module ActionView
       #   tag("img", { :src => "open &amp; shut.png" }, false, false)
       #   # => <img src="open &amp; shut.png" />
       def tag(name, options = nil, open = false, escape = true)
-        "<#{name}#{tag_options(options, escape) if options}#{open ? ">" : " />"}"
+        "<#{name}#{tag_options(options, escape) if options}#{open ? ">" : " />"}".html_safe!
       end
 
       # Returns an HTML block tag of type +name+ surrounding the +content+. Add
@@ -94,7 +94,7 @@ module ActionView
       #   cdata_section(File.read("hello_world.txt"))
       #   # => <![CDATA[<hello from a text file]]>
       def cdata_section(content)
-        "<![CDATA[#{content}]]>"
+        "<![CDATA[#{content}]]>".html_safe!
       end
 
       # Returns an escaped version of +html+ without affecting existing escaped entities.
@@ -128,7 +128,7 @@ module ActionView
 
         def content_tag_string(name, content, options, escape = true)
           tag_options = tag_options(options, escape) if options
-          "<#{name}#{tag_options}>#{content}</#{name}>"
+          "<#{name}#{tag_options}>#{content}</#{name}>".html_safe!
         end
 
         def tag_options(options, escape = true)
@@ -143,7 +143,7 @@ module ActionView
                 attrs << %(#{key}="#{final_value}")
               end
             end
-            " #{attrs.sort * ' '}" unless attrs.empty?
+            " #{attrs.sort * ' '}".html_safe! unless attrs.empty?
           end
         end
     end
diff --git a/actionpack/lib/action_view/helpers/url_helper.rb b/actionpack/lib/action_view/helpers/url_helper.rb
index 204d4d71e1..e651bc17a9 100644
--- a/actionpack/lib/action_view/helpers/url_helper.rb
+++ b/actionpack/lib/action_view/helpers/url_helper.rb
@@ -93,7 +93,7 @@ module ActionView
           polymorphic_path(options)
         end
 
-        escape ? escape_once(url) : url
+        (escape ? escape_once(url) : url).html_safe!
       end
 
       # Creates a link tag of the given +name+ using a URL created by the set
@@ -220,7 +220,7 @@ module ActionView
         if block_given?
           options      = args.first || {}
           html_options = args.second
-          concat(link_to(capture(&block), options, html_options))
+          concat(link_to(capture(&block), options, html_options).html_safe!)
         else
           name         = args[0]
           options      = args[1] || {}
@@ -238,7 +238,7 @@ module ActionView
           end
 
           href_attr = "href=\"#{url}\"" unless href
-          "<a #{href_attr}#{tag_options}>#{name || url}</a>"
+          "<a #{href_attr}#{tag_options}>#{ERB::Util.h(name || url)}</a>".html_safe!
         end
       end
 
@@ -309,8 +309,8 @@ module ActionView
 
         html_options.merge!("type" => "submit", "value" => name)
 
-        "<form method=\"#{form_method}\" action=\"#{escape_once url}\" class=\"button-to\"><div>" +
-          method_tag + tag("input", html_options) + request_token_tag + "</div></form>"
+        ("<form method=\"#{form_method}\" action=\"#{escape_once url}\" class=\"button-to\"><div>" +
+          method_tag + tag("input", html_options) + request_token_tag + "</div></form>").html_safe!
       end
 
 
-- 
cgit v1.2.3