Skip to content

UriComponentsBuilder Forwarded header parsing can throw java.lang.NumberFormatException [SPR-16660] #21201

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

Closed
spring-projects-issues opened this issue Mar 28, 2018 · 4 comments
Assignees
Labels
in: web Issues in web modules (web, webmvc, webflux, websocket) status: backported An issue that has been backported to maintenance branches type: bug A general bug
Milestone

Comments

@spring-projects-issues
Copy link
Collaborator

spring-projects-issues commented Mar 28, 2018

George Kearns opened SPR-16660 and commented

The UriComponentsBuilder.adaptForwardedHost method blindly executes Integer.parseInt on the string value following the ":" in the header being parsed; that string may cause a java.lang.NumberFormatException if it is not parseable to an int.

To reproduce, send a header like this:
X-Forwarded-Host=[]somewhere.com://badport/

Produces stack trace:
java.lang.NumberFormatException: For input string: "//badport/"
at java.lang.NumberFormatException.forInputString(NumberFormatException.java:65)
at java.lang.Integer.parseInt(Integer.java:569)
at java.lang.Integer.parseInt(Integer.java:615)
at org.springframework.web.util.UriComponentsBuilder.adaptForwardedHost(UriComponentsBuilder.java:716)
at org.springframework.web.util.UriComponentsBuilder.adaptFromForwardedHeaders(UriComponentsBuilder.java:690)
at org.springframework.web.util.WebUtils.isSameOrigin(WebUtils.java:833)
at org.springframework.web.cors.DefaultCorsProcessor.processRequest(DefaultCorsProcessor.java:76)
at org.springframework.web.servlet.handler.AbstractHandlerMapping$CorsInterceptor.preHandle(AbstractHandlerMapping.java:507)
at org.springframework.web.servlet.HandlerExecutionChain.applyPreHandle(HandlerExecutionChain.java:133)
at org.springframework.web.servlet.DispatcherServlet.doDispatch(DispatcherServlet.java:962)
at org.springframework.web.servlet.DispatcherServlet.doService(DispatcherServlet.java:901)
at org.springframework.web.servlet.FrameworkServlet.processRequest(FrameworkServlet.java:970)
at org.springframework.web.servlet.FrameworkServlet.doGet(FrameworkServlet.java:861)
at javax.servlet.http.HttpServlet.service(HttpServlet.java:635)
at org.springframework.web.servlet.FrameworkServlet.service(FrameworkServlet.java:846)
at javax.servlet.http.HttpServlet.service(HttpServlet.java:742)
at org.apache.catalina.core.ApplicationFilterChain.internalDoFilter(ApplicationFilterChain.java:231)
at org.apache.catalina.core.ApplicationFilterChain.doFilter(ApplicationFilterChain.java:166)
at org.apache.catalina.core.ApplicationDispatcher.invoke(ApplicationDispatcher.java:728)
at org.apache.catalina.core.ApplicationDispatcher.processRequest(ApplicationDispatcher.java:469)
at org.apache.catalina.core.ApplicationDispatcher.doForward(ApplicationDispatcher.java:392)
at org.apache.catalina.core.ApplicationDispatcher.forward(ApplicationDispatcher.java:311)
at org.apache.catalina.core.StandardHostValve.custom(StandardHostValve.java:395)
at org.apache.catalina.core.StandardHostValve.status(StandardHostValve.java:254)
at org.apache.catalina.core.StandardHostValve.invoke(StandardHostValve.java:177)
at org.apache.catalina.valves.ErrorReportValve.invoke(ErrorReportValve.java:80)
at org.apache.catalina.core.StandardEngineValve.invoke(StandardEngineValve.java:87)
at org.apache.catalina.connector.CoyoteAdapter.service(CoyoteAdapter.java:342)
at org.apache.coyote.http11.Http11Processor.service(Http11Processor.java:799)
at org.apache.coyote.AbstractProcessorLight.process(AbstractProcessorLight.java:66)
at org.apache.coyote.AbstractProtocol$ConnectionHandler.process(AbstractProtocol.java:868)
at org.apache.tomcat.util.net.NioEndpoint$SocketProcessor.doRun(NioEndpoint.java:1455)
at org.apache.tomcat.util.net.SocketProcessorBase.run(SocketProcessorBase.java:49)
at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1142)
at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:617)
at org.apache.tomcat.util.threads.TaskThread$WrappingRunnable.run(TaskThread.java:61)
at java.lang.Thread.run(Thread.java:748)


Affects: 4.3.14, 5.0.4

Issue Links:

Backported to: 4.3.15

@spring-projects-issues
Copy link
Collaborator Author

Juergen Hoeller commented

I wonder whether we should reject such an invalid X-Forwarded header with an explicit exception or rather silently ignore it...

Rossen Stoyanchev, what's your take on this?

@spring-projects-issues
Copy link
Collaborator Author

Rossen Stoyanchev commented

Rejecting it proactively is probably a good idea, especially if combined with a helpful error message pointing out the potential security concern and pointing to the ForwardedHeaderFilter which can also be configured to discard such headers if not behind a trusted proxy.

George Kearns is that a representative example of an actual request? Is it coming from a proxy, which would be surprising since they shouldn't send such badly formed values. Or is it potentially malicious, from an untrusted client?

@spring-projects-issues
Copy link
Collaborator Author

George Kearns commented

It was potentially malicious from an untrusted client, we found it through an uncaught exception alert.

@spring-projects-issues
Copy link
Collaborator Author

spring-projects-issues commented Mar 29, 2018

Rossen Stoyanchev commented

I've added an explicit rejection of errors from parsing a port along with a more helpful message. Note that in 5.1 we will move to a more centralized model for dealing with forwarded headers, see #21209.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: web Issues in web modules (web, webmvc, webflux, websocket) status: backported An issue that has been backported to maintenance branches type: bug A general bug
Projects
None yet
Development

No branches or pull requests

2 participants