Security fix for window.location.href

Question

I have the below js code

var a = window.location.href.substring(0,window.location.href.lastIndex('/')+1) + "logout.jsp";

setTimeout(function(){
      window.location.href = a;
},1000);

When I am running a fortify scan for the above file, it is showing a security risk on the above line with Dynamic Code Evaluation :Code Injection. Now I am not able to understand on how to fix it. Do I need to add any encoder for window.href or how to resolve this. Also if we have encode , what I need to do.

I can't really see how this can be exploited to inject arbitrary code since `location.href` is generated by your server (unless certain parts of the URL is from arbitrary user input.) However, your code can be reduced to just `location.href = "logout.jsp"` which does the same thing. — Derek 朕會功夫, Sep 08 '17 at 07:38
@Derek朕會功夫 In this case I too can't see how injection can happen. But someone can push JS after `#` in the URL, which might create issues if not handled properly. So probably the tool is reporting false positives every time it sees `window.location.href` being manipulated. — Nisarg Shah, Sep 08 '17 at 07:45
Does your Javascript code reside directly in HTML page or in a separate js file ? — Benjamin Keller, Sep 08 '17 at 07:50
@Derek朕會功夫 I didn't understand how to reduce it to just location.href='logout.jsp' work. Pardon me for my less knowledge. — pallabi das, Sep 08 '17 at 08:12
@NisargShah So what shall I do for it ? Shall I ask to supress? — pallabi das, Sep 08 '17 at 08:13
I don't know well `fortify`, but maybe the issue it's finding is about `var a` defined outside the function in a general scope... try just moving the definition of `a` in the function. Normally you shouldn't need any encoder there — Benjamin Keller, Sep 08 '17 at 08:13
@pallabidas See *Nisarg Shah*'s answer below. You can avoid all the string manipulations by letting the browser handling it. — Derek 朕會功夫, Sep 08 '17 at 08:25
@pallabidas Setting `location.href='logout.jsp'` will tell the browser to replace the page name from something like "abc.jsp" to "logout.asp", and remove the querystring (and hash). Which seems like what your code is doing anyway. — Nisarg Shah, Sep 08 '17 at 08:32

score 1 · Answer 1 · answered Sep 08 '17 at 07:40

If I understand the logic correctly, you are trying to get the path of the url without the page name, and then you intend to redirect to it.

If that is correct, you might be able to get it to work using,

var a = "logout.jsp";

setTimeout(function(){
      window.location.href = a;
},1000);

It should in principal get rid of the vulnerability, but I am not fully sure if the tool detects any other vulnerability in it.

score 0 · Answer 2 · edited Sep 08 '17 at 08:35

I've found the following link... maybe it could help you:

https://security.stackexchange.com/questions/151806/jquery-js-dynamic-code-evaluation-code-injection-on-settimeout-line

It's a false positive.

Reporting false code injection vulnerabilities is a well-known problem with HP Fortify and has confused developers before. Fortify just does basic static analysis of the Javascript code and can't go arbitrarily deep to understand how it works. As @AlexanderOMara suggested, it just seems to discover the potentially dangerous setTimeout() function which can, as setInterval(), take a string argument that would be executed as code, just like eval() does. This the sort of vulnerability, the tool aims to discover:
setTimeout('alert(' + document.location.hash.split('#')[1] + ')', 0);
But in your case there is no user-supplied, unfiltered input to the setTimeout() function and it therefore looks safe. Leaving you with a great conclusion from the linked thread:

My advice is to stop running HP fortify reports. Or pay the five thousand, or whatever dollars to go to their classes so you could actually understand their malarkey.

Answered by Arminius.

Security fix for window.location.href

2 Answers2