ESAPI for XSS prevention not working

Question

I am working on fixing Cross site scripting issues in our code mainly in JSPS.

Below is the original code

 //scriplet code
    <% String userId = request.getParameter("sid"); 
    ...%>

and in the same Jsp they have

     <input type = hidden name = "userID" value = "<%= userId %>" />

I have made changes to include esapi-2.1.0.jar in lib and ESAPI.properties, validation.properties in classpath. Then made below changes to scriplet code to fix the above code

      //scriplet code
    <% String userId = ESAPI.encoder().encodeForHTML(request.getParameter("sid")); 
    ...%>

I thought this would fix the issue but when I scan my code using Fortify, these lines are again highlighted as having XSS issue. Please help if you guys have any idea on how this should be handled. Thanks.

------- UPDATE

Thanks a lot @avgvstvs. This is very insightful.Follwd guidelines, Not sure if I am missng somethn. Code -

          String              userSID=ESAPI.encoder().encodeForHTMLAttribute(request.getHeader("janus_sid")); session.setAttribute("username",userSID);<input type=hidden name="USERNAME" value="<%= userSID %>"

And for another varibale debug, below is the usage

       String debugFlag =  ESAPI.encoder().encodeForJavaScript(request.getParameter("debug"));var debugFlag = "<%= debugFlag%>";if(debugFlag == "y"){       
        document.title=   title + " (" + host + ")";
        defaultAppTitle = title + " (" + host +  ")";           
    }                                                           

