aboutsummaryrefslogtreecommitdiffstats
path: root/actionpack/lib/action_view
diff options
context:
space:
mode:
authorMichael Koziarski <michael@koziarski.com>2009-10-08 09:31:20 +1300
committerMichael Koziarski <michael@koziarski.com>2009-10-08 09:31:20 +1300
commit9415935902f120a9bac0bfce7129725a0db38ed3 (patch)
tree654d184caccfd7e1de4d60236fb7813bf1177d84 /actionpack/lib/action_view
parentf27e7ebc0e2a55a268631c78d49a5b70b06ad59a (diff)
downloadrails-9415935902f120a9bac0bfce7129725a0db38ed3.tar.gz
rails-9415935902f120a9bac0bfce7129725a0db38ed3.tar.bz2
rails-9415935902f120a9bac0bfce7129725a0db38ed3.zip
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.
Diffstat (limited to 'actionpack/lib/action_view')
-rw-r--r--actionpack/lib/action_view/base.rb2
-rw-r--r--actionpack/lib/action_view/erb/util.rb12
-rw-r--r--actionpack/lib/action_view/helpers.rb2
-rw-r--r--actionpack/lib/action_view/helpers/active_model_helper.rb1
-rw-r--r--actionpack/lib/action_view/helpers/asset_tag_helper.rb6
-rw-r--r--actionpack/lib/action_view/helpers/capture_helper.rb2
-rw-r--r--actionpack/lib/action_view/helpers/date_helper.rb6
-rw-r--r--actionpack/lib/action_view/helpers/form_helper.rb4
-rw-r--r--actionpack/lib/action_view/helpers/form_options_helper.rb2
-rw-r--r--actionpack/lib/action_view/helpers/form_tag_helper.rb6
-rw-r--r--actionpack/lib/action_view/helpers/prototype_helper.rb2
-rw-r--r--actionpack/lib/action_view/helpers/raw_output_helper.rb9
-rw-r--r--actionpack/lib/action_view/helpers/sanitize_helper.rb12
-rw-r--r--actionpack/lib/action_view/helpers/tag_helper.rb8
-rw-r--r--actionpack/lib/action_view/helpers/url_helper.rb10
-rw-r--r--actionpack/lib/action_view/render/partials.rb2
-rw-r--r--actionpack/lib/action_view/safe_buffer.rb28
-rw-r--r--actionpack/lib/action_view/template/handlers/erb.rb28
-rw-r--r--actionpack/lib/action_view/test_case.rb2
19 files changed, 112 insertions, 32 deletions
diff --git a/actionpack/lib/action_view/base.rb b/actionpack/lib/action_view/base.rb
index 664cc3b562..5a4e1bee43 100644
--- a/actionpack/lib/action_view/base.rb
+++ b/actionpack/lib/action_view/base.rb
@@ -260,7 +260,7 @@ module ActionView #:nodoc:
@assigns = assigns_for_first_render.each { |key, value| instance_variable_set("@#{key}", value) }
@controller = controller
@helpers = self.class.helpers || Module.new
- @_content_for = Hash.new {|h,k| h[k] = "" }
+ @_content_for = Hash.new {|h,k| h[k] = ActionView::SafeBuffer.new }
self.view_paths = view_paths
end
diff --git a/actionpack/lib/action_view/erb/util.rb b/actionpack/lib/action_view/erb/util.rb
index 3c77c5ce76..f767a5e27e 100644
--- a/actionpack/lib/action_view/erb/util.rb
+++ b/actionpack/lib/action_view/erb/util.rb
@@ -15,9 +15,19 @@ class ERB
# puts html_escape("is a > 0 & a < 10?")
# # => is a &gt; 0 &amp; a &lt; 10?
def html_escape(s)
- s.to_s.gsub(/[&"><]/) { |special| HTML_ESCAPE[special] }
+ s = s.to_s
+ if s.html_safe?
+ s
+ else
+ s.gsub(/[&"><]/) { |special| HTML_ESCAPE[special] }.html_safe!
+ end
end
+ alias h html_escape
+
+ module_function :html_escape
+ module_function :h
+
# A utility method for escaping HTML entities in JSON strings.
# This method is also aliased as <tt>j</tt>.
#
diff --git a/actionpack/lib/action_view/helpers.rb b/actionpack/lib/action_view/helpers.rb
index 652561f7f8..d63e8602f1 100644
--- a/actionpack/lib/action_view/helpers.rb
+++ b/actionpack/lib/action_view/helpers.rb
@@ -15,6 +15,7 @@ module ActionView #:nodoc:
autoload :JavaScriptHelper, 'action_view/helpers/javascript_helper'
autoload :NumberHelper, 'action_view/helpers/number_helper'
autoload :PrototypeHelper, 'action_view/helpers/prototype_helper'
+ autoload :RawOutputHelper, 'action_view/helpers/raw_output_helper'
autoload :RecordIdentificationHelper, 'action_view/helpers/record_identification_helper'
autoload :RecordTagHelper, 'action_view/helpers/record_tag_helper'
autoload :SanitizeHelper, 'action_view/helpers/sanitize_helper'
@@ -46,6 +47,7 @@ module ActionView #:nodoc:
include JavaScriptHelper
include NumberHelper
include PrototypeHelper
+ include RawOutputHelper
include RecordIdentificationHelper
include RecordTagHelper
include SanitizeHelper
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
diff --git a/actionpack/lib/action_view/render/partials.rb b/actionpack/lib/action_view/render/partials.rb
index 7f10f54d2e..4f60566a09 100644
--- a/actionpack/lib/action_view/render/partials.rb
+++ b/actionpack/lib/action_view/render/partials.rb
@@ -223,7 +223,7 @@ module ActionView
end
result = template ? collection_with_template(template) : collection_without_template
- result.join(spacer)
+ result.join(spacer).html_safe!
end
def collection_with_template(template)
diff --git a/actionpack/lib/action_view/safe_buffer.rb b/actionpack/lib/action_view/safe_buffer.rb
new file mode 100644
index 0000000000..8ba9cd80d6
--- /dev/null
+++ b/actionpack/lib/action_view/safe_buffer.rb
@@ -0,0 +1,28 @@
+
+module ActionView #:nodoc:
+ class SafeBuffer < String
+ def <<(value)
+ if value.html_safe?
+ super(value)
+ else
+ super(CGI.escapeHTML(value))
+ end
+ end
+
+ def concat(value)
+ self << value
+ end
+
+ def html_safe?
+ true
+ end
+
+ def html_safe!
+ self
+ end
+
+ def to_s
+ self
+ end
+ end
+end \ No newline at end of file
diff --git a/actionpack/lib/action_view/template/handlers/erb.rb b/actionpack/lib/action_view/template/handlers/erb.rb
index aab7baf442..a780ab8d85 100644
--- a/actionpack/lib/action_view/template/handlers/erb.rb
+++ b/actionpack/lib/action_view/template/handlers/erb.rb
@@ -1,7 +1,31 @@
require 'active_support/core_ext/class/attribute_accessors'
+require 'active_support/core_ext/string/output_safety'
+require 'erubis'
module ActionView
module TemplateHandlers
+ class Erubis < ::Erubis::Eruby
+ def add_preamble(src)
+ src << "@output_buffer = ActionView::SafeBuffer.new;"
+ end
+
+ def add_text(src, text)
+ src << "@output_buffer << ('" << escape_text(text) << "'.html_safe!);"
+ end
+
+ def add_expr_literal(src, code)
+ src << '@output_buffer << ((' << code << ').to_s);'
+ end
+
+ def add_expr_escaped(src, code)
+ src << '@output_buffer << ' << escaped_expr(code) << ';'
+ end
+
+ def add_postamble(src)
+ src << '@output_buffer.to_s'
+ end
+ end
+
class ERB < TemplateHandler
include Compilable
@@ -15,11 +39,9 @@ module ActionView
self.default_format = Mime::HTML
def compile(template)
- require 'erb'
-
magic = $1 if template.source =~ /\A(<%#.*coding[:=]\s*(\S+)\s*-?%>)/
erb = "#{magic}<% __in_erb_template=true %>#{template.source}"
- ::ERB.new(erb, nil, erb_trim_mode, '@output_buffer').src
+ Erubis.new(erb, :trim=>(self.class.erb_trim_mode == "-")).src
end
end
end
diff --git a/actionpack/lib/action_view/test_case.rb b/actionpack/lib/action_view/test_case.rb
index 441f462bc9..8beda24aba 100644
--- a/actionpack/lib/action_view/test_case.rb
+++ b/actionpack/lib/action_view/test_case.rb
@@ -55,7 +55,7 @@ module ActionView
setup :setup_with_controller
def setup_with_controller
@controller = TestController.new
- @output_buffer = ''
+ @output_buffer = ActionView::SafeBuffer.new
@rendered = ''
self.class.send(:include_helper_modules!)