Skip to content
This repository has been archived by the owner on Oct 8, 2021. It is now read-only.

XSS with location.href behavior of some browsers #4787

Closed
masatokinugawa opened this issue Aug 2, 2012 · 18 comments
Closed

XSS with location.href behavior of some browsers #4787

masatokinugawa opened this issue Aug 2, 2012 · 18 comments
Assignees

Comments

@masatokinugawa
Copy link

This bug differs from Issue #1990. I tested on Safari 5.1.7 for Windows, Safari Mobile(iOS 5.1.1).
The vector is:

http://l0.cm%2F@jquerymobile.com/demos/1.2.0-alpha.1/#//l0.cm/jqm

These browsers percent-decode "user:password@" part of location.href. I think XSS comes from this behavior.
FYI, this behavior is fixed as CVE-2012-3695 in Safari 6. See: http://support.apple.com/kb/HT5400

@mala
Copy link

mala commented Aug 2, 2012

I tested with android emulator, it works on Android 4.0.3 and not works on Android 4.1.
So, it has influence on over 90% of Android users. http://developer.android.com/about/dashboards/index.html

It's a webkit's bug, but I think that it's also jQuery Mobile's bug.
Same-origin checking should be use location.protocol + "//" + location.host

http://trac.webkit.org/changeset/96779
https://bugs.webkit.org/show_bug.cgi?id=30225

Simple workaround code is here

- documentUrl = path.parseUrl( location.href ),
+ documentUrl = path.parseUrl( location.protocol + "//" + location.host + location.pathname + location.hash ),

@ghost ghost assigned johnbender Aug 6, 2012
@johnbender
Copy link
Contributor

@masatokinugawa

Thanks for the heads up. I'll be addressing this today.

@mala

Unfortunately firefox has the exact same issue but with the location.hash and not the location.href, so we'll have to do some twiddling there. Otherwise I'm going to centralize the handling so there's less chance of someone using location.href in the future to avoid this.

@johnbender
Copy link
Contributor

@mala

I'm curious if you can reproduce with window.location.toString(). I'd imagine that the underlying implementation is the same but it would make the fix more simple.

@johnbender
Copy link
Contributor

@masatokinugawa @mala

We're now in a position to fix this but I'd like to use window.location.toString() if possible. I'm working on reproducing the issue so I can test it.

@mala
Copy link

mala commented Aug 7, 2012

@johnbender

In my knowledge, location.toString cannot be overwritten, even if it can do, it will not necessarily work in the future.

I just forgot location.search, location.href without user:pass is
location.protocol + "//" + location.host + location.pathname + location.search + location.hash
it also works on file:// url.

It's not related but important, "isSameDomain" can load http page from https page.
I think isSameOrigin is required for more strict security.

@johnbender
Copy link
Contributor

@mala

In my knowledge, location.toString cannot be overwritten

I'm not sure what you mean. I was curious if window.location.toString() still decoded the user:pass segment. I'm going to try and reproduce tomorrow and if that doesn't work I'll use the string concat.

Thanks for all your help.

@mala
Copy link

mala commented Aug 7, 2012

@johnbender
window.location.toString() contains decoded username and
window.location.toString = myToStringFunction not work in almost all cases.
getLocation: function() { return … }, is OK.

@johnbender
Copy link
Contributor

@mala

Yah I would never overwrite a built in method like the toString on location even if it were possible.

@johnbender
Copy link
Contributor

@mala

Unfortunately the string concat doesn't appear to work on Android 2.3 which means we really have no recourse in addressing this issue for that platform. You said this works for Android 4.0.3 though?

@johnbender
Copy link
Contributor

@mala

I, of course, am a moron. I'll play around with some regex twiddling to see if I can fix it.

@johnbender
Copy link
Contributor

@mala @masatokinugawa

I would appreciate your eyes on this fix to make sure I've fully addressed the issue.

75ba273
4348eac

@johnbender
Copy link
Contributor

All,

Examining the possibility that our url parser might also be gamed by a well crafted authority in this case. More later.

@mala
Copy link

mala commented Aug 9, 2012

@johnbender

Hi,

user:pass in location.href is dependent on browser's implementation.
Non-webkit browsers remove user:pass in location.href by default.

see http://jsrun.it/miya2000/2Lyx

It is not necessary to consider percent encoding.
I think that just remove user:pass from location.href is no problem at all.

johnbender added a commit that referenced this issue Aug 9, 2012
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.
johnbender added a commit that referenced this issue Aug 9, 2012
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.
@johnbender
Copy link
Contributor

@mala

Ah! Sorry to take such a roundabout path to your original solution. I was laboring under the false impression that we should preserve the authority, but if most strip it anyway (which they do in my testing) we can't support that.

Thanks again for all your help.

@ojid49883
Copy link

Please teach it.

Is 1.2.0 RC1/RC2 a modified mistake?

http://www.yahoo.com%2F@jquerymobile.com/demos/1.2.0-beta.1/
getLocation
uri.host = jquerymobile.com
uri.hostname = jquerymobile.com

http://www.yahoo.com%2F@jquerymobile.com/demos/1.2.0-rc.1/
getLocation
uri.host = www.yahoo.com
uri.hostname = www.yahoo.com

http://www.yahoo.com%2F@jquerymobile.com/demos/1.2.0-rc.2/
getLocation
uri.host = www.yahoo.com
uri.hostname = www.yahoo.com

@mala
Copy link

mala commented Sep 24, 2012

@johnbender @ojid49883

ah, enbugged by this commit 51d82b0
location.href returns wrong value, you shouldn't use that.

@johnbender
Copy link
Contributor

Argh.

Sorry two problems confused there. The hash is also autodecoded by firefox, I'll have to get the hash from a parsed form of the url and everything else from the location object directly.

Thanks for keeping an eye on this :(

@johnbender johnbender reopened this Sep 25, 2012
@johnbender
Copy link
Contributor

https://github.com/jquery/jquery-mobile/blob/master/js/jquery.mobile.navigation.js#L55

@mala @ojid49883

If you guys can take a quick look that should address the issue. You'll note that it handles both cases.

By default it uses the location objects values and parses the hash out of location.href to avoid FF's decoding of location.hash.

Thanks

arschmitz pushed a commit to arschmitz/jquery-mobile that referenced this issue Oct 16, 2012
As explained by @mala in Issue jquery-archive#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.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants