Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Security-related HTTP headers not written if response is committed during INCLUDE dispatch #5499

Closed
chschu opened this issue Jul 11, 2018 · 2 comments
Assignees
Labels
in: web An issue in web modules (web, webmvc) type: bug A general bug
Milestone

Comments

@chschu
Copy link

chschu commented Jul 11, 2018

Summary

HeaderWriterFilter does not write HTTP headers if the response is committed inside a DispatcherType.INCLUDE request dispatch.

This means that critical headers might be missing in the response, introducing a potential security risk.

Actual Behavior

According to the servlet specification, response HTTP headers cannot be set during an INCLUDE dispatch, e.g. in an included JSP:

Section 9.3 - The Include method
[...] It cannot set headers or call any method that affects the headers of the response, with the exception of the HttpServletRequest.getSession() and HttpServletRequest.getSession(boolean) methods. Any attempt to set the headers must be ignored, [...]

So if the response is committed during a include that produced a lot of output to the response body, the HeaderWriterFilter calls the response methods that would normally set the HTTP headers, but those calls are ignored by the servlet container.

At least Tomcat 8.5.x adheres to the servlet specification an does not write the headers to the response.

Expected Behavior

The headers are always written to the response, even if the response is committed during an include.

Configuration

  • Tomcat 8.5.28
  • Spring-based web application with a JSP that includes other JSP producing more than 8 KB of output (default buffer size)

Version

5.0.6.RELEASE

Sample

spring-security-gh5499.zip

Attached is a Zip file containing a simple Spring Boot application. Simply run the de.chschu.spring.security.gh5499.Application main class.

The response of http://localhost:8080/positive will have the headers, while http://localhost:8080/negative (which simply includes the other one) will not.

@haasc
Copy link

haasc commented Sep 20, 2018

I'm have the same problem, but I've diagnosed it a little differently.

In my case, by the time the header writing is attempted during the processing of the include file, the HttpResponse is wrapped with org.apache.catalina.core.ApplicationHttpResponse with the "included" attribute set to true. With the included field set to true, all of the headers that get emitted are suppressed. However, as expected, the response is committed.

So, when the processing gets back to the header filter, the response has been committed, so again headers are suppressed.

The net result is that no headers get emitted.

@rwinch rwinch added this to the 5.1.1 milestone Sep 20, 2018
@rwinch
Copy link
Member

rwinch commented Sep 20, 2018

I think we can resolve this using an OnComittedRequestWrapper which triggers the headers to be written before an attempt to include content happens.

@rwinch rwinch added in: web An issue in web modules (web, webmvc) type: bug A general bug labels Sep 20, 2018
@rwinch rwinch modified the milestones: 5.1.1, 5.1.2 Oct 12, 2018
@jzheaux jzheaux added this to jzheaux in Security Team Oct 30, 2018
@jzheaux jzheaux moved this from jzheaux to In Progress in Security Team Oct 30, 2018
@jzheaux jzheaux self-assigned this Oct 30, 2018
jzheaux added a commit to jzheaux/spring-security that referenced this issue Oct 30, 2018
HeaderWriterFilter wraps request dispatcher so it can write security
headers before the include occurs.

Fixes: spring-projectsgh-5499
@jzheaux jzheaux moved this from In Progress to jzheaux in Security Team Oct 30, 2018
@rwinch rwinch modified the milestones: 5.1.2, 5.2.0.M1 Oct 31, 2018
rwinch pushed a commit that referenced this issue Oct 31, 2018
HeaderWriterFilter wraps request dispatcher so it can write security
headers before the include occurs.

Fixes: gh-5499
jzheaux added a commit that referenced this issue Oct 31, 2018
HeaderWriterFilter wraps request dispatcher so it can write security
headers before the include occurs.

Fixes: gh-5499
jzheaux added a commit that referenced this issue Oct 31, 2018
HeaderWriterFilter wraps request dispatcher so it can write security
headers before the include occurs.

Fixes: gh-5499
@jzheaux jzheaux removed this from jzheaux in Security Team Oct 31, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: web An issue in web modules (web, webmvc) type: bug A general bug
Projects
None yet
Development

No branches or pull requests

4 participants