Server info leak after corrupted request

[dreambrother]

I want to prevent any server info leak in error responses. I disabled whitelabel and stacktrace printing
server.error.include-stacktrace=never
server.error.whitelabel.enabled=false

I also tried to add my own error controller and custom error pages, but when I send corrupted request, embedded Tomcat responses with server info and stacktrace.

Example of corrupted request:

curl 'http://localhost:8080/foo' -H 'content-type: multipart/form-data; boundary=----WebKitFormBoundaryHBic2VjAi3E215bk' --data-binary $'------WebKitFormBoundaryHBic2VjAi3E215bk\r\nContent-Disposition: form-data; name="file"; filename="doc-xlsx.xlsx"\r\nContent-Type: application/vnd.openxmlformats-officedocument.spreadsheetml.sheet\r\n\r\n\r\n------WebKitFormBoundaryHBic2VjAi3E215bk\r\nContent-Disposition:  '

I tried to turn off report and server info printing manually, but exception message still presents in responses (it contains tomcat's and spring's classes names).

@Bean
public EmbeddedServletContainerFactory servletContainer() {
    // Disable tomcat version and exception logging by ErrorReportValve
    TomcatEmbeddedServletContainerFactory tomcat = new TomcatEmbeddedServletContainerFactory();

    ErrorReportValve errorReportValve = new ErrorReportValve();
    errorReportValve.setShowReport(false);
    errorReportValve.setShowServerInfo(false);

    tomcat.addContextValves(errorReportValve);

    return tomcat;
}

I think it's possible to overwrite TomcatEmbeddedServletContainerFactory#getEmbeddedServletContainer to return Tomcat instance with different host property, StandardHost#errorReportValveClass should be changed for another class in that case. I didn't try this solution, because it creates too much coupling to underlying implementations. Looks like spring-boot should be able to do this.

[wilkinsona]

Could you please provide a small sample that illustrates the problem you have described? I want to be sure that we're investigating exactly what you're describing.

[dreambrother]

I've created a project https://github.com/dreambrother/spring-boot-server-info-leak-example illustrating issue described above. I've added public/error/500.html page to handle 500 errors, and it works as expected for exceptions thrown in controller

@RequestMapping("/exception")
public String error() {
    throw new RuntimeException("Error test");
}

but not for corrupted request. Example of corrupted request was given in the issue description.

[wilkinsona]

@dreambrother Thanks. Your sample doesn't exactly reproduce the behaviour you've described, but I've been able to modify it to do so. The problem with your sample is that your custom configuration of the ErrorReportValve places it in the wrong place so the logic in Tomcat that would route to /error and then one of your custom error pages doesn't get called.

The underlying problem with the corrupted request is due in part to HiddenHttpMethodFilter. It's in the filter chain for the dispatch to /error and its call to request.getParameter("_method") fails due to the corrupted request. This failure while trying to dispatch to an error page causes Tomcat's default error page to be rendered. You can disable HiddenHttpMethodFilter by adding a FilterRegistrationBean that it not enabled:

@Bean
public FilterRegistrationBean hiddenHttpMethodFilterRegistration(OrderedHiddenHttpMethodFilter filter) {
    FilterRegistrationBean registration = new FilterRegistrationBean(filter);
    registration.setEnabled(false);
    return registration;
}

An alternative would be to provide a custom HiddenHttpMethodFilter that doesn't do anything when an error is being dispatched:

@Bean
public OrderedHiddenHttpMethodFilter hiddenHttpMethodFilter() {
    return new OrderedHiddenHttpMethodFilter() {

        @Override
        protected void doFilterInternal(HttpServletRequest request, HttpServletResponse response,
                FilterChain filterChain) throws ServletException, IOException {
            if (request.getAttribute(WebUtils.ERROR_EXCEPTION_ATTRIBUTE) != null) {
                filterChain.doFilter(request, response);
            }
            else {
                super.doFilterInternal(request, response, filterChain);
            }
        }

    };
}

