fix(xss): prevent XSS on omniauth external window vector

lynndylanhurley · Jun 30, 2017 · 9fdf831 · 9fdf831
1 parent 53f19d0
commit 9fdf831
Show file tree

Hide file tree

Showing 2 changed files with 12 additions and 12 deletions.
diff --git a/app/views/devise_token_auth/omniauth_external_window.html.erb b/app/views/devise_token_auth/omniauth_external_window.html.erb
@@ -15,7 +15,7 @@
             Cordova / PhoneGap)
       */
 
-      var data = <%= @data.to_json.html_safe %>;
+      var data = JSON.parse(decodeURIComponent('<%= URI::escape( @data.to_json ) %>'));
 
       window.addEventListener("message", function(ev) {
         if (ev.data === "requestCredentials") {

diff --git a/test/controllers/devise_token_auth/omniauth_callbacks_controller_test.rb b/test/controllers/devise_token_auth/omniauth_callbacks_controller_test.rb
@@ -16,6 +16,11 @@ class OmniauthTest < ActionDispatch::IntegrationTest
     @redirect_url = "http://ng-token-auth.dev/"
   end
 
+  def get_parsed_data_json
+    encoded_json_data = @response.body.match(/var data \= JSON.parse\(decodeURIComponent\(\'(.+)\'\)\)\;/)[1]
+    JSON.parse(URI::unescape(encoded_json_data))
+  end
+
   describe 'success callback' do
     setup do
       OmniAuth.config.mock_auth[:facebook] = OmniAuth::AuthHash.new({
@@ -207,8 +212,7 @@ class OmniauthTest < ActionDispatch::IntegrationTest
     end
 
     def assert_expected_data_in_new_window
-      data_json = @response.body.match(/var data \= (.+)\;/)[1]
-      data = ActiveSupport::JSON.decode(data_json)
+      data = get_parsed_data_json
       expected_data = @resource.as_json.merge(controller.auth_params.as_json)
       expected_data = ActiveSupport::JSON.decode(expected_data.to_json)
       assert_equal(expected_data.merge("message" => "deliverCredentials"), data)
@@ -262,8 +266,7 @@ def get_success(params = {})
       }
       assert_equal 200, response.status
 
-      data_json = @response.body.match(/var data \= (.+)\;/)[1]
-      data = ActiveSupport::JSON.decode(data_json)
+      data = get_parsed_data_json
 
       assert_equal({"error"=>"invalid_credentials", "message"=>"authFailure"}, data)
     end
@@ -310,9 +313,8 @@ def get_success(params = {})
                        auth_origin_url: @bad_redirect_url,
                        omniauth_window_type: 'newWindow'
 
-      data_json = @response.body.match(/var data \= (.+)\;/)[1]
-      data = ActiveSupport::JSON.decode(data_json)
-      assert_equal "Redirect to '#{@bad_redirect_url}' not allowed.",
+      data = get_parsed_data_json
+      assert_equal "Redirect to &#39;#{@bad_redirect_url}&#39; not allowed.",
                    data['error']
     end
 
@@ -321,8 +323,7 @@ def get_success(params = {})
                        auth_origin_url: @good_redirect_url,
                        omniauth_window_type: 'newWindow'
 
-      data_json = @response.body.match(/var data \= (.+)\;/)[1]
-      data = ActiveSupport::JSON.decode(data_json)
+      data = get_parsed_data_json
       assert_equal @user_email, data['email']
     end
 
@@ -332,8 +333,7 @@ def get_success(params = {})
                        auth_origin_url: @good_redirect_url,
                        omniauth_window_type: 'newWindow'
 
-      data_json = @response.body.match(/var data \= (.+)\;/)[1]
-      data = ActiveSupport::JSON.decode(data_json)
+      data = get_parsed_data_json
       assert_equal @user_email, data['email']
     end