On 4/19/2016 4:13 PM, Dimitar Valov wrote:
Hi Mark,
Yes, it is sensitive, that's why I'm suggesting such simple solution. For
me the issues comes because of AbstractFileResourceSet::file -> absPath =
absPath.substring(absoluteBase.length() + 1); and if
(!canPath.equals(absPath)). This + 1 should account for the /. However we
cannot count on not having the / at the end of absPath.
Why not explicitly check for the existence of the trailing "\", rather
than assuming it's (non)existence and just checking the string length?
I think Mark is saying that you have found an inconsistency, and pending
further testing thinks that there should be changes made so that you get
back what you supplied to those functions. I.E., if you gave it a
trailing "\" as an argument, you'll get one back, and if not, you won't.
I've played with specifying paths that end with / and without, and I think
that there's no way that the current implementation can be configured
correctly:
* if path like ../../ is used it is normalized to path ending with / =>
the substring will consume the first character of the filename.
* if path like ../.. is used it is normalized to path ending .. (which is
even worse), but this can be documented out, otherwise a method like
Path.normalize should be used or implemented.
* if path like ../../app or ../../app/ is used everything works
correctly, but this is bacause StandardContext::fixDocBase takes care to
delete the last /.
I think that if fixDocBase does something to strings like ../../../ it will
break the results returned from RequestUtil.normalize, i.e. deleting the
last / and the normalizing it will produce path ending with /.. which will
once again not work.
The behaviour of org.apache.tomcat.util.http.RequestUtil is quite similar
but different from the one getting unique file path (Path.normalize). And
then if (!canPath.equals(absPath)) is comparing two quite similar things
,but not the same "things". I think that with java.io.File APIs there's
nothing that can give the needed behaviour (the getCanonicalPath will
resolve symlinks)
Best Regards,
Dimitar Valov
On Tue, Apr 19, 2016 at 10:42 PM, Mark Thomas <ma...@apache.org> wrote:
On 19/04/2016 19:38, Dimitar Valov wrote:
All static resources such as index.html will not be found when
application
is added with <Context path="/" docBase="../../../"/>, for example tomcat
is put inside the application's META-INF.
I've drilled down that the AbstractFileResourceSet class is responsible
for
this behaviour, inside the protected final File file(String name, boolean
mustExist) method. There absoluteBase and canonicalBase (absolute, unique
with resolved symlinks) are used to determine if the file should be
accessed based on a case sensitivity check.
Everything works fine if the docBase is not just path like ../../.././../
but has an actual directory at the end, like ../../../web-app. However in
this edgy case the difference appears since the behaviour of
the org.apache.tomcat.util.http.RequestUtil.normalize method has slightly
different behavior than the File.getCanonicalPath.
System.out.println(RequestUtil.normalize("/base/1/2/3/../../../"));
/base/
System.out.println(RequestUtil.normalize("/base/1/2/3/../../.."));
/base/1/..
System.out.println(new File("/base/1/2/3/../../../").getCanonicalPath());
/base
The added /from RequestUtil breakes the logic inside the file method,
since
when doing the substring operation inside AbstractFileResourceSet.file
method it may or may not have a trailing /. In such situation <whatever
path>/index.html substring with the absoluteBase becomes ndex.html. At
the
end the file method returns from here:
if (!canPath.equals(absPath)) //
!"index.html".equals("ndex.html")
=> true
return null;
The RequestUtil comes from the http packages and follows their
conventions
when normalizing the path, so an obvious way to fix this is to add and if
statement after the normalization
inside AbstractFileResourceSet.initInternal.
this.absoluteBase = normalize(absolutePath);
if (this.absoluteBase.endsWith("/")) {
this.absoluteBase = this.absoluteBase.substring(0,
this.absoluteBase.length() - 1);
}
With Java 7 instead of using the RequestUtil for normalization, Path
java.nio.file.Path.normalize() can accomplish the correct thing.
Do you think that is something that can be fixed? Maybe the above is not
the best approach, however it's the least invasive.
We need to be very careful here since this is security sensitive.
Given that normalize handles URIs, there is an argument for slightly
different handling of trailing '/'. I'm currently of the view (subject
to the results of some local tests I am running) that if the input ends
in '/', so should the output. If the input doesn't end in '/' neither
should the output unless the output is the string "/".
The end result is likely to be that docBase values should not end in "/".
Mark
---------------------------------------------------------------------
To unsubscribe, e-mail: users-unsubscr...@tomcat.apache.org
For additional commands, e-mail: users-h...@tomcat.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: users-unsubscr...@tomcat.apache.org
For additional commands, e-mail: users-h...@tomcat.apache.org
