Why does the ESAPI ClickjackFilter have to come after the SiteMesh filter?

Question

We have an application using OpenSymphony SiteMesh to assemble pages, and we've added the OWASP ESAPI ClickjackFilter to add the X-FRAME-OPTIONS header to responses.

However, it only works if the ClickjackFilter mapping comes after the SiteMeshFilter mapping in web.xml. If the clickjacking filter comes first, then the X-FRAME-OPTIONS header isn't added.

This works:

<filter-mapping>
    <filter-name>sitemesh</filter-name>
    <url-pattern>/web/*</url-pattern>
</filter-mapping>

<filter-mapping>
    <filter-name>Clickjacking filter</filter-name>
    <url-pattern>/*</url-pattern>
</filter-mapping>

This doesn't work:

<filter-mapping>
    <filter-name>Clickjacking filter</filter-name>
    <url-pattern>/*</url-pattern>
</filter-mapping>

<filter-mapping>
    <filter-name>sitemesh</filter-name>
    <url-pattern>/web/*</url-pattern>
</filter-mapping>

Why would the ordering of these two filters matter?

Typically xml is processed in order specified, so what's probably happening is that sitemesh sets its own options for that header, and that's why if the clickjack filter loads first, it probably gets its settings blown away by sitemesh's option. — avgvstvs, Jun 05 '15 at 19:31

score 0 · Accepted Answer · answered Jun 06 '15 at 21:37

In my opinion, I think it is because the ESAPI ClickjackFilter's doFilter() method is written incorrectly. It is implemented like this:

public void doFilter(ServletRequest request, ServletResponse response, FilterChain chain) throws IOException, ServletException
        {
        HttpServletResponse res = (HttpServletResponse)response;
        chain.doFilter(request, response);
        res.addHeader("X-FRAME-OPTIONS", mode );
        }

However, because it is an output filter, it should be first wrapping the response using something like HttpServletResponseWrapper. I think it should be written something like this:

public void doFilter(ServletRequest request, ServletResponse response, FilterChain chain) throws IOException, ServletException
        {
        HttpServletResponse res =
            new javax.servlet.http.HttpServletResponseWrapper(
                                            (HttpServletResponse)response );
        chain.doFilter(request, res);
        res.addHeader("X-FRAME-OPTIONS", mode );
        }

If it were written in that manner, I think it should work regardless of the order the are applied in.

CAVEAT: Note that I've not verified this at all (in fact, I haven't even tried to compile it!), but I think this is likely what is wrong. In theory, the SiteMesh filter could be doing something funky as well, but I think that's less likely. If someone confirms this is what is wrong, let me know and I'll file an ESAPI bug report.

I think that you're on the right track. However, if I create a custom class, using the same code advertised at https://www.owasp.org/index.php/ClickjackFilter_for_Java_EE (but with a different package), then that also seems to work. It's only when I use the out-of-the-box ClickjackFilter, appearing before SiteMesh, that the header doesn't appear. — ThrawnCA, Jun 08 '15 at 23:01
Yeah, I think that would work too. Also, I should have probably used HttpServletResponse.setHeader() rather than addHeader() in my above example to ensure that if any intermediate JavaEE filters in the filter chain add the X-FRAME-OPTIONS, that this overrides it. That's unlikely, but possible and only likely to cause confusion of the two use different modes. I suppose a later filter in the chain could do the same thing (call setHeader() or addHeader() again with the same X-FRAME-OPTIONS header), but if you're going to use a filter, you should probably have some idea of what it is doing. — Kevin W. Wall, Sep 02 '15 at 02:34

score 0 · Answer 2 · answered Sep 02 '15 at 02:39

0

Looks like there is already a bug filed against this (with a supposed fix, which BTW, I have not tested or otherwise confirmed). The bug ID is 289. Details at: https://github.com/ESAPI/esapi-java-legacy/issues/289

answered Sep 02 '15 at 02:39

Kevin W. Wall

1,347
7
7

Why does the ESAPI ClickjackFilter have to come after the SiteMesh filter?

2 Answers2