CSRF protection from cross-origin <script> tags

Thanks to @homakov for sounding the alarm about JSONP-style data leaking

actionpack/CHANGELOG.md

@@ -1,3 +1,8 @@
+*   Extend cross-site request forgery (CSRF) protection to GET requests with
+    JavaScript responses, protecting apps from cross-origin `<script>` tags.
+
+    *Jeremy Kemper*
+
 *   Fix generating a path for engine inside a resources block.
 
     Fixes #8533.

actionpack/lib/action_controller/metal/request_forgery_protection.rb

@@ -5,14 +5,24 @@ module ActionController #:nodoc:
   class InvalidAuthenticityToken < ActionControllerError #:nodoc:
   end
 
+  class InvalidCrossOriginRequest < ActionControllerError #:nodoc:
+  end
+
   # Controller actions are protected from Cross-Site Request Forgery (CSRF) attacks
   # by including a token in the rendered html for your application. This token is
   # stored as a random string in the session, to which an attacker does not have
   # access. When a request reaches your application, \Rails verifies the received
   # token with the token in the session. Only HTML and JavaScript requests are checked,
   # so this will not protect your XML API (presumably you'll have a different
-  # authentication scheme there anyway). Also, GET requests are not protected as these
-  # should be idempotent.
+  # authentication scheme there anyway).
+  #
+  # GET requests are not protected since they don't have side effects like writing
+  # to the database and don't leak sensitive information. JavaScript requests are
+  # an exception: a third-party site can use a <script> tag to reference a JavaScript
+  # URL on your site. When your JavaScript response loads on their site, it executes.
+  # With carefully crafted JavaScript on their end, sensitive data in your JavaScript
+  # response may be extracted. To prevent this, only XmlHttpRequest (known as XHR or
+  # Ajax) requests are allowed to make GET requests for JavaScript responses.
   #
   # It's important to remember that XML or JSON requests are also affected and if
   # you're building an API you'll need something like:
@@ -65,17 +75,16 @@ module RequestForgeryProtection
     module ClassMethods
       # Turn on request forgery protection. Bear in mind that only non-GET, HTML/JavaScript requests are checked.
       #
+      #   class ApplicationController < ActionController::Base
+      #     protect_from_forgery
+      #   end
+      #
       #   class FooController < ApplicationController
       #     protect_from_forgery except: :index
       #
-      # You can disable csrf protection on controller-by-controller basis:
-      #
+      # You can disable CSRF protection on controller by skipping the verification before_action:
       #   skip_before_action :verify_authenticity_token
       #
-      # It can also be disabled for specific controller actions:
-      #
-      #   skip_before_action :verify_authenticity_token, except: [:create]
-      #
       # Valid Options:
       #
       # * <tt>:only/:except</tt> - Passed to the <tt>before_action</tt> call. Set which actions are verified.
@@ -89,6 +98,7 @@ def protect_from_forgery(options = {})
         self.forgery_protection_strategy = protection_method_class(options[:with] || :null_session)
         self.request_forgery_protection_token ||= :authenticity_token
         prepend_before_action :verify_authenticity_token, options
+        append_after_action :verify_same_origin_request
       end
 
       private
@@ -169,18 +179,56 @@ def handle_unverified_request
     end
 
     protected
+      # The actual before_action that is used to verify the CSRF token.
+      # Don't override this directly. Provide your own forgery protection
+      # strategy instead. If you override, you'll disable same-origin
+      # `<script>` verification.
+      #
+      # Lean on the protect_from_forgery declaration to mark which actions are
+      # due for same-origin request verification. If protect_from_forgery is
+      # enabled on an action, this before_action flags its after_action to
+      # verify that JavaScript responses are for XHR requests, ensuring they
+      # follow the browser's same-origin policy.
+      def verify_authenticity_token
+        @marked_for_same_origin_verification = true
+
+        if !verified_request?
+          logger.warn "Can't verify CSRF token authenticity" if logger
+          handle_unverified_request
+        end
+      end
+
       def handle_unverified_request
         forgery_protection_strategy.new(self).handle_unverified_request
       end
 
