[Owasp-csrfguard] Potential Vulnerability in isValidUrl

Eric Sheridan eric.sheridan at owasp.org
Sat Feb 12 09:49:10 EST 2011


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