|Summary:||FileDirContext validate not working properly for directories|
|Product:||Tomcat 7||Reporter:||Martin Drößler <m.droessler>|
|Component:||Catalina||Assignee:||Tomcat Developers Mailing List <dev>|
Description Martin Drößler 2018-06-27 15:14:21 UTC
Created attachment 35993 [details] Patch We encounter weird errors with our webapp since tomcat 7.0.82 A change in org.apache.naming.resources.FileDirContext from Mark Thomas at 28.09.17 at 13:54 introduced the wrong behaviour due to difference in how constructors of java.io.File work. e.g.: File f = new File("/tmp/"); is not the same as: File fp = new File("/tmp"); File f = new File(f, "/"); Thus, the changed check in the validate-method can result in absPath = "/" and canPath = "" ...and the check for equals fails, although its the same path! I attached a patch to reverse the handling of absPath and canPath.
Comment 1 Mark Thomas 2018-06-27 16:52:49 UTC
That patch, and the related patches, are part of the fix for security vulnerability CVE-2017-12617. You are going to need to provide a test case that demonstrates an incorrect behaviour before we are going to consider making any changes along the lines you request.
Comment 2 Martin Drößler 2018-06-28 07:36:18 UTC
Created attachment 35994 [details] TestCase I attached a TestCase to demonstrate the behaviour.
Comment 3 Mark Thomas 2018-06-28 10:42:34 UTC
Note: The provided test cases will fail on Windows. Note: testValidateFailure() only passes because it calls testValidateInternal(dummyFile, null) but it should be testValidateInternal(dummyFile, dummyFile) which currently fails. I don't see a way for this failure to occur in a standard directory deployment. There is code in FileDirContext that explicitly prevents this problem from occurring. It may be possible to trigger the issue with VirtualDirContext. I'll take a look. What are the steps to trigger this issue starting from a clean install of the latest Tomcat 7.0.x release? I suspect a fix, if required, is required somewhere other than where proposed but to be able be sure we need to be able to reproduce it.
Comment 4 Mark Thomas 2018-06-28 10:56:44 UTC
I can't see a way to trigger this with VirtualDirContext either. Awaiting reproduction steps from a clean Tomcat install.
Comment 5 Martin Drößler 2018-06-28 13:32:22 UTC
> Note: testValidateFailure() only passes because it calls > testValidateInternal(dummyFile, null) but it should be > testValidateInternal(dummyFile, dummyFile) which currently fails. Well, thats the whole point. It shouldn't pass. I didn't know that you prefer a failing test for the correct behaviour instead of a working test to demonstrate the wrong behaviour. > I can't see a way to trigger this with VirtualDirContext either. Well, I do! So let me show you: Compare the following lines of VirtualDirContext: 215 and 305 In the second one (part of the method "doLookup") the path variable is extended to have a trailing slash - as is should be! But in the other (part of method "file") it's not! You check for > name.startsWith(path + "/") but then do a > String res = name.substring(path.length()) which of course will result in "/" - which is then passed to > file = new File(resourcesDir, res); and this results in exactly the described behaviour! Do you really want me to implement a whole webapp to further demonstrate this obvious bug?
Comment 6 Mark Thomas 2018-06-30 20:40:26 UTC
Fixed in 7.0.x for 7.0.90 onwards.