diff options
| -rw-r--r-- | actionpack/CHANGELOG | 2 | ||||
| -rwxr-xr-x | actionpack/lib/action_controller.rb | 2 | ||||
| -rwxr-xr-x | actionpack/lib/action_controller/base.rb | 4 | ||||
| -rw-r--r-- | actionpack/lib/action_controller/request_forgery_protection.rb | 75 | ||||
| -rw-r--r-- | actionpack/lib/action_controller/rescue.rb | 3 | ||||
| -rw-r--r-- | actionpack/lib/action_view/base.rb | 2 | ||||
| -rw-r--r-- | actionpack/lib/action_view/helpers/form_tag_helper.rb | 12 | ||||
| -rw-r--r-- | actionpack/lib/action_view/helpers/prototype_helper.rb | 9 | ||||
| -rw-r--r-- | actionpack/lib/action_view/helpers/text_helper.rb | 38 | ||||
| -rw-r--r-- | actionpack/lib/action_view/helpers/url_helper.rb | 4 | ||||
| -rw-r--r-- | actionpack/test/controller/request_forgery_protection_test.rb | 217 | ||||
| -rw-r--r-- | actionpack/test/template/form_helper_test.rb | 4 | ||||
| -rw-r--r-- | actionpack/test/template/form_tag_helper_test.rb | 5 | ||||
| -rw-r--r-- | actionpack/test/template/prototype_helper_test.rb | 5 | ||||
| -rw-r--r-- | actionpack/test/template/scriptaculous_helper_test.rb | 4 | ||||
| -rw-r--r-- | actionpack/test/template/url_helper_test.rb | 4 |
16 files changed, 368 insertions, 22 deletions
diff --git a/actionpack/CHANGELOG b/actionpack/CHANGELOG index b684148f91..49fb5a1b3d 100644 --- a/actionpack/CHANGELOG +++ b/actionpack/CHANGELOG @@ -1,5 +1,7 @@ *SVN* +* Merge csrf_killer plugin into rails. Adds RequestForgeryProtection model that verifies session-specific _tokens for non-GET requests. [Rick] + * Secure #sanitize, #strip_tags, and #strip_links helpers against xss attacks. Closes #8877. [Rick, lifofifo, Jacques Distler] This merges and renames the popular white_list helper (along with some css sanitizing from Jacques Distler version of the same plugin). diff --git a/actionpack/lib/action_controller.rb b/actionpack/lib/action_controller.rb index 14d1539e17..e7a9eba560 100755 --- a/actionpack/lib/action_controller.rb +++ b/actionpack/lib/action_controller.rb @@ -54,6 +54,7 @@ require 'action_controller/session_management' require 'action_controller/http_authentication' require 'action_controller/components' require 'action_controller/record_identifier' +require 'action_controller/request_forgery_protection' require 'action_view' ActionController::Base.template_class = ActionView::Base @@ -74,4 +75,5 @@ ActionController::Base.class_eval do include ActionController::HttpAuthentication::Basic::ControllerMethods include ActionController::Components include ActionController::RecordIdentifier + include ActionController::RequestForgeryProtection end diff --git a/actionpack/lib/action_controller/base.rb b/actionpack/lib/action_controller/base.rb index 4630103119..d6dd698c6e 100755 --- a/actionpack/lib/action_controller/base.rb +++ b/actionpack/lib/action_controller/base.rb @@ -326,6 +326,10 @@ module ActionController #:nodoc: # Controls the resource action separator @@resource_action_separator = "/" cattr_accessor :resource_action_separator + + # Sets the token parameter name for RequestForgery. Calling #verify_token sets it to :_token by default + @@request_forgery_protection_token = nil + cattr_accessor :request_forgery_protection_token # Holds the request object that's primarily used to get environment variables through access like # <tt>request.env["REQUEST_URI"]</tt>. diff --git a/actionpack/lib/action_controller/request_forgery_protection.rb b/actionpack/lib/action_controller/request_forgery_protection.rb new file mode 100644 index 0000000000..9aef1ae833 --- /dev/null +++ b/actionpack/lib/action_controller/request_forgery_protection.rb @@ -0,0 +1,75 @@ +module ActionController #:nodoc: + class InvalidToken < ActionControllerError; end + + # Protect a controller's actions with the #verify_token method. Failure to validate will result in a ActionController::InvalidToken + # exception. Customize the error message through the use of rescue_templates and rescue_action_in_public. + # + # class FooController < ApplicationController + # # uses the cookie session store + # verify_token :except => :index + # + # # uses one of the other session stores that uses a session_id value. + # verify_token :secret => 'my-little-pony', :except => :index + # end + # + # Valid Options: + # + # * <tt>:only/:except</tt> - passed to the before_filter call. Set which actions are verified. + # * <tt>:secret</tt> - Custom salt used to generate the form_token. Leave this off if you are using the cookie session store. + # * <tt>:digest</tt> - Message digest used for hashing. Defaults to 'SHA1' + module RequestForgeryProtection + def self.included(base) + base.class_eval do + class_inheritable_accessor :verify_token_options + self.verify_token_options = {} + helper_method :form_token + end + base.extend(ClassMethods) + end + + module ClassMethods + def verify_token(options = {}) + self.request_forgery_protection_token ||= :_token + before_filter :verify_request_token, :only => options.delete(:only), :except => options.delete(:except) + verify_token_options.update(options) + end + end + + protected + # The actual before_filter that is used. Modify this to change how you handle unverified requests. + def verify_request_token + verified_request? || raise(ActionController::InvalidToken) + end + + # Returns true or false if a request is verified. Checks: + # + # * is the format restricted? By default, only HTML and AJAX requests are checked. + # * is it a GET request? Gets should be safe and idempotent + # * Does the form_token match the given _token value from the params? + def verified_request? + request_forgery_protection_token.nil? || request.method == :get || !verifiable_request_format? || form_token == params[request_forgery_protection_token] + end + + def verifiable_request_format? + request.format.html? || request.format.js? + end + + # Sets the token value for the current session. Pass a :secret option in #verify_token to add a custom salt to the hash. + def form_token + @form_token ||= verify_token_options[:secret] ? token_from_session_id : token_from_cookie_session + end + + # Generates a unique digest using the session_id and the CSRF secret. + def token_from_session_id + key = verify_token_options[:secret].respond_to?(:call) ? verify_token_options[:secret].call(@session) : verify_token_options[:secret] + digest = verify_token_options[:digest] || 'SHA1' + OpenSSL::HMAC.hexdigest(OpenSSL::Digest::Digest.new(digest), key.to_s, session.session_id.to_s) + end + + # No secret was given, so assume this is a cookie session store. + def token_from_cookie_session + session[:csrf_id] ||= CGI::Session.generate_unique_id + session.dbman.generate_digest(session[:csrf_id]) + end + end +end
\ No newline at end of file diff --git a/actionpack/lib/action_controller/rescue.rb b/actionpack/lib/action_controller/rescue.rb index 8cffc90d33..c1d2152acb 100644 --- a/actionpack/lib/action_controller/rescue.rb +++ b/actionpack/lib/action_controller/rescue.rb @@ -20,7 +20,8 @@ module ActionController #:nodoc: 'ActiveRecord::RecordInvalid' => :unprocessable_entity, 'ActiveRecord::RecordNotSaved' => :unprocessable_entity, 'ActionController::MethodNotAllowed' => :method_not_allowed, - 'ActionController::NotImplemented' => :not_implemented + 'ActionController::NotImplemented' => :not_implemented, + 'ActionController::InvalidToken' => :unprocessable_entity } DEFAULT_RESCUE_TEMPLATE = 'diagnostics' diff --git a/actionpack/lib/action_view/base.rb b/actionpack/lib/action_view/base.rb index 8e778f6830..ee908214db 100644 --- a/actionpack/lib/action_view/base.rb +++ b/actionpack/lib/action_view/base.rb @@ -328,6 +328,8 @@ module ActionView #:nodoc: @@sanitized_allowed_protocols.merge(attributes) end + delegate :request_forgery_protection_token, :to => :controller + @@template_handlers = HashWithIndifferentAccess.new module CompiledTemplates #:nodoc: diff --git a/actionpack/lib/action_view/helpers/form_tag_helper.rb b/actionpack/lib/action_view/helpers/form_tag_helper.rb index d8e8f2005e..cb16131cc4 100644 --- a/actionpack/lib/action_view/helpers/form_tag_helper.rb +++ b/actionpack/lib/action_view/helpers/form_tag_helper.rb @@ -401,10 +401,10 @@ module ActionView '' when /^post$/i, "", nil html_options["method"] = "post" - '' + request_forgery_protection_token ? content_tag(:div, token_tag, :style => 'margin:0;padding:0') : '' else html_options["method"] = "post" - content_tag(:div, tag(:input, :type => "hidden", :name => "_method", :value => method), :style => 'margin:0;padding:0') + content_tag(:div, tag(:input, :type => "hidden", :name => "_method", :value => method) + token_tag, :style => 'margin:0;padding:0') end end @@ -419,6 +419,14 @@ module ActionView concat(content, block.binding) concat("</form>", block.binding) end + + def token_tag + if request_forgery_protection_token.nil? + '' + else + tag(:input, :type => "hidden", :name => request_forgery_protection_token.to_s, :value => form_token) + end + end end end end diff --git a/actionpack/lib/action_view/helpers/prototype_helper.rb b/actionpack/lib/action_view/helpers/prototype_helper.rb index cc8c5ad54f..df28a0395b 100644 --- a/actionpack/lib/action_view/helpers/prototype_helper.rb +++ b/actionpack/lib/action_view/helpers/prototype_helper.rb @@ -738,6 +738,15 @@ module ActionView elsif options[:with] js_options['parameters'] = options[:with] end + + if request_forgery_protection_token + if js_options['parameters'] + js_options['parameters'] << " + '&" + else + js_options['parameters'] = "'" + end + js_options['parameters'] << "_token=' + encodeURIComponent('#{escape_javascript form_token}')" + end options_for_javascript(js_options) end diff --git a/actionpack/lib/action_view/helpers/text_helper.rb b/actionpack/lib/action_view/helpers/text_helper.rb index af6f6e4bb8..35896c44fb 100644 --- a/actionpack/lib/action_view/helpers/text_helper.rb +++ b/actionpack/lib/action_view/helpers/text_helper.rb @@ -325,15 +325,15 @@ module ActionView # strip_links('Blog: <a href="http://www.myblog.com/" class="nav" target=\"_blank\">Visit</a>.') # # => Blog: Visit def strip_links(html) - # Stupid firefox treats '<href="http://whatever.com" onClick="alert()">something' as link! - if html.index("<a") || html.index("<href") - tokenizer = HTML::Tokenizer.new(html) - result = '' - while token = tokenizer.next - node = HTML::Node.parse(nil, 0, 0, token, false) - result << node.to_s unless node.is_a?(HTML::Tag) && ["a", "href"].include?(node.name) - end - strip_links(result) # Recurse - handle all dirty nested links + if !html.blank? && html.index("<a") || html.index("<href") + tokenizer = HTML::Tokenizer.new(html) + result = returning [] do |result| + while token = tokenizer.next + node = HTML::Node.parse(nil, 0, 0, token, false) + result << node.to_s unless node.is_a?(HTML::Tag) && ["a", "href"].include?(node.name) + end + end + strip_links(result.join) # Recurse - handle all dirty nested links else html end @@ -441,6 +441,7 @@ module ActionView # that of html-scanner. # # ==== Examples + # # strip_tags("Strip <i>these</i> tags!") # # => Strip these tags! # @@ -450,22 +451,21 @@ module ActionView # strip_tags("<div id='top-bar'>Welcome to my website!</div>") # # => Welcome to my website! def strip_tags(html) - return html if html.blank? - if html.index("<") - text = "" - tokenizer = HTML::Tokenizer.new(html) + return html if html.blank? || !html.index("<") + tokenizer = HTML::Tokenizer.new(html) + text = returning [] do |text| while token = tokenizer.next node = HTML::Node.parse(nil, 0, 0, token, false) # result is only the content of any Text nodes text << node.to_s if node.class == HTML::Text end - # strip any comments, and if they have a newline at the end (ie. line with - # only a comment) strip that too - strip_tags(text.gsub(/<!--(.*?)-->[\n]?/m, "")) # Recurse - handle all dirty nested tags - else - html # already plain text - end + end + + # strip any comments, and if they have a newline at the end (ie. line with + # only a comment) strip that too + # Recurse - handle all dirty nested tags + strip_tags(text.join.gsub(/<!--(.*?)-->[\n]?/m, "")) end # Creates a Cycle object whose _to_s_ method cycles through elements of an diff --git a/actionpack/lib/action_view/helpers/url_helper.rb b/actionpack/lib/action_view/helpers/url_helper.rb index 010a789b85..02c5c40727 100644 --- a/actionpack/lib/action_view/helpers/url_helper.rb +++ b/actionpack/lib/action_view/helpers/url_helper.rb @@ -472,6 +472,10 @@ module ActionView submit_function << "m.setAttribute('name', '_method'); m.setAttribute('value', '#{method}'); f.appendChild(m);" end + if request_forgery_protection_token + submit_function << "var s = document.createElement('input'); s.setAttribute('type', 'hidden'); " + submit_function << "s.setAttribute('name', '_token'); s.setAttribute('value', '#{escape_javascript form_token}'); f.appendChild(s);" + end submit_function << "f.submit();" end diff --git a/actionpack/test/controller/request_forgery_protection_test.rb b/actionpack/test/controller/request_forgery_protection_test.rb new file mode 100644 index 0000000000..5c4dc1ee5f --- /dev/null +++ b/actionpack/test/controller/request_forgery_protection_test.rb @@ -0,0 +1,217 @@ +require File.dirname(__FILE__) + '/../abstract_unit' + +ActionController::Routing::Routes.draw do |map| + map.connect ':controller/:action/:id' +end + +class RequestForgeryProtectionController < ActionController::Base + verify_token :only => :index, :secret => 'abc' + + def index + render :inline => "<%= form_tag('/') {} %>" + end + + def unsafe + render :text => 'pwn' + end + + def rescue_action(e) raise e end +end + +class RequestForgeryProtectionControllerTest < Test::Unit::TestCase + def setup + @controller = RequestForgeryProtectionController.new + @request = ActionController::TestRequest.new + @response = ActionController::TestResponse.new + class << @request.session + def session_id() '123' end + end + @token = OpenSSL::HMAC.hexdigest(OpenSSL::Digest::Digest.new('SHA1'), 'abc', '123') + ActionController::Base.request_forgery_protection_token = :_token + end + + def teardown + ActionController::Base.request_forgery_protection_token = nil + end + + def test_should_render_form_with_token_tag + get :index + assert_select 'form>div>input[name=?][value=?]', '_token', @token + end + + # Replace this with your real tests. + def test_should_allow_get + get :index + assert_response :success + end + + def test_should_allow_post_without_token_on_unsafe_action + post :unsafe + assert_response :success + end + + def test_should_not_allow_post_without_token + assert_raises(ActionController::InvalidToken) { post :index } + end + + def test_should_not_allow_put_without_token + assert_raises(ActionController::InvalidToken) { put :index } + end + + def test_should_not_allow_delete_without_token + assert_raises(ActionController::InvalidToken) { delete :index } + end + + def test_should_not_allow_xhr_post_without_token + assert_raises(ActionController::InvalidToken) { xhr :post, :index } + end + + def test_should_not_allow_xhr_put_without_token + assert_raises(ActionController::InvalidToken) { xhr :put, :index } + end + + def test_should_not_allow_xhr_delete_without_token + assert_raises(ActionController::InvalidToken) { xhr :delete, :index } + end + + def test_should_allow_post_with_token + post :index, :_token => @token + assert_response :success + end + + def test_should_allow_put_with_token + put :index, :_token => @token + assert_response :success + end + + def test_should_allow_delete_with_token + delete :index, :_token => @token + assert_response :success + end + + def test_should_allow_post_with_xml + post :index, :format => 'xml' + assert_response :success + end + + def test_should_allow_put_with_xml + put :index, :format => 'xml' + assert_response :success + end + + def test_should_allow_delete_with_xml + delete :index, :format => 'xml' + assert_response :success + end +end + +# no token is given, assume the cookie store is used +class CsrfCookieMonsterController < ActionController::Base + verify_token :only => :index + + def index + render :inline => "<%= form_tag('/') {} %>" + end + + def unsafe + render :text => 'pwn' + end + + def rescue_action(e) raise e end +end + +class FakeSessionDbMan + def self.generate_digest(data) + Digest::SHA1.hexdigest("secure") + end +end + +class CsrfCookieMonsterControllerTest < Test::Unit::TestCase + def setup + @controller = CsrfCookieMonsterController.new + @request = ActionController::TestRequest.new + @response = ActionController::TestResponse.new + # simulate a cookie session store + @request.session.instance_variable_set(:@dbman, FakeSessionDbMan) + class << @request.session + attr_reader :dbman + end + @token = Digest::SHA1.hexdigest("secure") + ActionController::Base.request_forgery_protection_token = :_token + end + + def teardown + ActionController::Base.request_forgery_protection_token = nil + end + + def test_should_render_form_with_token_tag + get :index + assert_select 'form>div>input[name=?][value=?]', '_token', @token + end + + # Replace this with your real tests. + def test_should_allow_get + get :index + assert_response :success + end + + def test_should_allow_post_without_token_on_unsafe_action + post :unsafe + assert_response :success + end + + def test_should_not_allow_post_without_token + assert_raises(ActionController::InvalidToken) { post :index } + end + + def test_should_not_allow_put_without_token + assert_raises(ActionController::InvalidToken) { put :index } + end + + def test_should_not_allow_delete_without_token + assert_raises(ActionController::InvalidToken) { delete :index } + end + + def test_should_not_allow_xhr_post_without_token + assert_raises(ActionController::InvalidToken) { xhr :post, :index } + end + + def test_should_not_allow_xhr_put_without_token + assert_raises(ActionController::InvalidToken) { xhr :put, :index } + end + + def test_should_not_allow_xhr_delete_without_token + assert_raises(ActionController::InvalidToken) { xhr :delete, :index } + end + + def test_should_allow_post_with_token + post :index, :_token => @token + assert_response :success + end + + def test_should_allow_put_with_token + put :index, :_token => @token + assert_response :success + end + + def test_should_allow_delete_with_token + delete :index, :_token => @token + assert_response :success + end + + def test_should_allow_post_with_xml + post :index, :format => 'xml' + assert_response :success + end + + def test_should_allow_put_with_xml + put :index, :format => 'xml' + assert_response :success + end + + def test_should_allow_delete_with_xml + delete :index, :format => 'xml' + assert_response :success + end +end + diff --git a/actionpack/test/template/form_helper_test.rb b/actionpack/test/template/form_helper_test.rb index af63a056f8..9b22d4cef3 100644 --- a/actionpack/test/template/form_helper_test.rb +++ b/actionpack/test/template/form_helper_test.rb @@ -711,4 +711,8 @@ class FormHelperTest < Test::Unit::TestCase def post_path(post) "/posts/#{post.id}" end + + def request_forgery_protection_token + nil + end end diff --git a/actionpack/test/template/form_tag_helper_test.rb b/actionpack/test/template/form_tag_helper_test.rb index c582c26c68..f2c6678ddd 100644 --- a/actionpack/test/template/form_tag_helper_test.rb +++ b/actionpack/test/template/form_tag_helper_test.rb @@ -177,4 +177,9 @@ class FormTagHelperTest < Test::Unit::TestCase expected = %(<fieldset>Hello world!</fieldset>) assert_dom_equal expected, _erbout end + + def request_forgery_protection_token + nil + + end end diff --git a/actionpack/test/template/prototype_helper_test.rb b/actionpack/test/template/prototype_helper_test.rb index 4adfc90180..7bb4245c58 100644 --- a/actionpack/test/template/prototype_helper_test.rb +++ b/actionpack/test/template/prototype_helper_test.rb @@ -60,6 +60,11 @@ module BaseTest end protected + + def request_forgery_protection_token + nil + end + def create_generator block = Proc.new { |*args| yield *args if block_given? } JavaScriptGenerator.new self, &block diff --git a/actionpack/test/template/scriptaculous_helper_test.rb b/actionpack/test/template/scriptaculous_helper_test.rb index 514ba9107e..722839f15e 100644 --- a/actionpack/test/template/scriptaculous_helper_test.rb +++ b/actionpack/test/template/scriptaculous_helper_test.rb @@ -89,4 +89,8 @@ class ScriptaculousHelperTest < Test::Unit::TestCase assert_dom_equal %(<script type=\"text/javascript\">\n//<![CDATA[\nDroppables.add(\"droptarget1\", {accept:['tshirts','mugs'], onDrop:function(element){new Ajax.Updater('infobox', 'http://www.example.com/', {asynchronous:true, evalScripts:true, parameters:'id=' + encodeURIComponent(element.id)})}})\n//]]>\n</script>), drop_receiving_element("droptarget1", :accept => ['tshirts','mugs'], :update => 'infobox') end + + def request_forgery_protection_token + nil + end end diff --git a/actionpack/test/template/url_helper_test.rb b/actionpack/test/template/url_helper_test.rb index db1e226a7e..5707beeab1 100644 --- a/actionpack/test/template/url_helper_test.rb +++ b/actionpack/test/template/url_helper_test.rb @@ -267,6 +267,10 @@ class UrlHelperTest < Test::Unit::TestCase assert_dom_equal "<a href=\"mailto:%6d%65@%64%6f%6d%61%69%6e.%63%6f%6d\">me(at)domain(dot)com</a>", mail_to("me@domain.com", nil, :encode => "hex", :replace_at => "(at)", :replace_dot => "(dot)") assert_dom_equal "<script type=\"text/javascript\">eval(unescape('%64%6f%63%75%6d%65%6e%74%2e%77%72%69%74%65%28%27%3c%61%20%68%72%65%66%3d%22%6d%61%69%6c%74%6f%3a%6d%65%40%64%6f%6d%61%69%6e%2e%63%6f%6d%22%3e%4d%79%20%65%6d%61%69%6c%3c%2f%61%3e%27%29%3b'))</script>", mail_to("me@domain.com", "My email", :encode => "javascript", :replace_at => "(at)", :replace_dot => "(dot)") end + + def request_forgery_protection_token + nil + end end class UrlHelperWithControllerTest < Test::Unit::TestCase |