How to fix the HTTP Response splitting vulnerability with ESAPI

Question

after a recent findbugs (FB) run it complains about a: Security - HTTP Response splitting vulnerability The following code triggers it:

String referrer = req.getParameter("referrer");
 if (referrer != null) {
  launchURL += "&referrer="+(referrer);
 }
resp.sendRedirect(launchURL);

Basically the 'referrer' http parameter contains an url, to which, when clicking on a back button in our application the browser returns to. It is appended to the url as a parameter. After a bit research i know that i need to sanitize the referrer url. After a bit more research i found the esapi project which seem to offer this kind of functionality:

//1st canonicalize
import org.owasp.esapi.Encoder;
import org.owasp.esapi.Validator;
import org.owasp.esapi.reference.DefaultEncoder;
import org.owasp.esapi.reference.DefaultValidator;
[...]
Encoder encoder = new DefaultEncoder(new ArrayList<String>());
String cReferrer = encoder.canonicalize(referrer);

However I didn't figure out how to detect e.g. jscript code or other stuff which doesn't belong to a referrer url. So how can I achieve that with esapi?

I tried:

Validator validator = new DefaultValidator(encoder);
validator.isValidInput("Redirect URL",referrer,"HTTPParameterValue",512,false);

however this doesn't work. What I need is a function which results in:

http://www.google.com (ok)

http://www.google.com/login?dest=http://google.com/%0D%0ALocation: javascript:%0D%0A%0D%0Aalert(document.cookie) (not ok)

Or is it enough to call the following statement?

encoder.encodeForHTMLAttribute(referrer);

Any help appreciated.

Answer 1

Here's my final solution if anyone is interested. First I canonicalize and then URL decode the string. If a CR or LF exists (\n \r) I just cut of the rest of that potential 'attack' string starting with \n or \r.

String sanitize(String url) throws EncodingException{
    
    Encoder encoder = new DefaultEncoder(new ArrayList<String>());
    //first canonicalize
    String clean = encoder.canonicalize(url).trim();
    //then url decode 
    clean = encoder.decodeFromURL(clean);
    
    //detect and remove any existent \r\n == %0D%0A == CRLF to prevent HTTP Response Splitting
    int idxR = clean.indexOf('\r');
    int idxN = clean.indexOf('\n');
    
    if(idxN >= 0 || idxR>=0){
        if(idxN<idxR){
            //just cut off the part after the LF
            clean = clean.substring(0,idxN);
        }
        else{
            //just cut off the part after the CR
            clean = clean.substring(0,idxR);
        }
    }
    
    //re-encode again
    return encoder.encodeForURL(clean);
}

Theoretically i could have later verified the value against 'HTTPParameterValue' regex which is defined in the ESAPI.properties however it didn't like colon in the http:// and I didn't investigated further.

And one more remark after testing it: Most modern browser nowadays (Firefox > 3.6, Chrome, IE10 etc.) detect this kind of vulnerability and do not execute the code...

Answer 2

I think you have the right idea, but are using an inappropriate encoder. The Referer [sic] header value is really a URL, not an HTML attribute, so you really want to use:

encoder.encodeForURL(referrer);

-kevin

Answer 3

I would suggest white-listing approach wherein you check the referrer string only for permissible characters. Regex would be a good option.

EDIT:

The class org.owasp.esapi.reference.DefaultEncoder being used by you is not really encoding anything. Look at the source code of the method encodeForHTMLAttribute(referrer) here at grepcode. A typical URL encoding (encoding carriage return and line feed) too wont help.

So the way forward would be device some validation logic which checks for valid set of characters. Here is another insightful article.

Answer 4

The accepted answer will not work if in case there is "\n\r" in the string. Example: If I have string: "This is str\n\rstr", it returns "This is str\nstr"

Rectified version of above accepted answer is:

 String sanitizeCarriageReturns(String value) {
        int idxR = value.indexOf('\r');
        int idxN = value.indexOf('\n');

        if (idxN >= 0 || idxR >= 0) {
            if ((idxN > idxR && idxR<0) || (idxR > idxN && idxR>=0)) {
                    value = value.substring(0, idxN);
                } else if (idxN < idxR){
                    value = value.substring(0, idxR);
                } 

        }
        return value;
    }