It feels to me like HiddenHttpMethodFilter should do something like this by default. I've opened SPR-15179.

Either approach doesn't fully resolve the problem as the dispatch to /error still fails. The request now makes it to DispatcherServlet but its attempt to check the multipart request fails:

Caused by: org.springframework.web.multipart.MultipartException: Could not parse multipart servlet request; nested exception is java.io.IOException: org.apache.tomcat.util.http.fileupload.FileUploadException: Stream ended unexpectedly
	at org.springframework.web.multipart.support.StandardMultipartHttpServletRequest.parseRequest(StandardMultipartHttpServletRequest.java:111) ~[spring-web-4.3.5.RELEASE.jar:4.3.5.RELEASE]
	at org.springframework.web.multipart.support.StandardMultipartHttpServletRequest.<init>(StandardMultipartHttpServletRequest.java:85) ~[spring-web-4.3.5.RELEASE.jar:4.3.5.RELEASE]
	at org.springframework.web.multipart.support.StandardServletMultipartResolver.resolveMultipart(StandardServletMultipartResolver.java:76) ~[spring-web-4.3.5.RELEASE.jar:4.3.5.RELEASE]
	at org.springframework.web.servlet.DispatcherServlet.checkMultipart(DispatcherServlet.java:1099) ~[spring-webmvc-4.3.5.RELEASE.jar:4.3.5.RELEASE]
	at org.springframework.web.servlet.DispatcherServlet.doDispatch(DispatcherServlet.java:932) ~[spring-webmvc-4.3.5.RELEASE.jar:4.3.5.RELEASE]
	at org.springframework.web.servlet.DispatcherServlet.doService(DispatcherServlet.java:897) ~[spring-webmvc-4.3.5.RELEASE.jar:4.3.5.RELEASE]
	at org.springframework.web.servlet.FrameworkServlet.processRequest(FrameworkServlet.java:970) ~[spring-webmvc-4.3.5.RELEASE.jar:4.3.5.RELEASE]
	... 26 common frames omitted

The logging in checkMultipart suggests that this problem should be avoided by checking the javax.servlet.error.exception request attribute:

else if (request.getAttribute(WebUtils.ERROR_EXCEPTION_ATTRIBUTE) instanceof MultipartException) {
	logger.debug("Multipart resolution failed for current request before - " +
			"skipping re-resolution for undisturbed error rendering");
}

However, this doesn't work as the request attribute is a NestedServletException with a MultipartException cause. You can work around this by declaring a custom DispatcherServlet bean that overrides checkMultipart():

@Bean
public DispatcherServlet dispatcherServlet() {
    return new DispatcherServlet() {
        protected HttpServletRequest checkMultipart(HttpServletRequest request) throws MultipartException {
            if (getMultipartResolver() != null && getMultipartResolver().isMultipart(request)) {
                if (WebUtils.getNativeRequest(request, MultipartHttpServletRequest.class) != null) {
                    logger.debug("Request is already a MultipartHttpServletRequest - if not in a forward, " +
                            "this typically results from an additional MultipartFilter in web.xml");
                } else {
                    Throwable error = (Throwable)request.getAttribute(WebUtils.ERROR_EXCEPTION_ATTRIBUTE);
                    while (error != null) {
                        if (error instanceof MultipartException) {
                            logger.debug("Multipart resolution failed for current request before - " +
                                    "skipping re-resolution for undisturbed error rendering");
                            return request;
                        }
                        error = error.getCause();
                    }
                    return getMultipartResolver().resolveMultipart(request);
                }
            }
            // If not returned before: return original request.
            return request;
        }
    };
}

I've opened https://jira.spring.io/browse/SPR-15178

[wilkinsona]

@dreambrother Can you please let us know how you get on with the workarounds described above? If both SPR issues are accepted, there may be nothing to do in Boot.

[wilkinsona]

The two SPR issues have been fixed. The workarounds described above are no longer required when using Spring Framework 4.3.6 snapshots. We'll upgrade to 4.3.6.RELEASE in #7774.