-      # The actual before_action that is used. Modify this to change how you handle unverified requests.
-      def verify_authenticity_token
-        unless verified_request?
-          logger.warn "Can't verify CSRF token authenticity" if logger
-          handle_unverified_request
+      CROSS_ORIGIN_JAVASCRIPT_WARNING = "Security warning: an embedded " \
+        "<script> tag on another site requested protected JavaScript. " \
+        "If you know what you're doing, go ahead and disable forgery " \
+        "protection on this action to permit cross-origin JavaScript embedding."
+      private_constant :CROSS_ORIGIN_JAVASCRIPT_WARNING
+
+      # If `verify_authenticity_token` was run (indicating that we have
+      # forgery protection enabled for this request) then also verify that
+      # we aren't serving an unauthorized cross-origin response.
+      def verify_same_origin_request
+        if marked_for_same_origin_verification? && non_xhr_javascript_response?
+          logger.warn CROSS_ORIGIN_JAVASCRIPT_WARNING if logger
+          raise ActionController::InvalidCrossOriginRequest, CROSS_ORIGIN_JAVASCRIPT_WARNING
         end
       end
 
+      # If the `verify_authenticity_token` before_action ran, verify that
+      # JavaScript responses are only served to same-origin GET requests.
+      def marked_for_same_origin_verification?
+        defined? @marked_for_same_origin_verification
+      end
+
+      # Check for cross-origin JavaScript responses.
+      def non_xhr_javascript_response?
+        content_type =~ %r(\Atext/javascript) && !request.xhr?
+      end
+
       # Returns true or false if a request is verified. Checks:
       #
       # * is it a GET or HEAD request?  Gets should be safe and idempotent

actionpack/test/controller/render_js_test.rb

@@ -22,7 +22,7 @@ def show_partial
   tests TestController
 
   def test_render_vanilla_js
-    get :render_vanilla_js_hello
+    xhr :get, :render_vanilla_js_hello
     assert_equal "alert('hello')", @response.body
     assert_equal "text/javascript", @response.content_type
   end

actionpack/test/controller/render_json_test.rb

@@ -100,13 +100,13 @@ def test_render_json_with_status
   end
 
   def test_render_json_with_callback
-    get :render_json_hello_world_with_callback
+    xhr :get, :render_json_hello_world_with_callback
     assert_equal 'alert({"hello":"world"})', @response.body
     assert_equal 'text/javascript', @response.content_type
   end
 
   def test_render_json_with_custom_content_type
-    get :render_json_with_custom_content_type
+    xhr :get, :render_json_with_custom_content_type
     assert_equal '{"hello":"world"}', @response.body
     assert_equal 'text/javascript', @response.content_type
   end

actionpack/test/controller/request_forgery_protection_test.rb

@@ -52,18 +52,36 @@ def form_for_remote_with_external_token
     render :inline => "<%= form_for(:some_resource, :remote => true, :authenticity_token => 'external_token') {} %>"
   end
 
+  def same_origin_js
+    render js: 'foo();'
+  end
+
+  def negotiate_same_origin
+    respond_to do |format|
+      format.js { same_origin_js }
+    end
+  end
+
+  def cross_origin_js
+    same_origin_js
+  end
+
+  def negotiate_cross_origin
+    negotiate_same_origin
+  end
+
   def rescue_action(e) raise e end
 end
 
 # sample controllers
 class RequestForgeryProtectionControllerUsingResetSession < ActionController::Base
   include RequestForgeryProtectionActions
-  protect_from_forgery :only => %w(index meta), :with => :reset_session
+  protect_from_forgery :only => %w(index meta same_origin_js negotiate_same_origin), :with => :reset_session
 end
 
 class RequestForgeryProtectionControllerUsingException < ActionController::Base
   include RequestForgeryProtectionActions
-  protect_from_forgery :only => %w(index meta), :with => :exception
+  protect_from_forgery :only => %w(index meta same_origin_js negotiate_same_origin), :with => :exception
 end
 
 class RequestForgeryProtectionControllerUsingNullSession < ActionController::Base
@@ -201,7 +219,7 @@ def test_should_not_allow_post_without_token
   end
 
   def test_should_not_allow_post_without_token_irrespective_of_format
-    assert_blocked { post :index, :format=>'xml' }
+    assert_blocked { post :index, format: 'xml' }
   end
 
   def test_should_not_allow_patch_without_token
@@ -271,6 +289,38 @@ def test_should_warn_on_missing_csrf_token
     end
   end
 
