Permalink
Browse files

Ajax: Mitigate possible XSS vulnerability

Proposed by @jaubourg

Cherry-picked from b078a62
Fixes gh-2432
Closes gh-2588
  • Loading branch information...
markelog committed Sep 10, 2015
1 parent 5da5035 commit f60729f3903d17917dc351f3ac87794de379b0cc
Showing with 55 additions and 0 deletions.
  1. +7 −0 src/ajax/script.js
  2. +48 −0 test/unit/ajax.js
View
@@ -4,6 +4,13 @@ define( [
"../ajax"
], function( jQuery, document ) {
+// Prevent auto-execution of scripts when no explicit dataType was provided (See gh-2432)
+jQuery.ajaxPrefilter( function( s ) {
+ if ( s.crossDomain ) {
+ s.contents.script = false;
+ }

This comment has been minimized.

Show comment
Hide comment
@arthurvr

arthurvr Oct 12, 2015

Member

The indentation here seems wrong (it's 4 spaces)

@arthurvr

arthurvr Oct 12, 2015

Member

The indentation here seems wrong (it's 4 spaces)

This comment has been minimized.

Show comment
Hide comment
@arthurvr

arthurvr Oct 12, 2015

Member

How is that related? I'm talking about lines 9, 10 and 11, not the whole file. It is indented too, just using 4 spaces not using tabs.

@arthurvr

arthurvr Oct 12, 2015

Member

How is that related? I'm talking about lines 9, 10 and 11, not the whole file. It is indented too, just using 4 spaces not using tabs.

This comment has been minimized.

Show comment
Hide comment
@timmywil

timmywil Oct 12, 2015

Member

Nice catch. Probably a copypasta from github.

@timmywil

timmywil Oct 12, 2015

Member

Nice catch. Probably a copypasta from github.

This comment has been minimized.

Show comment
Hide comment
@markelog

markelog Oct 12, 2015

Member

Nuh, i just constantly switching between projects and sometimes forget which one use which style :/

@markelog

markelog Oct 12, 2015

Member

Nuh, i just constantly switching between projects and sometimes forget which one use which style :/

This comment has been minimized.

Show comment
Hide comment
@arthurvr

arthurvr Oct 12, 2015

Member

That's why we use EditorConfig :)

@arthurvr

arthurvr Oct 12, 2015

Member

That's why we use EditorConfig :)

This comment has been minimized.

Show comment
Hide comment
@markelog

markelog Oct 12, 2015

Member

Yeah, i have this plugin, but for some reason, it sometimes fails :/

@markelog

markelog Oct 12, 2015

Member

Yeah, i have this plugin, but for some reason, it sometimes fails :/

+} );
+
// Install script dataType
jQuery.ajaxSetup( {
accepts: {
View
@@ -85,6 +85,54 @@ QUnit.module( "ajax", {
};
} );
+ ajaxTest( "jQuery.ajax() - do not execute js (crossOrigin)", 2, function( assert ) {
+ return {
+ create: function( options ) {
+ options.crossDomain = true;
+ return jQuery.ajax( url( "data/script.php?header=ecma" ), options );
+ },
+ success: function() {
+ assert.ok( true, "success" );
+ },
+ complete: function() {
+ assert.ok( true, "complete" );
+ }
+ };
+ } );
+
+ ajaxTest( "jQuery.ajax() - execute js for crossOrigin when dataType option is provided", 3,
+ function( assert ) {
+ return {
+ create: function( options ) {
+ options.crossDomain = true;
+ options.dataType = "script";
+ return jQuery.ajax( url( "data/script.php?header=ecma" ), options );
+ },
+ success: function() {
+ assert.ok( true, "success" );
+ },
+ complete: function() {
+ assert.ok( true, "complete" );
+ }
+ };
+ }
+ );
+
+ ajaxTest( "jQuery.ajax() - do not execute js (crossOrigin)", 2, function( assert ) {
+ return {
+ create: function( options ) {
+ options.crossDomain = true;
+ return jQuery.ajax( url( "data/script.php" ), options );
+ },
+ success: function() {
+ assert.ok( true, "success" );
+ },
+ complete: function() {
+ assert.ok( true, "complete" );
+ }
+ };
+ } );
+
ajaxTest( "jQuery.ajax() - success callbacks (late binding)", 8, function( assert ) {
return {
setup: addGlobalEvents( "ajaxStart ajaxStop ajaxSend ajaxComplete ajaxSuccess", assert ),

9 comments on commit f60729f

@jorgsmash

This comment has been minimized.

Show comment
Hide comment
@jorgsmash

jorgsmash May 8, 2018

I have a question regarding this mitigation. I am a vulnerability analyst and am trying to fully understand how this can be properly mitigated. Is this code provided here a recommended fix that someone would have to manually add to their instance of jQuery? Or is this implemented already if the "ajaxPrefilter" is present? For example, a code snippet from example.com/jquery-1.12.3.min.js, doing a Ctrl+f for "ajaxPrefilter" shows this:

...ajaxPrefilter:Tb(Ob),ajaxTransport:Tb(Pb),ajax:function(b,c)...

....n.ajaxPrefilter("script",function(a){void 0===a.cache&&(a.cache=!1),a.crossDomain&&(a.type="GET",a.global=!1)}),n.ajaxTransport("script",function(a){if(a.crossDomain){var b,c=d.head||n("head")[0]||d.documentElement;return{send:function(e,f)......

.....n.ajaxPrefilter("json jsonp",function(b,c,d){var e,f,g,h=b.jsonp!......

Is this how one would determine if this mitigation has been implemented? Or is this an addition to the ajaxPrefilter function that someone would need to manually add to their running instance of jQuery. Thanks all!

I have a question regarding this mitigation. I am a vulnerability analyst and am trying to fully understand how this can be properly mitigated. Is this code provided here a recommended fix that someone would have to manually add to their instance of jQuery? Or is this implemented already if the "ajaxPrefilter" is present? For example, a code snippet from example.com/jquery-1.12.3.min.js, doing a Ctrl+f for "ajaxPrefilter" shows this:

...ajaxPrefilter:Tb(Ob),ajaxTransport:Tb(Pb),ajax:function(b,c)...

....n.ajaxPrefilter("script",function(a){void 0===a.cache&&(a.cache=!1),a.crossDomain&&(a.type="GET",a.global=!1)}),n.ajaxTransport("script",function(a){if(a.crossDomain){var b,c=d.head||n("head")[0]||d.documentElement;return{send:function(e,f)......

.....n.ajaxPrefilter("json jsonp",function(b,c,d){var e,f,g,h=b.jsonp!......

Is this how one would determine if this mitigation has been implemented? Or is this an addition to the ajaxPrefilter function that someone would need to manually add to their running instance of jQuery. Thanks all!

@dmethvin

This comment has been minimized.

Show comment
Hide comment
@dmethvin

dmethvin May 8, 2018

Member

Why examine the minified version of the code when the unminified is available? If you look at the CDN copies of the unminified source you can see whether this code is present. Search for "Prevent auto-execution".
http://code.jquery.com/jquery-3.3.1.js

If you have the commit hash (it's above) you can use the command git describe --contains f60729f3903d17917dc351f3ac87794de379b0cc to find the first release where the commit appeared.

Member

dmethvin replied May 8, 2018

Why examine the minified version of the code when the unminified is available? If you look at the CDN copies of the unminified source you can see whether this code is present. Search for "Prevent auto-execution".
http://code.jquery.com/jquery-3.3.1.js

If you have the commit hash (it's above) you can use the command git describe --contains f60729f3903d17917dc351f3ac87794de379b0cc to find the first release where the commit appeared.

@jorgsmash

This comment has been minimized.

Show comment
Hide comment
@jorgsmash

jorgsmash May 8, 2018

The path including "jquery-1.12.3.min.js" was detected in a vulnerability scan. So I was just taking the path that the scanner provided. The scanned website has source code that links to this version. I am still trying to figure this vulnerability out and how it is implemented on people's websites. When you say to look at the unminified version, is that specific to a particular website that implements jQuery, or is it used globally, as in all 1.12.3 versions are the same and not modified/customized when used on someone's website?

The path including "jquery-1.12.3.min.js" was detected in a vulnerability scan. So I was just taking the path that the scanner provided. The scanned website has source code that links to this version. I am still trying to figure this vulnerability out and how it is implemented on people's websites. When you say to look at the unminified version, is that specific to a particular website that implements jQuery, or is it used globally, as in all 1.12.3 versions are the same and not modified/customized when used on someone's website?

@dmethvin

This comment has been minimized.

Show comment
Hide comment
@dmethvin

dmethvin May 8, 2018

Member

Since I'm not being paid to do this research, I'll try to restrict my comments to the volunteer work that I do on the project. The output of git describe --contains f60729f3903d17917dc351f3ac87794de379b0cc gives 1.12.0~111 which says that version 1.12.0 was the first containing this particular commit. That does not indicate whether the web site uses the API in a way that would even invoke this particular code. If you need more assistance in your work, I'm available for consulting and you can find my contact info via Google.

Member

dmethvin replied May 8, 2018

Since I'm not being paid to do this research, I'll try to restrict my comments to the volunteer work that I do on the project. The output of git describe --contains f60729f3903d17917dc351f3ac87794de379b0cc gives 1.12.0~111 which says that version 1.12.0 was the first containing this particular commit. That does not indicate whether the web site uses the API in a way that would even invoke this particular code. If you need more assistance in your work, I'm available for consulting and you can find my contact info via Google.

@msmolka

This comment has been minimized.

Show comment
Hide comment
@msmolka

msmolka Jun 6, 2018

@jorgsmash the fix is not applied to 1.12.3 and 1.12.4
see #3011

@jorgsmash the fix is not applied to 1.12.3 and 1.12.4
see #3011

@lucasatace

This comment has been minimized.

Show comment
Hide comment
@lucasatace

lucasatace Jul 3, 2018

To implement this, do I need to have the offending codebase of jQuery local and customize it from there, or can these files be something that's separate and called in via the HTML?

To implement this, do I need to have the offending codebase of jQuery local and customize it from there, or can these files be something that's separate and called in via the HTML?

@msmolka

This comment has been minimized.

Show comment
Hide comment
@msmolka

msmolka Jul 3, 2018

@lucasatace just use JQuery 1.12.2, unless you need features from 12.3 or 12.4

@lucasatace just use JQuery 1.12.2, unless you need features from 12.3 or 12.4

@mgol

This comment has been minimized.

Show comment
Hide comment
@mgol

mgol Jul 3, 2018

Member

@lucasatace You can also invoke the code from the patch:
b078a62#diff-bee4304906ea68bebadfc11be4368419
manually just after loading jQuery:

// Prevent auto-execution of scripts when no explicit dataType was provided (See gh-2432)
jQuery.ajaxPrefilter( function( s ) {
    if ( s.crossDomain ) {
        s.contents.script = false;
    }
} );
Member

mgol replied Jul 3, 2018

@lucasatace You can also invoke the code from the patch:
b078a62#diff-bee4304906ea68bebadfc11be4368419
manually just after loading jQuery:

// Prevent auto-execution of scripts when no explicit dataType was provided (See gh-2432)
jQuery.ajaxPrefilter( function( s ) {
    if ( s.crossDomain ) {
        s.contents.script = false;
    }
} );
@lucasatace

This comment has been minimized.

Show comment
Hide comment
@lucasatace

lucasatace Jul 3, 2018

@msmolka and @mgol

Thanks!!!! Only reason I'm looking is because we failed our PCI scan, so we need something that will get us back compliant.

@msmolka and @mgol

Thanks!!!! Only reason I'm looking is because we failed our PCI scan, so we need something that will get us back compliant.

Please sign in to comment.