Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP
Browse files

strip authority to avoid exploits in parse regex

As explained by @mala in Issue #4787, most browsers simply strip the
authority from `location.href` anyway. We can simply mimick this more
secure behavior for the browsers that don't thereby avoiding the
decoding xss.
  • Loading branch information...
commit 352f1964b685614d27a3e4ef669093778368f915 1 parent ddb62b4
@johnbender johnbender authored
View
24 js/jquery.mobile.navigation.js
@@ -49,27 +49,13 @@ define( [
//
urlParseRE: /^(((([^:\/#\?]+:)?(?:(\/\/)((?:(([^:@\/#\?]+)(?:\:([^:@\/#\?]+))?)@)?(([^:\/#\?\]\[]+|\[[^\/\]@#?]+\])(?:\:([0-9]+))?))?)?)?((\/?(?:[^\/\?#]+\/+)*)([^\?#]*)))?(\?[^#]+)?)(#.*)?/,
- // Abstraction to address xss (Issue #4787) in browsers that auto decode the username:pass
- // portion of location.href. All references to location.href should be replaced with a call
- // to this method so that it can be dealt with properly here
+ // Abstraction to address xss (Issue #4787) by removing the authority in
+ // browsers that auto decode it. All references to location.href should be
@staabm
staabm added a note

Small whitespace error between "auto" and "decode"

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
+ // replaced with a call to this method so that it can be dealt with properly here
getLocation: function( url ) {
- var uri = this.parseUrl( url || location.href ),
- encodedUserPass = "";
+ var uri = url ? $.mobile.path.parseUrl( url ) : location;
- if( uri.username ){
- encodedUserPass = encodeURI( uri.username );
- }
-
- if( uri.password ){
- encodedUserPass = encodedUserPass + ":" + encodeURI( uri.password );
- }
-
- if( encodedUserPass ){
- return uri.protocol + "//" + encodedUserPass + "@" +
- uri.host + uri.pathname + uri.search + uri.hash;
- }
-
- return uri.href;
+ return uri.protocol + "//" + uri.host + uri.pathname + uri.search + uri.hash;
},
parseLocation: function() {
View
10 tests/unit/navigation/navigation_helpers.js
@@ -241,14 +241,12 @@
test( "path.getLocation works properly", function() {
equal( $.mobile.path.getLocation("http://example.com/"), "http://example.com/" );
- equal( $.mobile.path.getLocation("http://foo@example.com"), "http://foo@example.com" );
- equal( $.mobile.path.getLocation("http://foo:bar@example.com"), "http://foo:bar@example.com" );
- equal( $.mobile.path.getLocation("http://<foo<:bar@example.com"), "http://%3Cfoo%3C:bar@example.com" );
- equal( $.mobile.path.getLocation("http://foo:<bar<@example.com"), "http://foo:%3Cbar%3C@example.com" );
- equal( $.mobile.path.getLocation("http://<foo<:<bar<@example.com"), "http://%3Cfoo%3C:%3Cbar%3C@example.com" );
+ equal( $.mobile.path.getLocation("http://foo@example.com"), "http://example.com" );
@jblas
jblas added a note

We might need to add a test that contains a slash or @ sign in it and see how that throws off the parsing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
+ equal( $.mobile.path.getLocation("http://foo:bar@example.com"), "http://example.com" );
+ equal( $.mobile.path.getLocation("http://<foo<:bar@example.com"), "http://example.com" );
var allUriParts = "http://jblas:password@mycompany.com:8080/mail/inbox?msg=1234&type=unread#msg-content";
- equal( $.mobile.path.getLocation( allUriParts ), allUriParts );
+ equal( $.mobile.path.getLocation( allUriParts ), allUriParts.replace( "jblas:password@", "") );
});
})(jQuery);
@staabm

Small whitespace error between "auto" and "decode"

@jblas

We might need to add a test that contains a slash or @ sign in it and see how that throws off the parsing.

Please sign in to comment.
Something went wrong with that request. Please try again.