Latest Fortify scan still lists them as vulnerabilities :-(

Solution 1

Thanks for help guys. Finally figured out a solution to prevent XSS issue and pass Fortify static code analysis. I have used ESAPI together with Anitsamy library. Here are the 3 main changes required.

1. Implement Anitsamy Filter

   Add a new filter and override request methods getParameter , getParameterValues to strip out any suspicious tags in the request. Filter loads a policy file where we define our rules like

   a. tags which needs to be removed from the requests ( tags like , etc)

   b. Regexs for common attributes like href, align etc.

Example for implementation of filter is here http://barrypitman.com/2011/04/14/using-input-validation-XSS/

2. Perform input validation using ESAPI library

    String reportName = request.getParameter("reportName");
    ESAPI.validator().getValidInput("Report Name", 
                                     reportName, "ReportNamePattern", 100, false);
   

   In above code,

   1. "Report Name" is the context
   2. reportName is the data field
   3. ReportNamePattern is the regex pattern defined in ESAPI.properties as Validator.ReportNamePattern =^[a-zA-Z]{1}[0-9]{6}$
   4. 100 is max length for data field reportName
   5. false is a flag to say null value is not allowed.

3. Perform output encoding
   As pointed by @avgvstvs, output encoding is also a must.

   If reportName field is to be used in HTML, below is how to encode

   <tr> <th> Report :     <%=ESAPI.encoder().encodeForHTML(reportName)%> </th> </tr>
   

   If reportName field is to be used in javascript code , below is how to encode

    var reportName = "<%= ESAPI.encoder().encodeForJavaScript(reportName)%>";
   

   If reportName field is to be used in HTML Attribute, below is how to encode

   <input type=hidden name="USERNAME" value="<%=ESAPI.encoder().encodeForHTMLAttribute
       (reportName)%>"/>       
   

Solution 2

The first question you need to ask should be "What code interpreters am I handing this value off to?"

Preventing XSS unfortunately isn't a recipe-based task, and from the looks of it--your application is using scriptlets, which aside from being bad Java practice, makes it much more difficult for static code-scanning tools like Fortify to give you accurate results. JSPs are compiled at runtime, but Fortify scans the source only. You should note that there should be future tasks/stories filed to refactor scriptlets to JSTL--you'll thank me for that later. THEN you can use esapi taglibs for these use cases. Fortify does an okay job of scanning pure Java code, and taglibs give you that.

1. ESAPI.encoder().encodeForHTML(userId) is only going to protect you from XSS in the use case where the variable in question is going to be placed between tags, like <span><%= userId %></span> This isn't the case you have.

2. ESAPI.encoder().encodeForHTMLAttribute(userId) is what you want in your particular, narrow use-case. This is because escaping rules are different in Html attributes than they are for data placed within tags. This should fix your immediate problem.

3. If the value is going to be used exclusively by JavaScript, then you want ESAPI.encoder().encodeForJavaScript(userId)

4. If the value is going to be renderable HTML being sent to a javascript function that will render your markup, like element.innerHTML = “<HTML> Tags and markup”; you want <%=Encoder.encodeForJS(Encoder.encodeForHTML(userId))%>

This is just a few examples, here are a few more--but the overriding gist of my answer is this: You need to know the complete flow of data for every variable in your application and always encode for the appropriate output context. And in the security world, "context" means "Data is being handed off to a new code interpreter." If you implement this understanding well, you won't need Fortify anymore! :-)

Added complexity

In your comment you noted that the value is later being used by JavaScript. So the correct solution in this case would likely to be to fork two different variables, each one set for the proper encoding. For the HTML Attribute case:

String escapedHtmlUserId = ESAPI.encoder().encodeForHTMLAttribute(userId);

For the Javascript case:

String escapedJSUserId = ESAPI.encoder().encodeForJavaScript(userId);

Then use the values appropriately. If you used JSTL/taglibs, you could just use the right esapi taglib in the right place instead of splitting into two separate scriptlet variables.

AND ONE MORE THING

From the comment on the OP:

We have an initial scriptlet declaration:

<% String userId = ESAPI.encoder().encodeForHTML(request.getParameter("sid")); 
...%>

and it is used later in a javascript function:

<%= ESAPI.encoder().encodeForJavascripl(userId)%>`

Just to point out, as presented, the userId variable stops being raw text on initialization. Effectively, the encoding for javascript becomes this:

<% ESAPI.encoder().encodeForJavascript(ESAPI.encoder().encodeForHTML(request.getParameter("sid"))); 
...%>

If the javascript is going to be rendering HTML using *.innerHTML() or *.outerHTML() this is fine. Otherwise, just note that the input has changed from its raw state.

Also, watch for the fact some JavaScript can undo what you've done and ensure your JavaScript isn't doing something similar.

==========More added code, more questions========

So we've traced all our data paths, we've double-checked that our JSP in question isn't being included in a different JSP that introduces new contexts for us to defend against, it's at this point where I'd smell "false positive," but if I were you I'd contact HP support and raise the issue for a false positive. I don't know enough of the specifics of your application to really be of much more use to you here unless I had access to the full source. Because you're dealing with scriptlets--its possible that Fortify can't trace the full data path from variable instantiation to eventual output, so is failing fast so it can at least warn you as to what its found "so far."

Comments

[Eric]

Is it giving you XSS Poor Validation as a medium risk or a critical?

[Pro]

Its a medium risk and we are planning to get rid of medium risks.

[Pro]

@Eric I have just followed the cheat sheet owasp.org/index.php/… but Fortify still complains :-(

[Eric]

Yeah, and i'm not sure why it's not fully supporting that encoding. You can also try using getValidHTML from the Validator class. I'm still looking into why that encoding is not trusted, so I'll report back when I have something.

[avgvstvs]

You need to share the lines of code where userId is being utilized. Some scanning tools are going to target the initial assignment instead of where the risk actually resides. Also, if the same value is going to be used on javascript on the same page, this could be an XSS issue that a tool like Fortify can't discover because it can't attack the page at runtime. You should use a tool like ZAP or burpsuite to run some XSS fuzz tests on the target field before you mark it as a false positive. Still, for this question, I need more code.

[avgvstvs]

^^^Since Fortify can't know about runtime values it could be flagging it just to let you know that you need to do further testing. Also include any information about any javascript that utlizes the value.

[Pro]

Thanks @avgvstvs. There is also a javascript code using this value

[avgvstvs]

I updated my answer to include this new piece of information.

[Pro]

Thanks @avgvstvs. There is also a javascript code using this value var userId = "<%= ESAPI.encoder().encodeForJavascripl(userId)%>"; Apart from that its not used anywhere else

[avgvstvs]

I updated my answer once more to discuss possible issues to consider given the most recent comment.

[Pro]

Thanks a lot @avgvstvs. This is quite useful. I think am doing right in preventing XSS issue but not sure why Fortify still keeps complaining. I have edited my question to post more code. Do you still see any issue with it?

[avgvstvs]

Updated answer again, but you're now at the limit of what the internet can reasonably help with without full access to the source.

[avgvstvs]

Wow, great answer! You managed to capture the three priorities for fighting XSS in Java. Duly upvoted!

[Pro]

Thanks @avgvstvs. Credit to you for pointing me in right direction :-)

[Venkaiah Yepuri]

@Pro : I Pro, I have gone through yours solution & up voted too its really helpful but in my case I am directly doing attribute encding at element level in my jsp like <input id="rlv" type="text" name="releaseVersion" maxlength="250" size="25" value="<%=$ESAPI.encoder().encodeForHTMLAttribute(request.ge‌​‌​tAttribute("relV")‌​)%‌​>" style="font-size:12px;font-family:arial,helvetica,sans-serif‌​‌​;" onkeyup = "releaseVersionSearch();" onblink="releaseVersionSearch();" class="modulecontent" /> when I am doing like this my page is not loading on browser

[Venkaiah Yepuri]

And in browser console tab I able to see this error net::ERR_INCOMPLETE_CHUNKED_ENCODING can please help me what I am doing mistake here ? Ty.

[Venkaiah Yepuri]

Hi, I have impressed a lot with yours simple & understandable explanations & I up voted yours answer too. Can you please help me out about these two usages too where & how we can use too plz. 1. encodeForURL 2. encodeForBase64 ?

[avgvstvs]

encodeForURL is when a value will be used as a parameter in a URL, or when constructing URLs dynamically. encodeForBase64 is a bit more specific, basically you'll see that when sending encrypted data that is then sent across the wire as base64. You'll know it when you need it.

[Sam]

Hi, I have scenario where i have to encrypt or protect integer. Example //scriplet code <% int userId = request.getParameter("sid"); ...%> <input type = hidden name = "userID" value = "<%= userId %>" /> Any idea how can i encrypt integer or can i raise false positive in above code. Please help as i have stucked up very badly.