diff --git a/WHATSNEW b/WHATSNEW index b34197e39..b950a35bc 100644 --- a/WHATSNEW +++ b/WHATSNEW @@ -9,6 +9,12 @@ Fixed bugs: an exception. Bugzilla Report 62499 + * the new allowFilesToEscapeDest didn't work when set to false and + archive entries contained relative paths with so many ".." + segnments that the resulting path would go beyond the file system + root. + Bugzilla Report 62502 + Changes from Ant 1.10.3 TO Ant 1.10.4 ===================================== diff --git a/src/main/org/apache/tools/ant/taskdefs/Expand.java b/src/main/org/apache/tools/ant/taskdefs/Expand.java index 3b62ea820..b526a78de 100644 --- a/src/main/org/apache/tools/ant/taskdefs/Expand.java +++ b/src/main/org/apache/tools/ant/taskdefs/Expand.java @@ -332,9 +332,9 @@ public class Expand extends Task { mappedNames = new String[] {entryName}; } File f = fileUtils.resolveFile(dir, mappedNames[0]); - if (!allowedOutsideOfDest && !fileUtils.isLeadingPath(dir, f)) { - log("skipping " + entryName + " as its target " + f + " is outside of " - + dir + ".", Project.MSG_VERBOSE); + if (!allowedOutsideOfDest && !fileUtils.isLeadingPath(dir, f, true)) { + log("skipping " + entryName + " as its target " + f.getCanonicalPath() + + " is outside of " + dir.getCanonicalPath() + ".", Project.MSG_VERBOSE); return; } diff --git a/src/main/org/apache/tools/ant/util/FileUtils.java b/src/main/org/apache/tools/ant/util/FileUtils.java index 864db5e5b..34b5d9d0d 100644 --- a/src/main/org/apache/tools/ant/util/FileUtils.java +++ b/src/main/org/apache/tools/ant/util/FileUtils.java @@ -729,8 +729,12 @@ public class FileUtils { *
Unlike {@link File#getCanonicalPath()} this method + * specifically does not resolve symbolic links.
+ * + *If the path tries to go beyond the file system root (i.e. it + * contains more ".." segments than can be travelled up) the + * method will return the original path unchanged.
* * @param path the path to be normalized. * @return the normalized version of the path. @@ -1155,6 +1159,9 @@ public class FileUtils { /** * Removes a leading path from a second path. * + *This method uses {@link #normalize} under the covers and + * does not resolve symbolic links.
+ * * @param leading The leading path, must not be null, must be absolute. * @param path The path to remove from, must not be null, must be absolute. * @@ -1179,8 +1186,16 @@ public class FileUtils { /** * Learn whether one path "leads" another. + * + *This method uses {@link #normalize} under the covers and + * does not resolve symbolic links.
+ * + *If either path tries to go beyond the file system root + * (i.e. it contains more ".." segments than can be travelled up) + * the method will return false.
+ * * @param leading The leading path, must not be null, must be absolute. - * @param path The path to remove from, must not be null, must be absolute. + * @param path The path to check, must not be null, must be absolute. * @return true if path starts with leading; false otherwise. * @since Ant 1.7 */ @@ -1195,6 +1210,41 @@ public class FileUtils { if (!l.endsWith(File.separator)) { l += File.separator; } + // ensure "/foo/" is not considered a parent of "/foo/../../bar" + String up = File.separator + ".." + File.separator; + if (l.contains(up) || p.contains(up) || (p + File.separator).contains(up)) { + return false; + } + return p.startsWith(l); + } + + /** + * Learn whether one path "leads" another. + * + * @param leading The leading path, must not be null, must be absolute. + * @param path The path to check, must not be null, must be absolute. + * @param resolveSymlinks whether symbolic links shall be resolved + * prior to comparing the paths. + * @return true if path starts with leading; false otherwise. + * @since Ant 1.10.5 + * @throws IOException if resolveSymlinks is true and invoking + * getCanonicaPath on either argument throws an exception + */ + public boolean isLeadingPath(File leading, File path, boolean resolveSymlinks) + throws IOException { + if (!resolveSymlinks) { + return isLeadingPath(leading, path); + } + String l = leading.getCanonicalPath(); + String p = path.getCanonicalPath(); + if (l.equals(p)) { + return true; + } + // ensure that l ends with a / + // so we never think /foo was a parent directory of /foobar + if (!l.endsWith(File.separator)) { + l += File.separator; + } return p.startsWith(l); } diff --git a/src/tests/junit/org/apache/tools/ant/util/FileUtilsTest.java b/src/tests/junit/org/apache/tools/ant/util/FileUtilsTest.java index cdd68e148..b7d34457d 100644 --- a/src/tests/junit/org/apache/tools/ant/util/FileUtilsTest.java +++ b/src/tests/junit/org/apache/tools/ant/util/FileUtilsTest.java @@ -601,11 +601,53 @@ public class FileUtilsTest { } } + + @Test public void testGetDefaultEncoding() { // This just tests that the function does not blow up FILE_UTILS.getDefaultEncoding(); } + /** + * @see "https://bz.apache.org/bugzilla/show_bug.cgi?id=62502" + */ + @Test + public void isLeadingPathCannotBeFooledByTooManyDoubleDots() { + assertFalse(FILE_UTILS.isLeadingPath(new File("/foo"), new File("/foo/../../bar"))); + assertFalse(FILE_UTILS.isLeadingPath(new File("c:\\foo"), new File("c:\\foo\\..\\..\\bar"))); + assertFalse(FILE_UTILS.isLeadingPath(new File("/foo"), new File("/foo/../.."))); + } + + /** + * @see "https://bz.apache.org/bugzilla/show_bug.cgi?id=62502" + */ + @Test + public void isLeadingPathCanonicalVersionCannotBeFooledByTooManyDoubleDots() throws IOException { + assertFalse(FILE_UTILS.isLeadingPath(new File("/foo"), new File("/foo/../../bar"), true)); + assertFalse(FILE_UTILS.isLeadingPath(new File("c:\\foo"), new File("c:\\foo\\..\\..\\bar"), true)); + assertFalse(FILE_UTILS.isLeadingPath(new File("/foo"), new File("/foo/../.."), true)); + } + + @Test + public void isLeadingPathCanonicalVersionWorksAsExpectedOnUnix() throws IOException { + assumeFalse("Test doesn't run on DOS", Os.isFamily("dos")); + assertTrue(FILE_UTILS.isLeadingPath(new File("/foo"), new File("/foo/bar"), true)); + assertTrue(FILE_UTILS.isLeadingPath(new File("/foo"), new File("/foo/baz/../bar"), true)); + assertTrue(FILE_UTILS.isLeadingPath(new File("/foo"), new File("/foo/../foo/bar"), true)); + assertFalse(FILE_UTILS.isLeadingPath(new File("/foo"), new File("/foobar"), true)); + assertFalse(FILE_UTILS.isLeadingPath(new File("/foo"), new File("/bar"), true)); + } + + @Test + public void isLeadingPathCanonicalVersionWorksAsExpectedOnDos() throws IOException { + assumeTrue("Test only runs on DOS", Os.isFamily("dos")); + assertTrue(FILE_UTILS.isLeadingPath(new File("C:\\foo"), new File("C:\\foo\\bar"), true)); + assertTrue(FILE_UTILS.isLeadingPath(new File("C:\\foo"), new File("C:\\foo\\baz\\..\\bar"), true)); + assertTrue(FILE_UTILS.isLeadingPath(new File("C:\\foo"), new File("C:\\foo\\..\\foo\\bar"), true)); + assertFalse(FILE_UTILS.isLeadingPath(new File("C:\\foo"), new File("C:\\foobar"), true)); + assertFalse(FILE_UTILS.isLeadingPath(new File("C:\\foo"), new File("C:\\bar"), true)); + } + /** * adapt file separators to local conventions */