[Owasp-csrfguard] Potential Vulnerability in isValidUrl

Eric Sheridan eric.sheridan at owasp.org
Mon Feb 14 15:30:04 EST 2011

I've implemented a fix to address this issue. I also modified how
CSRFGuard is installed within a webapp. Instead of just installing a
filter, you now have to install an HttpSessionListener as well. This
session listener is responsible for constructing the CSRFGuard context
whenever a new session is created. This helps address one or more state
related bugs when the developer creates/invalidates sessions and yet
uses API requiring existence of the CSRFGuard context.


Let me know what you think!


On 2/12/11 9:49 AM, Eric Sheridan wrote:
> Botched testing on my part - the browser assumes the current protocol. I
> ran this locally and it assumed file. If I hosted the doc, it would
> become http(s). Botched test on my part. I'll be pushing a fix shortly.
> -Eric
> On 2/11/11 10:12 AM, Eric Sheridan wrote:
>> Just an update: I'm having trouble reproducing this in every browser
>> that I've tried - always resolves to file://attacker.com which shouldn't
>> have the token but is still local resource. So this might just be a
>> functional bug and not a security bug.
>> -Eric
>> On 2/11/11 9:35 AM, Eric Sheridan wrote:
>>> List,
>>> Koto on GitHub pointed out a potential vulnerability in the "isValidUrl"
>>> method of the dynamic JavaScript code - and I believe this person is
>>> correct. This method is poorly written and this problem only highlights
>>> that fact. Here is the snippet:
>>> /** determine if uri/url points to valid domain * */
>>> function isValidUrl(src) {
>>> 	var result = false;
>>> 	/** parse out domain to make sure it points to our own * */
>>> 	if(src.substring(0, 7) == "http://" || src.substring(0, 8) == "https://") {
>>> 		// check if is valid domain
>>> 	} else if(src.charAt(0) == '#') {
>>> 		...
>>> 	} else if(src.charAt(0) == '/' || src.indexOf(':') == -1) {
>>> 		result = true;
>>> 	}
>>> 	return result;
>>> }
>>> The idea behind this code is to determine if the form/url location
>>> points to a page for which we must include the CSRF token. This helps
>>> ensure that a token destined for abc.com is not sent off site to an
>>> xyz.com. However, the following URL //attacker.com/whatever is not local
>>> yet will translate to http://attacker.com/whatever and the token will be
>>> included. If the user clicks the link, their CSRF token destined for
>>> abc.com is sent to attacker.com.
>>> Any thoughts on how I could do this uri/url parsing logic in a cleaner
>>> fashion without introducing a third party library? I could sneak in a
>>> third conditional (&& !src.startsWith("//")) but this seems really fragile.
>>> -Eric

More information about the Owasp-csrfguard mailing list