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

Reply via email to