unzip and friends could monitor where they write more closely

Author: bodewig

WHATSNEW

@@ -1,6 +1,19 @@
 Changes from Ant 1.9.11 TO Ant 1.9.12
 =====================================
 
+Changes that could break older environments:
+-------------------------------------------
+
+ * <unzip>, <unjar> and <untar> will no longer extract entries whose
+   names would make the created files be placed outside of the
+   destination directory anymore by default. A new attribute
+   allowFilesToEscapeDest can be used to override the behavior.
+   Another special case is when stripAbsolutePathSpec is false (which
+   still is the default) and the entry's name starts with a
+   (back)slash and allowFilesToEscapeDest hasn't been specified
+   explicitly, in this case the file may be created outside of the
+   dest directory as well.
+
 Fixed bugs:
 -----------
 

src/main/org/apache/tools/ant/taskdefs/Expand.java

@@ -69,6 +69,7 @@ public class Expand extends Task {
     private boolean failOnEmptyArchive = false;
     private boolean stripAbsolutePathSpec = false;
     private boolean scanForUnicodeExtraFields = true;
+    private Boolean allowFilesToEscapeDest = null;
 
     public static final String NATIVE_ENCODING = "native-encoding";
 
@@ -259,14 +260,17 @@ protected void extractFile(FileUtils fileUtils, File srcF, File dir,
                                boolean isDirectory, FileNameMapper mapper)
                                throws IOException {
 
-        if (stripAbsolutePathSpec && entryName.length() > 0
+        final boolean entryNameStartsWithPathSpec = entryName.length() > 0
             && (entryName.charAt(0) == File.separatorChar
                 || entryName.charAt(0) == '/'
-                || entryName.charAt(0) == '\\')) {
+                || entryName.charAt(0) == '\\');
+        if (stripAbsolutePathSpec && entryNameStartsWithPathSpec) {
             log("stripped absolute path spec from " + entryName,
                 Project.MSG_VERBOSE);
             entryName = entryName.substring(1);
         }
+        boolean allowedOutsideOfDest = Boolean.TRUE == getAllowFilesToEscapeDest()
+            || null == getAllowFilesToEscapeDest() && !stripAbsolutePathSpec && entryNameStartsWithPathSpec;
 
         if (patternsets != null && patternsets.size() > 0) {
             String name = entryName.replace('/', File.separatorChar)
@@ -332,6 +336,12 @@ protected void extractFile(FileUtils fileUtils, File srcF, File dir,
             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);
+                return;
+        }
+
         try {
             if (!overwrite && f.exists()
                 && f.lastModified() >= entryDate.getTime()) {
@@ -533,4 +543,25 @@ public boolean getScanForUnicodeExtraFields() {
         return scanForUnicodeExtraFields;
     }
 
+    /**
+     * Whether to allow the extracted file or directory to be outside of the dest directory.
+     *
+     * @param b the flag
+     * @since Ant 1.9.12
+     */
+    public void setAllowFilesToEscapeDest(boolean b) {
+        allowFilesToEscapeDest = b;
+    }
+
+    /**
+     * Whether to allow the extracted file or directory to be outside of the dest directory.
+     *
+     * @return {@code null} if the flag hasn't been set explicitly,
+     * otherwise the value set by the user.
+     * @since Ant 1.9.12
+     */
+    public Boolean getAllowFilesToEscapeDest() {
+        return allowFilesToEscapeDest;
+    }
+
 }

src/tests/antunit/taskdefs/unzip-test.xml

@@ -24,6 +24,10 @@
     <mkdir dir="${output}" />
   </target>
 
+  <target name="tearDown" depends="antunit-base.tearDown">
+    <delete dir="/tmp/testdir"/>
+  </target>
+
   <target name="testFailureOnBrokenCentralDirectoryStructure">
     <au:expectfailure
        expectedmessage="central directory is empty, can't expand corrupt archive.">
@@ -67,4 +71,46 @@
     <!-- failed on Windows and other OSes with implicit file locking -->
     <au:assertFileDoesntExist file="${input}/test.zip"/>
   </target>
+
+  <target name="testEntriesDontEscapeDestByDefault">
+    <mkdir dir="${input}/"/>
+    <mkdir dir="${output}/"/>
+    <unzip src="zip/direscape.zip" dest="${output}"/>
+    <au:assertFileDoesntExist file="${input}/a"/>
+  </target>
+
+  <target name="testEntriesCanEscapeDestIfRequested">
+    <mkdir dir="${input}/"/>
+    <mkdir dir="${output}/"/>
+    <unzip src="zip/direscape.zip" dest="${output}" allowFilesToEscapeDest="true"/>
+    <au:assertFileExists file="${input}/a"/>
+  </target>
+
+  <target name="-can-write-to-tmp?">
+    <mkdir dir="${input}"/>
+    <echo file="${input}/A.java"><![CDATA[
+public class A {
+    public static void main(String[] args) {
+        new java.io.File("/tmp/testdir/").mkdirs();
+    }
+}
+]]></echo>
+    <mkdir dir="${output}"/>
+    <javac srcdir="${input}" destdir="${output}"/>
+    <java classname="A" classpath="${output}"/>
+    <available property="can-write-to-tmp!" file="/tmp/testdir/"/>
+  </target>
+
+  <target name="testEntriesCanEscapeDestViaAbsolutePathByDefault"
+          depends="-can-write-to-tmp?" if="can-write-to-tmp!">
+    <unzip src="zip/direscape-absolute.zip" dest="${output}"/>
+    <au:assertFileExists file="/tmp/testdir/a"/>
+  </target>
+
+  <target name="testEntriesDontEscapeDestViaAbsolutePathIfProhibited"
+          depends="-can-write-to-tmp?" if="can-write-to-tmp!">
+    <unzip src="zip/direscape-absolute.zip" dest="${output}"
+           allowFilesToEscapeDest="false"/>
+    <au:assertFileDoesntExist file="/tmp/testdir/a"/>
+  </target>
 </project>