Skip to content

Commit

Permalink
unzip and friends could monitor where they write more closely
Browse files Browse the repository at this point in the history
  • Loading branch information
bodewig committed Apr 21, 2018
1 parent c851c31 commit e56e545
Show file tree
Hide file tree
Showing 5 changed files with 92 additions and 2 deletions.
13 changes: 13 additions & 0 deletions WHATSNEW
@@ -1,6 +1,19 @@
Changes from Ant 1.9.11 TO Ant 1.9.12 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: Fixed bugs:
----------- -----------


Expand Down
35 changes: 33 additions & 2 deletions src/main/org/apache/tools/ant/taskdefs/Expand.java
Expand Up @@ -69,6 +69,7 @@ public class Expand extends Task {
private boolean failOnEmptyArchive = false; private boolean failOnEmptyArchive = false;
private boolean stripAbsolutePathSpec = false; private boolean stripAbsolutePathSpec = false;
private boolean scanForUnicodeExtraFields = true; private boolean scanForUnicodeExtraFields = true;
private Boolean allowFilesToEscapeDest = null;


public static final String NATIVE_ENCODING = "native-encoding"; public static final String NATIVE_ENCODING = "native-encoding";


Expand Down Expand Up @@ -259,14 +260,17 @@ protected void extractFile(FileUtils fileUtils, File srcF, File dir,
boolean isDirectory, FileNameMapper mapper) boolean isDirectory, FileNameMapper mapper)
throws IOException { throws IOException {


if (stripAbsolutePathSpec && entryName.length() > 0 final boolean entryNameStartsWithPathSpec = entryName.length() > 0
&& (entryName.charAt(0) == File.separatorChar && (entryName.charAt(0) == File.separatorChar
|| entryName.charAt(0) == '/' || entryName.charAt(0) == '/'
|| entryName.charAt(0) == '\\')) { || entryName.charAt(0) == '\\');
if (stripAbsolutePathSpec && entryNameStartsWithPathSpec) {
log("stripped absolute path spec from " + entryName, log("stripped absolute path spec from " + entryName,
Project.MSG_VERBOSE); Project.MSG_VERBOSE);
entryName = entryName.substring(1); entryName = entryName.substring(1);
} }
boolean allowedOutsideOfDest = Boolean.TRUE == getAllowFilesToEscapeDest()
|| null == getAllowFilesToEscapeDest() && !stripAbsolutePathSpec && entryNameStartsWithPathSpec;


if (patternsets != null && patternsets.size() > 0) { if (patternsets != null && patternsets.size() > 0) {
String name = entryName.replace('/', File.separatorChar) String name = entryName.replace('/', File.separatorChar)
Expand Down Expand Up @@ -332,6 +336,12 @@ protected void extractFile(FileUtils fileUtils, File srcF, File dir,
mappedNames = new String[] {entryName}; mappedNames = new String[] {entryName};
} }
File f = fileUtils.resolveFile(dir, mappedNames[0]); 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 { try {
if (!overwrite && f.exists() if (!overwrite && f.exists()
&& f.lastModified() >= entryDate.getTime()) { && f.lastModified() >= entryDate.getTime()) {
Expand Down Expand Up @@ -533,4 +543,25 @@ public boolean getScanForUnicodeExtraFields() {
return scanForUnicodeExtraFields; 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;
}

} }
46 changes: 46 additions & 0 deletions src/tests/antunit/taskdefs/unzip-test.xml
Expand Up @@ -24,6 +24,10 @@
<mkdir dir="${output}" /> <mkdir dir="${output}" />
</target> </target>


<target name="tearDown" depends="antunit-base.tearDown">
<delete dir="/tmp/testdir"/>
</target>

<target name="testFailureOnBrokenCentralDirectoryStructure"> <target name="testFailureOnBrokenCentralDirectoryStructure">
<au:expectfailure <au:expectfailure
expectedmessage="central directory is empty, can't expand corrupt archive."> expectedmessage="central directory is empty, can't expand corrupt archive.">
Expand Down Expand Up @@ -67,4 +71,46 @@
<!-- failed on Windows and other OSes with implicit file locking --> <!-- failed on Windows and other OSes with implicit file locking -->
<au:assertFileDoesntExist file="${input}/test.zip"/> <au:assertFileDoesntExist file="${input}/test.zip"/>
</target> </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> </project>
Binary file not shown.
Binary file added src/tests/antunit/taskdefs/zip/direscape.zip
Binary file not shown.

0 comments on commit e56e545

Please sign in to comment.