Replace many following slashes of client URI with single slash. [SPR-15529]

[spring-projects-issues]

Maksim Kostromin opened SPR-15529 and commented

(Updated title and description)

Right now when request curl http://localhost:3000// (or http :3000//) is sending (which is ends at least with one additional '/' symbol):

1. URI.create will fail with message:

2017-05-08 23:30:21.770 WARN 24578 --- [ctor-http-nio-7] i.n.u.concurrent.AbstractEventExecutor : A task raised an exception. Task: reactor.ipc.netty.channel.ContextHandler$$Lambda$159/430674510@75bc1117

java.lang.IllegalArgumentException: Expected authority at index 2: //

2. Terminal is hangs:

$ http :3000//

http: error: Request timed out (30s).
I didn't investigate what's going on with connection itself, probably it will not be closed some time. but anyway, As an attacker, I can easily DDOS spring 5 apps, I need only generate 65k these request for 30 sec and your service will be unavailable

issue can be reproduced using this example (reactive-service)

possible fix for that case is replace in uri-string few slashes with only one before it URI will be resolved. for example we can use:

uri.replaceAll("/{2,}", "/");

I've created PR with such fix: #1423

---

Affects: 5.0 RC1

Reference URL: spring-projects/spring-boot#9133

Issue Links:

• DDoS Spring 5 RouterFunction apps [SPR-15560] #20119 DDoS Spring 5 RouterFunction apps ("is superseded by")

[spring-projects-issues]

Rossen Stoyanchev commented

The link to RFC 2396 gives a 404 and that spec is replaced by RFC 3986. The URL syntax requires at least 1 character in a path segment. I am inclined to proactively reject such invalid URLs rather than mask it. Unless they're re-written at a very low level (server or proxy), then different parts of the code can make decisions based on a different URL, and that can be a source of security issues.

Rob Winch, Brian Clozel any comments on this?

[spring-projects-issues]

Rob Winch commented

Given the specification requires at least one character in the path segment and allowing for // would increase the likelihood of various attacks (i.e. directory traversal) I also suggest that we reject the request.

[spring-projects-issues]

Brian Clozel commented

I think the provided fix would create other issues, since it would encode "//" instances in the query and fragment parts of the URI, where they are valid.
I'd rather reject those URLs as invalid since that's what they are, and relying on URI.create is probably the best we can do in those cases.

[spring-projects-issues]

Maksim Kostromin commented

I think the provided fix would create other issues, since it would encode "//" instances in the query and fragment parts of the URI

@Brian Clozel, no it's not (seems like I should rename title), please read whole description.
provided PF is fixing in other way: is replacing any flowing slashes into single slash
ie ////// -> /
so any http://example.com////z//y///z will be transformed into http://example.com/z/y/z

[spring-projects-issues]

Maksim Kostromin commented

@Rossen Stoyanchev, way issue is closed? This is a serious bug and it's currently available for any spring 5 apps. What can you propose and how can I handle this particular case on service side? I can't control clients request and I don't want let someone DDOS me

[spring-projects-issues]

Rossen Stoyanchev commented

I see that you have since re-worked the original description with information about hanging. We will of course address that but this ticket was closed because we will not quietly replace multiple slashes as suggested in the original ticket. We choose to treat it as bad input and reject it. We should not be hanging however.

Please create a separate ticket and describe the hanging scenario.

[spring-projects-issues]

Maksim Kostromin commented

I created another JIRA: #20119