Monday, September 13, 2010

Danger of JSP Includes and Parameter Passing

I just recently built a JSP application to demonstrate the capabilities of AppSensor and ESAPI.  I decided to use strictly JSPs to keep the code relatively simple and easy to use as a reference for AppSensor ideas.  During the development of this app I discovered an interesting (and concerning) behavior of JSP include tags.

Consider the following design where JSP includes are used to maximize code reusability. For each lesson the lessonIdentifier and lessonName are defined at the top of the lessonX.jsp file and then those values are passed to "preContent.jsp" which displays a bunch of info in the final jsp that is constructed.
lesson1.jsp

<%@include file="header.jsp"%>
<%
    String lessonIdentifier="1";
    String lessonName="lesson 1 - attack detection";
    String lessonObjective="Do something";   
%>

<jsp:include page="WEB-INF/preContent.jsp">
    <jsp:param name="lessonIdentifier" value="<%=lessonIdentifier%>"/>
    <jsp:param name="lessonName" value="<%=lessonName%>"/>
    <jsp:param name="lessonObjective" value="<%=lessonObjective%>"/>
</jsp:include>

More JSP/html content here.

<%@include file="footer.jsp"%>



WEB-INF/preContet.jsp


<div id="lessonTitle">
<b>Lesson:</b><br/> <%=request.getParameter(lessonName)%><br />
<b>Objective:</b><br/> <%=request.getParameter(lessonObjective)%>
</div>


preContent.jsp is located within the WEB-INF folder and cannot be directly requested by a user. The only way the lesson name could be passed to this jsp is through the jsp:include from lesson1.jsp.  So in the above scenario everything works great and there is no XSS concern.

However, take a look at lesson2.jsp below. Its the same as lesson1.jsp but the author forget to define the lessonName and lessonObjective variables and also did not include them as parameters for the jsp include statement.
lesson2.jsp

<%@include file="header.jsp"%>
<%
    String lessonIdentifier="2";
    //String lessonName="xxx";
    //String lessonObjective="xxx";
%>

<jsp:include page="WEB-INF/preContent.jsp">
    <jsp:param name="lessonIdentifier" value="<%=lessonIdentifier%>"/>
</jsp:include>

More JSP/html content here.

<%@include file="footer.jsp"%>

Now we have a scenario where lesson2.jsp still includes the preContent.jsp from WEB-INF but has forgotten to pass the lessonName and lessonObjective variables.  The code still works and no errors are thrown.  This is a problem because we have now introduced a huge XSS issue.  The preContent.jsp did not receive the parameters during the <jsp:include>; however, instead of displaying a null value or even throwing an error message, the reqeust.getParameter simply fails over and looks for a URL argument.

An attacker with knowledge of the source code could make the following malicious request

http://somesite.com/lesson2.jsp?lessonName=<script>alert('xss fun')</script>

Resulting Processing by the JSP:
1: lesson2.jsp includes WEB-INF/preContent.jsp
2. preContent.jsp was not given the lessonName parameter from the jsp:include and looks to the URL
3. preContent.jsp finds the URL argument and dangerously includes the user controlled data

It should be noted that the same attack on lesson1.jsp is not exploitable since the preContent.jsp finds the lessonName parameter from lesson1.jsp as part of step 2 in the process.


Conclusions

I'm very concerned by the overloaded behavior of parameter passing. In my opinion the call to request parameters from a jsp include should be different from the call to get parameters from GET or POST statements.  With these changes the above scenario would not be vulnerable and would also result in a run time error indicating that the jsp include did not send the expected parameters.  The current "fail-over" type behavior of reqeust.getParameter is not expected and can result in dangerous XSS vulnerabilities as indicated above.


I am interested in feedback from experienced JSP developers. How do you structure your code with JSP includes? Is this a plausible scenario in your opinion or have I just identified a poorly architected edge case scenario?


-Michael Coates

10 comments:

  1. Can't we just escape it in precontent.jsp? Assuming double encoding isn't an issue

    ReplyDelete
  2. Escape it there and don't apply POST restrictions and set default values in precontent is a more complete followup

    ReplyDelete
  3. Definitely, its not hard to protect against the flaw once you've identified the issue. The main point of the article is that its not a desired or expected behavior for the call which grabs a parameter from a JSP:include tag parameter to fail over to the URL.

    ReplyDelete
  4. I am by no means a JSP expert but a typo is a common issue in any project - which is why I very much dislike languages that won't report them. Even two identical typos can happen more often than one would think, people simply copy over the mistyped variable name. So this is a serious issue IMO, on par with register_globals in PHP.

    Does it really require two typos? What happens if the variable lessonName is declared but somebody forgets to pass it in as parameter (or mistypes its name there)?

    ReplyDelete
  5. Excellent point on the typo Wladimir. A single typo can introduce this problem and no errors are generated. The typo would occur within the name of jsp:param. For example:

    jsp:param name="lessonObjectiveOOPS"

    instead of

    jsp:param name="lessonObjective"


    Ikes. Very dangerous and prone to errors.

    ReplyDelete
  6. damn - forgot to esape my code... the code should read (where the big space in the middle of my comment is)

    <jsp:include page="/WEB-INF/tiles/nav/topnav.jsp">
    <jsp:param name="persistParms" value="q,aff"/>
    </jsp:include>

    ReplyDelete
  7. We're going to try this one more time. My comment posting abilities are under question. This is the entire comment:

    The ultimate problem here is that the container is persisting the request meta-data across multiple requests. The jsp:include tag actually makes a second request *inside* the container. This decision was probably made as a conveinience so that if you have say a search form, that is shared across several pages in something like a navigation component on your site, you don't have to keep passing the "q" parameter around. In a lot of ways this makes sense. It seems like a whitelist approach combined with a filter in the container is the right approach to take for this issue. Where a parameter could be passed along with the second request. Something along the lines of:

    <jsp:include page="/WEB-INF/tiles/nav/topnav.jsp">
    <jsp:param name="persistParms" value="q,aff"/>
    </jsp:include>

    The filter could then enumerate the request and strip anything not whitelisted. The biggest problem is that this would break a ton of existing functionality - I know a ton of sites that are relying on jsp:includes and the ability to access parms from the original request without any additional work.

    If you didn't submit a bug to tomcat, I think I probably would. I am sure that other servlet containers have this same issue, however, most of them will follow suit with whatever the tomcat folks decide to do. At bare minimum, it will at least be getting in front of a lot of people who really understand the ins and outs of the container and generate some interesting conversation and possibly ideas to resolve this is a manner that doesn't affect usability while making it behave in a more predictable manner.

    ReplyDelete
  8. Now wanting to sound evil the include is working like intended, to include code in the jsp with the same parameter set. the " is only changing an available parameter if it already exists. You are also breaking one of the primary/sacred rules of programming by not validating user input. Any or all input is to be verified, especially if it is going to be used for control.

    A safer way to do this would be to use request attributes as they are server internal and even scoped on the request.

    ReplyDelete
  9. The point of the post isn't to debate input validation. We all agree that is the correct approach. The purpose was to point out how data that would otherwise never be publicly accessible could be exposed by a simple slip up as indicated above.

    ReplyDelete
  10. Can't this simply be avoided by putting the include file preContent.jsp in a protected floder.

    ReplyDelete

Note: Only a member of this blog may post a comment.