-
Notifications
You must be signed in to change notification settings - Fork 1.9k
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
Path Normalization/Traversal - Context Matching #4275
Path Normalization/Traversal - Context Matching #4275
Comments
The use of https://tools.ietf.org/html/rfc3986#section-3.3 As for how path parameters are to be treated. There's been discussion about how to handle normalization when the URI is complicated with path parameters, URI templates variables, etc. |
Java package uri;
import java.net.URI;
public class URITest
{
public static void main(String[] args)
{
dump(URI.create("http://machine.com/ws/v1/..;/..;/foo"));
dump(URI.create("http://machine.com/ws/v1/../../foo"));
}
public static void dump(URI uri)
{
System.out.println("URI = " + uri);
System.out.println(" .normalize = " + uri.normalize());
}
} results in ...
|
Yup agree Seems like this is being misused. Was checking to see if there were any controls/options in Jetty - to not match paths like this i.e. in org.eclipse.jetty.server.Request I see _originalURI is |
@venukb It is correct for jetty to match that normalised path to a servlet. But then the protection of /foo must also use normalised paths. We should be applying security constraints to normalised URIs so |
Also, as this potentially has security ramifications can you send your response via email to [email protected] |
Note that I have retested Jetty security constraint handling and I cannot reproduce a problem. Please email us more details of your issue - I expect that you are using some other form of protection that is not working on normalised paths.... but don't say what it is publicly. |
Thanks. I'll share details on email. Was told that Undertow does not allow this i.e. it rejects request rather than matching So was checking if there was a config to disallow this kind of request pattern. |
There is no configuration to disable, as it is a perfectly legal URI. However, it would be trivial to write a filter and/or customizer that rejected any requests that have .. in their raw request URI... or if you wanted to be really string if the URI does not end with exactly the decoded contextPath+servletPath+pathInfo. |
Out of an abundance of caution I have reviewed this issue again to see if we really should be normalising /..;/ out. Re-reading https://tools.ietf.org/html/rfc3986#section-3.3 the answer is once again NO because the RFC only defines the behaviour of path segments ".." and ".", so a path segment of "..;" is not ".." and should not walk the hierarchy. So we are technically correct..... HOWEVER! It is obvious that there is still a risk if path parameters are later stripped from the URI, which is then passed to a component that does interpret "..". Moreover, I really don't see any valid use-case for /..;/ other than trying to hack around web security. Thus I'm now thinking that we should just treat such URIs as illegal. Not even follow the .., but just return a 400 response treating these as bad URIs. |
Hmmm, but then should we also reject paths with |
We should review any delimiter then, not just limited to the one specific to path parameters. https://tools.ietf.org/html/rfc3986#section-2.2 I remembered this from a prior issue #2067 |
Actually, I think we are good here. We feed the request handling process with: I'm closing again. |
Well, it's not just about security restriction in Jetty. It's also a publishing issue. You publish /ws/v1 on a reverse proxy to the Internet, but /foo only to internal networks. Both without security configuration in Jetty, but solely access restrictions on reverse proxy. Now any Internet use can abuse this path traversal to access /foo. |
On review, I agree that the normalisation is strange. It is a bad combination of two reasonable steps: it is according to the spec to not consider "..;" the same as a ".." segment; It is a reasonable decoding of a "..;" segment to result in "..". Internally we don't have a security issue, because we apply the same process to matching and security constraints, but I agree that in a wider system it is problematic. I'm currently thinking that we should just throw an ISE when parsing any URI that includes a segment with "..;" or ".;<param" in it (where param can be empty). I do not see any valid uses for such segments. |
I don't think that is correct since To have @gregw I'm more on the lines of just dropping middle-segments parameters (the only one to retain is a final segment parameter named |
@sbordet The problem is that there are multiple paths through the specifications:
So the URI is both I think the best solution is to treat any URIs with segments of |
@sbordet I agree. It was new to me that each path segment could have parameters. And probably it is to reverse proxy implementors. Apache for example doesn't normalize it like that and will happily request the wrong backend. |
@sbordet .... and we do just drop middle segment parameters.... that's the problem as it normalises to |
@gregw I don't see in https://tools.ietf.org/html/rfc3986#section-3.3 that In any case, I agree that a 400 is acceptable -- such weird paths are typically just used as attacks, not in real applications. |
@sbordet the RFC talks about
Also it says:
So |
Fix #4275 fail URIs with ambiguous segments.
A URI like `/foo/%2e%2e;/bar` should be ambiguous both because of the encoded dots and because of the parameters. This means that the default setting of jetty-9 is a bit more secure as this path is considered ambiguous if either Violation.SEGMENT or Violation.PARAM is set. Signed-off-by: Greg Wilkins <[email protected]>
* Improve #4275 ambiguous URIs A URI like `/foo/%2e%2e;/bar` should be ambiguous both because of the encoded dots and because of the parameters. This means that the default setting of jetty-9 is a bit more secure as this path is considered ambiguous if either Violation.SEGMENT or Violation.PARAM is set.
Need to cherry-pick #6939 to jetty-10 |
* Improve #4275 ambiguous URIs A URI like `/foo/%2e%2e;/bar` should be ambiguous both because of the encoded dots and because of the parameters. This means that the default setting of jetty-9 is a bit more secure as this path is considered ambiguous if either Violation.SEGMENT or Violation.PARAM is set. Signed-off-by: Lachlan Roberts <[email protected]>
We have an app with 2 servlet path defined
/ws/v1
/foo
/ws/v1
was exposed on edge (ATS). However/foo
accessed on Jetty using this/ws/v1/..;/..;/foo
- so we had a path exposed on the edge which was not intended to be exposed(Jetty version: 9.4.22 - same behavior seen in older versions as well)
The text was updated successfully, but these errors were encountered: