2017-09-20 20:09 GMT+03:00 Konstantin Kolinko <knst.koli...@gmail.com>:
> 2017-09-20 15:23 GMT+03:00 <ma...@apache.org>:
>> Author: markt
>> Date: Wed Sep 20 12:23:44 2017
>> New Revision: 1809011
>>
>> URL: http://svn.apache.org/viewvc?rev=1809011&view=rev
>> Log:
>> Fix https://bz.apache.org/bugzilla/show_bug.cgi?id=61542
>> Partial fix for CVE-2017-12617
>> This moves a check from the Default servlet where it applied to GET, POST,
>> HEAD and OPTIONS to the resources implementation where it applies to any
>> method that expects the resource to exist (e.g.DELETE)
>> Still need to address the case where the resource does not exist (e.g. PUT)
>>
>> Modified:
>> tomcat/trunk/java/org/apache/catalina/servlets/DefaultServlet.java
>>
>> tomcat/trunk/java/org/apache/catalina/webresources/AbstractFileResourceSet.java
>>
>> tomcat/trunk/test/org/apache/catalina/webresources/AbstractTestResourceSet.java
>>
>> Modified: tomcat/trunk/java/org/apache/catalina/servlets/DefaultServlet.java
>> URL:
>> http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/catalina/servlets/DefaultServlet.java?rev=1809011&r1=1809010&r2=1809011&view=diff
>> ==============================================================================
>> --- tomcat/trunk/java/org/apache/catalina/servlets/DefaultServlet.java
>> (original)
>> +++ tomcat/trunk/java/org/apache/catalina/servlets/DefaultServlet.java Wed
>> Sep 20 12:23:44 2017
>> @@ -820,20 +820,6 @@ public class DefaultServlet extends Http
>> return;
>> }
>>
>> - // If the resource is not a collection, and the resource path
>> - // ends with "/" or "\", return NOT FOUND
>> - if (resource.isFile() && (path.endsWith("/") ||
>> path.endsWith("\\"))) {
>> - // Check if we're included so we can return the appropriate
>> - // missing resource name in the error
>> - String requestUri = (String) request.getAttribute(
>> - RequestDispatcher.INCLUDE_REQUEST_URI);
>> - if (requestUri == null) {
>> - requestUri = request.getRequestURI();
>> - }
>> - response.sendError(HttpServletResponse.SC_NOT_FOUND,
>> requestUri);
>> - return;
>> - }
>> -
>> boolean included = false;
>> // Check if the conditions specified in the optional If headers are
>> // satisfied.
>>
>> Modified:
>> tomcat/trunk/java/org/apache/catalina/webresources/AbstractFileResourceSet.java
>> URL:
>> http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/catalina/webresources/AbstractFileResourceSet.java?rev=1809011&r1=1809010&r2=1809011&view=diff
>> ==============================================================================
>> ---
>> tomcat/trunk/java/org/apache/catalina/webresources/AbstractFileResourceSet.java
>> (original)
>> +++
>> tomcat/trunk/java/org/apache/catalina/webresources/AbstractFileResourceSet.java
>> Wed Sep 20 12:23:44 2017
>> @@ -57,6 +57,14 @@ public abstract class AbstractFileResour
>> name = "";
>> }
>> File file = new File(fileBase, name);
>> +
>> + // If the requested names ends in '/', the Java File API will
>> return a
>> + // matching file if one exists. This isn't what we want as it is not
>> + // consistent with the Servlet spec rules for request mapping.
>> + if (file.isFile() && name.endsWith("/")) {
>
> I think this has to be
>
> if (name.endsWith("/") && !file.isDirectory())
>
> Two concerns:
> 1) performance: do in-memory checks first and I/O performing ones later
> 2) "isFile()" tests whether "this is a normal file". The definition of
> "normal file" is OS-dependent.
My bad:
"if (... !file.isDirectory())" check would be wrong
I missed that isFile()/isDirectory() also checks for existence, and
there is "mustExist" flag in this method.
Thinking about
if (name.endsWith("/") && file.exists() && !file.isDirectory())
it looks odd.
Falling back to
if (name.endsWith("/") && file.isFile()) {
Best regards,
Konstantin Kolinko
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org
