Obscure XSS using url tag

[Javex]

Hey there,

today a few fellow students (@qll, @Immortalem, myself) took a look at your library and scanned it for potential security issues. We came up pretty much empty handed but were able to find a pretty nasty way to inject javascript through your parser, though not all browsers are vulnerable to this (most notably IE 10 and Chrome 40).

The exact details of this XSS include sending, e.g. a \x01 byte before the javascript: part so it bypasses your filter but still executes as valid JavaScript:

<a href="\x01javascript:alert(1)">Test</a>

We developed this case (and found other possible cases) using Shazzer (see 1 and 2).

While this seems like a weird edge case, there actually lies a real vulnerability as any user could generate this and run JS in the context of another user. And Chrome 40 is actually the current recent version.

Your current approach using blacklisting has the unfortunate result that there may always come up some way to bypass it (for example, there exists an ancient vbscript: URI for IE6 or something like that, though these would be very ancient machines indeed...).

However, using a whitelisting approach would involve building a whitelist of allowed tags and that can be infinite (think irc://, steam://, ...).

We would like to contribute to this project by offering a fix. But before blindly sending a pull request, we want to get your input on this.

Method 1: We develop some method that fixes the above bug and still keeps blacklisting intact. While we feel this should be possible in a reasonably secure way, using the blacklisting approach might turn some other, weirder way, to evade it. However, we think such a fix is possible.

Method 2: We implement a whitelisting approach that allows augmenting allowed tags by users of your libary.

I'd personally prefer the first method as the other one limits functionality. However, since this is your library, we want this to be your decision in the end.

Please let me know how you stand on this issue and we can get started on a fix :+1:

[dcwatson]

Thanks for the report. I'd probably keep the blacklist approach, but strip off non-printable characters from the front of the string before checking. I can't think of a case where a (legitimate) URL would start with non-printable characters. Maybe even strip out all non-printable characters? And since this is just in the default url tag implementation, users could always replace the behavior if need be.

[Javex]

From your response I gather that you have a tendency for Method 1. I think this is reasonable and would be happy to work with you to implement a fix. And if I understand your comment correctly, you'd have no problem breaking functionality for users that'd rely und such a weird feature?

We should take care that functionality always is secure-by-default and the user has to take matters in their own hands if they want functionality that is not possible because of security measures implemeneted for that tag.

If you'd like, we can get started on a fix and work something out.

[dcwatson]

Sure, if you want to have a crack at it, go ahead and submit a pull request with a fix and a test case. Or if you don't have time or aren't comfortable with the code, I will fix this next time I'm in the code.

[Javex]

We decided to implement this by keeping the blacklist approach (extending it by vbscript for old IE) and decided to remove all characters that are neither a-z, 0-9 or + (as per Web-based protocol handlers).

The tests check all of this except for a + in the handler (because there is no XSS vector we know of, it's just a valid char we need to keep).

Hope this is what you expected.

[dcwatson]

Looks great, thanks again for the contribution. This has been merged, and I just pushed out version 1.0.21 to PyPI.