From 96561a2ac0fab58e3e248458e19003e09f106ec4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rafael=20Mendon=C3=A7a=20Fran=C3=A7a?= Date: Wed, 25 Mar 2015 17:23:33 -0300 Subject: [PATCH] Properly check if the request is cross domain Fix CVE-2015-1840 --- src/rails.js | 31 ++++++++++++++++++++++++------ test/public/test/call-remote-callbacks.js | 14 -------------- test/public/test/call-remote.js | 32 ------------------------------- test/public/test/data-method.js | 26 +++++++++++++++++++++++++ test/public/test/override.js | 2 +- 5 files changed, 52 insertions(+), 53 deletions(-) diff --git a/src/rails.js b/src/rails.js index a4fb0be..1ee8859 100644 --- a/src/rails.js +++ b/src/rails.js @@ -86,16 +86,14 @@ // Default way to get an element's href. May be overridden at $.rails.href. href: function(element) { - return element.attr('href'); + return element[0].href; }, // Submits "remote" forms and links with ajax handleRemote: function(element) { - var method, url, data, elCrossDomain, crossDomain, withCredentials, dataType, options; + var method, url, data, withCredentials, dataType, options; if (rails.fire(element, 'ajax:before')) { - elCrossDomain = element.data('cross-domain'); - crossDomain = elCrossDomain === undefined ? null : elCrossDomain; withCredentials = element.data('with-credentials') || null; dataType = element.data('type') || ($.ajaxSettings && $.ajaxSettings.dataType); @@ -147,7 +145,7 @@ error: function(xhr, status, error) { element.trigger('ajax:error', [xhr, status, error]); }, - crossDomain: crossDomain + crossDomain: rails.isCrossDomain(url) }; // There is no withCredentials for IE6-8 when @@ -167,6 +165,27 @@ } }, + // Determines if the request is a cross domain request. + isCrossDomain: function(url) { + var originAnchor = document.createElement("a"); + originAnchor.href = location.href; + var urlAnchor = document.createElement("a"); + + try { + urlAnchor.href = url; + // This is a workaround to a IE bug. + urlAnchor.href = urlAnchor.href; + + // Make sure that the browser parses the URL and that the protocols and hosts match. + return !urlAnchor.protocol || !urlAnchor.host || + (originAnchor.protocol + "//" + originAnchor.host !== + urlAnchor.protocol + "//" + urlAnchor.host); + } catch (e) { + // If there is an error parsing the URL, assume it is crossDomain. + return true; + } + }, + // Handles "data-method" on links such as: // Delete handleMethod: function(link) { @@ -178,7 +197,7 @@ form = $('
'), metadataInput = ''; - if (csrfParam !== undefined && csrfToken !== undefined) { + if (csrfParam !== undefined && csrfToken !== undefined && !rails.isCrossDomain(href)) { metadataInput += ''; } diff --git a/test/public/test/call-remote-callbacks.js b/test/public/test/call-remote-callbacks.js index c1791f6..ad306a3 100644 --- a/test/public/test/call-remote-callbacks.js +++ b/test/public/test/call-remote-callbacks.js @@ -64,20 +64,6 @@ asyncTest('modifying data("type") with "ajax:before" requests new dataType in re }); }); -asyncTest('setting data("cross-domain",true) with "ajax:before" uses new setting in request', 2, function(){ - $('form[data-remote]').data('cross-domain',false) - .bind('ajax:before', function() { - var form = $(this); - form.data('cross-domain',true); - }); - - submit(function(form) { - form.bind('ajax:beforeSend', function(e, xhr, settings) { - equal(settings.crossDomain, true, 'setting modified in ajax:before should have forced cross-domain request'); - }); - }); -}); - asyncTest('setting data("with-credentials",true) with "ajax:before" uses new setting in request', 2, function(){ $('form[data-remote]').data('with-credentials',false) .bind('ajax:before', function() { diff --git a/test/public/test/call-remote.js b/test/public/test/call-remote.js index d78ce56..94316e8 100644 --- a/test/public/test/call-remote.js +++ b/test/public/test/call-remote.js @@ -122,22 +122,6 @@ asyncTest('sends CSRF token in custom header', 1, function() { }); }); -asyncTest('does not send CSRF token in custom header if crossDomain', 1, function() { - buildForm({ 'data-cross-domain': 'true' }); - $('#qunit-fixture').append(''); - - // Manually set request header to be XHR, since setting crossDomain: true in .ajax() - // causes jQuery to skip setting the request header, to prevent our test/server.rb from - // raising an an error (when request.xhr? is false). - $('#qunit-fixture').find('form').bind('ajax:beforeSend', function(e, xhr) { - xhr.setRequestHeader('X-Requested-With', "XMLHttpRequest"); - }); - - submit(function(e, data, status, xhr) { - equal(data.HTTP_X_CSRF_TOKEN, undefined, 'X-CSRF-Token header should NOT be sent'); - }); -}); - asyncTest('intelligently guesses crossDomain behavior when target URL is a different domain', 1, function(e, xhr) { // Don't set data-cross-domain here, just set action to be a different domain than localhost @@ -156,20 +140,4 @@ asyncTest('intelligently guesses crossDomain behavior when target URL is a diffe setTimeout(function() { start(); }, 13); }); - -asyncTest('does not set crossDomain if explicitly set to false on element', 1, function() { - buildForm({ action: 'http://www.alfajango.com', 'data-cross-domain': false }); - $('#qunit-fixture').append(''); - - $('#qunit-fixture').find('form') - .bind('ajax:beforeSend', function(e, xhr, settings) { - equal(settings.crossDomain, false, 'crossDomain should be set to false'); - // prevent request from actually getting sent off-domain - return false; - }) - .trigger('submit'); - - setTimeout(function() { start(); }, 13); -}); - })(); diff --git a/test/public/test/data-method.js b/test/public/test/data-method.js index c442662..5752837 100644 --- a/test/public/test/data-method.js +++ b/test/public/test/data-method.js @@ -46,4 +46,30 @@ asyncTest('link "target" should be carried over to generated form', 1, function( }); }); +asyncTest('link with "data-method" and cross origin', 1, function() { + var data = {}; + + $('#qunit-fixture') + .append('') + .append(''); + + $(document).on('submit', 'form', function(e) { + $(e.currentTarget).serializeArray().map(function(item) { + data[item.name] = item.value; + }); + + return false; + }); + + var link = $('#qunit-fixture').find('a'); + + link.attr('href', 'http://www.alfajango.com'); + + link.trigger('click'); + + start(); + + notEqual(data.authenticity_token, 'cf50faa3fe97702ca1ae'); +}); + })(); diff --git a/test/public/test/override.js b/test/public/test/override.js index ba84b6d..0dca60c 100644 --- a/test/public/test/override.js +++ b/test/public/test/override.js @@ -32,7 +32,7 @@ asyncTest("the getter for an element's href is overridable", 1, function() { asyncTest("the getter for an element's href works normally if not overridden", 1, function() { $.rails.ajax = function(options) { - equal('/real/href', options.url); + equal(location.protocol + '//' + location.host + '/real/href', options.url); } $.rails.handleRemote($('#qunit-fixture').find('a')); start(); -- 2.3.1