+  def test_should_only_allow_same_origin_js_get_with_xhr_header
+    assert_cross_origin_blocked { get :same_origin_js }
+    assert_cross_origin_blocked { get :same_origin_js, format: 'js' }
+    assert_cross_origin_blocked do
+      @request.accept = 'text/javascript'
+      get :negotiate_same_origin
+    end
+
+    assert_cross_origin_not_blocked { xhr :get, :same_origin_js }
+    assert_cross_origin_not_blocked { xhr :get, :same_origin_js, format: 'js' }
+    assert_cross_origin_not_blocked do
+      @request.accept = 'text/javascript'
+      xhr :get, :negotiate_same_origin
+    end
+  end
+
+  def test_should_only_allow_cross_origin_js_get_without_xhr_header_if_protection_disabled
+    assert_cross_origin_not_blocked { get :cross_origin_js }
+    assert_cross_origin_not_blocked { get :cross_origin_js, format: 'js' }
+    assert_cross_origin_not_blocked do
+      @request.accept = 'text/javascript'
+      get :negotiate_cross_origin
+    end
+
+    assert_cross_origin_not_blocked { xhr :get, :cross_origin_js }
+    assert_cross_origin_not_blocked { xhr :get, :cross_origin_js, format: 'js' }
+    assert_cross_origin_not_blocked do
+      @request.accept = 'text/javascript'
+      xhr :get, :negotiate_cross_origin
+    end
+  end
+
   def assert_blocked
     session[:something_like_user_id] = 1
     yield
@@ -282,6 +332,16 @@ def assert_not_blocked
     assert_nothing_raised { yield }
     assert_response :success
   end
+
+  def assert_cross_origin_blocked
+    assert_raises(ActionController::InvalidCrossOriginRequest) do
+      yield
+    end
+  end
+
+  def assert_cross_origin_not_blocked
+    assert_not_blocked { yield }
+  end
 end
 
 # OK let's get our test on
@@ -305,13 +365,13 @@ class RequestForgeryProtectionControllerUsingResetSessionTest < ActionController
   end
 end
 
-class NullSessionDummyKeyGenerator
-  def generate_key(secret)
-    '03312270731a2ed0d11ed091c2338a06'
+class RequestForgeryProtectionControllerUsingNullSessionTest < ActionController::TestCase
+  class NullSessionDummyKeyGenerator
+    def generate_key(secret)
+      '03312270731a2ed0d11ed091c2338a06'
+    end
   end
-end
 
-class RequestForgeryProtectionControllerUsingNullSessionTest < ActionController::TestCase  
   def setup
     @request.env[ActionDispatch::Cookies::GENERATOR_KEY] = NullSessionDummyKeyGenerator.new
   end
@@ -375,8 +435,8 @@ def test_should_allow_all_methods_without_token
 
 class CustomAuthenticityParamControllerTest < ActionController::TestCase
   def setup
-    ActionController::Base.request_forgery_protection_token = :custom_token_name
     super
+    ActionController::Base.request_forgery_protection_token = :custom_token_name
   end
 
   def teardown

guides/source/security.md

@@ -230,13 +230,15 @@ Or the attacker places the code into the onmouseover event handler of an image:
 <img src="http://www.harmless.com/img" width="400" height="400" onmouseover="..." />
 ```
 
-There are many other possibilities, including Ajax to attack the victim in the background.
The _solution to this is including a security token in non-GET requests_ which check on the server-side. In Rails 2 or higher, this is a one-liner in the application controller:
+There are many other possibilities, like using a `<script>` tag to make a cross-site request to a URL with a JSONP or JavaScript response. The response is executable code that the attacker can find a way to run, possibly extracting sensitive data. To protect against this data leakage, we disallow cross-site `<script>` tags. Only Ajax requests may have JavaScript responses since XmlHttpRequest is subject to the browser Same-Origin policy - meaning only your site can initiate the request.
+
+To protect against all other forged requests, we introduce a _required security token_ that our site knows but other sites don't know. We include the security token in requests and verify it on the server. This is a one-liner in your application controller:
 
 ```ruby
-protect_from_forgery secret: "123456789012345678901234567890..."
+protect_from_forgery
 ```
 
-This will automatically include a security token, calculated from the current session and the server-side secret, in all forms and Ajax requests generated by Rails. You won't need the secret, if you use CookieStorage as session storage. If the security token doesn't match what was expected, the session will be reset. **Note:** In Rails versions prior to 3.0.4, this raised an `ActionController::InvalidAuthenticityToken` error.
+This will automatically include a security token in all forms and Ajax requests generated by Rails. If the security token doesn't match what was expected, the session will be reset.
 
 It is common to use persistent cookies to store user information, with `cookies.permanent` for example. In this case, the cookies will not be cleared and the out of the box CSRF protection will not be effective. If you are using a different cookie store than the session for this information, you must handle what to do with it yourself: