SEC-1606: FirewalledRequest.reset() not called when Filters in the FilterChainProxy do not complete the FilterChain #1846

Closed
spring-issuemaster opened this Issue Nov 1, 2010 · 12 comments

2 participants

@spring-issuemaster

Rob Winch (Migrated from SEC-1606) said:

When the FilterChain is not completed within FilterChainProxy, FirewalledRequest.reset() is not called. This can cause the wrong pathInfo and servletPath to be exposed when a forward or include is performed within the FilterChainProxy's filters and using DefaultHttpFirewall. The reason is because the pathInfo and the servletPath are still cached from the original request and reset was never called.

The following configuration will demonstrate the issue. When an invalid username/password is submitted, the request is forwarded to /login.jsp?login_error=1. However, when the JspServlet attempts to process the URL it sees the HttpServeltRequest.servletPath as RequestWrapper.strippedServletPath (/j_spring_security_check) instead of the new servletPath /login.jsp.






p:defaultFailureUrl="/login.jsp?login_error=1"
p:useForward="true"/>






I have not yet validated this occurs in 3.1.0.M2 but marked it as impacted to ensure it at least gets looked at.

@spring-issuemaster

Rob Winch said:

removed code macro because it did not work

@spring-issuemaster

Luke Taylor said:

Thanks Rob. There's also a similar bug when using filters="none" (which is easier to fix). The reset method isn't called in cases where there are no matching filters.

I guess we can also wrap calls to request.getRequestDispatcher and create a RequestDispatcher which calls the reset method on the wrapped request when its "forward" method is called. Does that make sense?

@spring-issuemaster

Rob Winch said:

A place to at least start for those encountering this issue might be to override the Firewall implementation using the attached file. Next update the spring security configuration to ensure that the spring security configuration used is 3.0.4 and override the firewall. An example of the relevant configuration is below:

<?xml version="1.0" encoding="UTF-8"?>
xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
xmlns="http://www.springframework.org/schema/security"
xmlns:p="http://www.springframework.org/schema/p"
xsi:schemaLocation="http://www.springframework.org/schema/security http://www.springframework.org/schema/security/spring-security-3.0.4.xsd
http://www.springframework.org/schema/beans http://www.springframework.org/schema/beans/spring-beans-3.0.xsd">


@spring-issuemaster

Rob Winch said:

Sorry I got interrupted and didn't see your post (I had left the tab open). I actually was thinking of the same solution. The attached file is intended for those that are trying for a quick workaround (i.e. it isn't as clean as I would think a patch would be). For those following the bug, the attached fix doesn't address the filters="none"

@spring-issuemaster

Rob Winch said:

Something else we may need to consider is that forwards can happen from ServletContext as well (i.e. getRequestDispatcher(String path) and getNamedDispatcher(String name)). While Filters injected into the FilterChainProxy may have a hard time getting a hold of FilterConfig that has the ServletContext, they may be implementing ServletContextAware which would inject the ServletContext into them. At the moment I am a bit perplexed on if/how to address this.

@spring-issuemaster

Rob Winch said:

PS: We may just be able to leverage the presence of forward request attributes (i.e. javax.servlet.forward.request_uri) to determine if the request was forwarded. If it was forwarded, then set stripPaths to false. I think this feature was introduced in Servlet 2.4+ spec, but I am not certain this approach works for all the Servlet Versions we need to support

@spring-issuemaster

Rob Winch said:

For record keeping purposes. Anyone that is forwarding using ServletContext can provide their own implementation of the HttpFirewall or switch to using the HttpServletRequest to obtain the RequestDispatcher.

@spring-issuemaster

Rob Winch said:

For those experiencing this issue, a variation of the fix that was submitted can be applied to 3.0.4.RELEASE using the following.

<?xml version="1.0" encoding="UTF-8"?>
xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
xmlns="http://www.springframework.org/schema/security"
xmlns:p="http://www.springframework.org/schema/p"
xsi:schemaLocation="http://www.springframework.org/schema/security http://www.springframework.org/schema/security/spring-security-3.0.4.xsd
http://www.springframework.org/schema/beans http://www.springframework.org/schema/beans/spring-beans-3.0.xsd">



...

@spring-issuemaster

Stéphane Nicoll said:

The fix attached to this issue has two issues:

The imports on spring security are wrong, it's missing a web

It does not fix the problem with filter="none"

@spring-issuemaster

Rob Winch said:

Thank you for your feedback.

The imports on spring security are wrong, it's missing a web

Can you elaborate on what you mean by this?

It does not fix the problem with filter="none"

You are correct and it does not attempt to. This is logged as a separate issue (SEC-1608)

@spring-issuemaster

Stéphane Nicoll said:

Rob,

Your patch is built against Spring Security 2.0.x. I use 3.0.x. In 3.0.x, these classes are in org.springframework.security.web.firewall

No biggie. For the separated issue, thanks. I can see the support has added a comment with the workarounds.

@spring-issuemaster spring-issuemaster added this to the 3.1.0.M2 milestone Feb 5, 2016
@spring-issuemaster

This issue relates to #1848

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment