-->

ESAPI for XSS prevention not working

2019-02-05 10:00发布

问题:

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 :-(

回答1:

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."



回答2:

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/

  1. 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.
  2. 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)%>"/>