From e56e54565804991c62ec76dad385d2bdda8972a7 Mon Sep 17 00:00:00 2001
From: Stefan Bodewig <bodewig@apache.org>
Date: Sat, 21 Apr 2018 19:55:02 +0200
Subject: [PATCH] unzip and friends could monitor where they write more closely

---
 WHATSNEW                                      |  13 +++++
 .../org/apache/tools/ant/taskdefs/Expand.java |  35 ++++++++++++-
 src/tests/antunit/taskdefs/unzip-test.xml     |  46 ++++++++++++++++++
 .../taskdefs/zip/direscape-absolute.zip       | Bin 0 -> 332 bytes
 src/tests/antunit/taskdefs/zip/direscape.zip  | Bin 0 -> 332 bytes
 5 files changed, 92 insertions(+), 2 deletions(-)
 create mode 100644 src/tests/antunit/taskdefs/zip/direscape-absolute.zip
 create mode 100644 src/tests/antunit/taskdefs/zip/direscape.zip

diff --git a/WHATSNEW b/WHATSNEW
index a40c4ed3f..3041bc5da 100644
--- a/WHATSNEW
+++ b/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:
 -----------
 
diff --git a/src/main/org/apache/tools/ant/taskdefs/Expand.java b/src/main/org/apache/tools/ant/taskdefs/Expand.java
index a586556bd..a3a27452d 100644
--- a/src/main/org/apache/tools/ant/taskdefs/Expand.java
+++ b/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 @@ public class Expand extends Task {
                                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 @@ 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);
+                return;
+        }
+
         try {
             if (!overwrite && f.exists()
                 && f.lastModified() >= entryDate.getTime()) {
@@ -533,4 +543,25 @@ public class Expand extends Task {
         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;
+    }
+
 }
diff --git a/src/tests/antunit/taskdefs/unzip-test.xml b/src/tests/antunit/taskdefs/unzip-test.xml
index b2c2105dd..a220bc186 100644
--- a/src/tests/antunit/taskdefs/unzip-test.xml
+++ b/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>
diff --git a/src/tests/antunit/taskdefs/zip/direscape-absolute.zip b/src/tests/antunit/taskdefs/zip/direscape-absolute.zip
new file mode 100644
index 0000000000000000000000000000000000000000..0bae4aaf1f693a73c2c756070f10158f498577ed
GIT binary patch
literal 332
zcmWIWW@h1H0D+ciQ+>b;D8b7h!=PW1TcBT(T3nKnS)?Bt!pXpFo^v}&B=2@qX$3a}
zBg+eB1_m$@fYTU0bYl|X=6C_k!Z3%CNsbwpgCw9%6JU7j2x6i+j}_uP5{%?yfEmfa
cu%yu)-AJHQahb`=266xs5H1JO{U8nl095}yUjP6A

literal 0
HcmV?d00001

diff --git a/src/tests/antunit/taskdefs/zip/direscape.zip b/src/tests/antunit/taskdefs/zip/direscape.zip
new file mode 100644
index 0000000000000000000000000000000000000000..63cefd2d82cc01471413cc80f3d6c384ebe5b34e
GIT binary patch
literal 332
zcmWIWW@h1H0D+ciQ+>b;D8b7h!=R_9Uy@o}l9^XfTB08s!pXpFo^v}&B=2@qX$3a}
zBg+eB1_m$@fYTU0bYl|X=6C_k!Z3%CNsbwpgCw9%6JU7j2x6i+j}_uP5{%?yfEmfa
cu%yu)-AJHQahb`=266xs5H1JO{U8nl0KQ~AlK=n!

literal 0
HcmV?d00001