[Owasp-csrfguard] Potential Vulnerability in isValidUrl

Fri Feb 11 10:12:38 EST 2011

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