import sources
This commit is contained in:
parent
5603a29473
commit
2001c2e615
1
.gitignore
vendored
1
.gitignore
vendored
@ -1 +1,2 @@
|
||||
SOURCES/apache-ant-1.9.4-src.tar.bz2
|
||||
/apache-ant-1.10.7-src.tar.bz2
|
||||
|
@ -1,235 +0,0 @@
|
||||
From 89a75b024e45f687b1152fbb89ea47f7cbad75ce Mon Sep 17 00:00:00 2001
|
||||
From: Michael Simacek <msimacek@redhat.com>
|
||||
Date: Tue, 26 Jun 2018 15:17:33 +0200
|
||||
Subject: [PATCH] Fix arbitrary file write vulnerability
|
||||
|
||||
---
|
||||
WHATSNEW | 18 +++++++
|
||||
manual/Tasks/unzip.html | 12 ++++-
|
||||
.../org/apache/tools/ant/taskdefs/Expand.java | 37 ++++++++++++--
|
||||
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
|
||||
6 files changed, 109 insertions(+), 4 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 1e9b3398b..1ff99fc98 100644
|
||||
--- a/WHATSNEW
|
||||
+++ b/WHATSNEW
|
||||
@@ -1,3 +1,21 @@
|
||||
+Backported changes
|
||||
+==================
|
||||
+
|
||||
+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
|
||||
+ no longer 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.
|
||||
+ In addition stripAbsolutePathSpec is now true by default.
|
||||
+ Based on a recommendation by the Snyk Security Research Team.
|
||||
+
|
||||
Changes from Ant 1.9.3 TO Ant 1.9.4
|
||||
===================================
|
||||
|
||||
diff --git a/manual/Tasks/unzip.html b/manual/Tasks/unzip.html
|
||||
index 02df7acf9..a5fc40fd3 100644
|
||||
--- a/manual/Tasks/unzip.html
|
||||
+++ b/manual/Tasks/unzip.html
|
||||
@@ -125,7 +125,8 @@ archive.</p>
|
||||
Note that this changes the entry's name before applying
|
||||
include/exclude patterns and before using the nested mappers (if
|
||||
any). <em>since Ant 1.8.0</em></td>
|
||||
- <td valign="top" align="center">No, defaults to false</td>
|
||||
+ <td valign="top" align="center">No, defaults to true since 1.9.12
|
||||
+ (used to defaukt to false prior to that)</td>
|
||||
</tr>
|
||||
<tr>
|
||||
<td valign="top">scanForUnicodeExtraFields</td>
|
||||
@@ -137,6 +138,15 @@ archive.</p>
|
||||
zip task page</a></td>
|
||||
<td align="center" valign="top">No, defaults to true</td>
|
||||
</tr>
|
||||
+ <tr>
|
||||
+ <td valign="top">allowFilesToEscapeDest</td>
|
||||
+ <td valign="top">Whether to allow the extracted file or directory
|
||||
+ to be outside of the dest directory.
|
||||
+ <em>since Ant 1.9.12</em></td>
|
||||
+ <td valign="top" align="center">No, defaults to false unless
|
||||
+ stripAbsolutePathSpec is true and the entry's name starts with a leading
|
||||
+ path spec.</td>
|
||||
+ </tr>
|
||||
</table>
|
||||
<h3>Examples</h3>
|
||||
<pre>
|
||||
diff --git a/src/main/org/apache/tools/ant/taskdefs/Expand.java b/src/main/org/apache/tools/ant/taskdefs/Expand.java
|
||||
index 8722e2402..ac5287cb8 100644
|
||||
--- a/src/main/org/apache/tools/ant/taskdefs/Expand.java
|
||||
+++ b/src/main/org/apache/tools/ant/taskdefs/Expand.java
|
||||
@@ -67,8 +67,9 @@ public class Expand extends Task {
|
||||
private Union resources = new Union();
|
||||
private boolean resourcesSpecified = false;
|
||||
private boolean failOnEmptyArchive = false;
|
||||
- private boolean stripAbsolutePathSpec = false;
|
||||
+ private boolean stripAbsolutePathSpec = true;
|
||||
private boolean scanForUnicodeExtraFields = true;
|
||||
+ private Boolean allowFilesToEscapeDest = null;
|
||||
|
||||
public static final String NATIVE_ENCODING = "native-encoding";
|
||||
|
||||
@@ -240,14 +241,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)
|
||||
@@ -313,6 +317,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()) {
|
||||
@@ -508,4 +518,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..bdf5f61e1 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="testEntriesCanEscapeDestViaAbsolutePathIfPermitted"
|
||||
+ depends="-can-write-to-tmp?" if="can-write-to-tmp!">
|
||||
+ <unzip src="zip/direscape-absolute.zip" dest="${output}"
|
||||
+ stripAbsolutePathSpec="false"/>
|
||||
+ <au:assertFileExists file="/tmp/testdir/a"/>
|
||||
+ </target>
|
||||
+
|
||||
+ <target name="testEntriesDontEscapeDestViaAbsolutePathByDefault"
|
||||
+ depends="-can-write-to-tmp?" if="can-write-to-tmp!">
|
||||
+ <unzip src="zip/direscape-absolute.zip" dest="${output}"/>
|
||||
+ <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 000000000..0bae4aaf1
|
||||
--- /dev/null
|
||||
+++ b/src/tests/antunit/taskdefs/zip/direscape-absolute.zip
|
||||
@@ -0,0 +1,5 @@
|
||||
+PK
|
||||
+ |