diff --git a/core/src/main/java/hudson/FilePath.java b/core/src/main/java/hudson/FilePath.java index 0bcc31c32b97..089d8f8ff8e5 100644 --- a/core/src/main/java/hudson/FilePath.java +++ b/core/src/main/java/hudson/FilePath.java @@ -598,7 +598,7 @@ private void unzip(File dir, File zipFile) throws IOException { while (entries.hasMoreElements()) { ZipEntry e = entries.nextElement(); File f = new File(dir, e.getName()); - if (!f.toPath().normalize().startsWith(dir.toPath())) { + if (!f.getCanonicalPath().startsWith(dir.getCanonicalPath())) { throw new IOException( "Zip " + zipFile.getPath() + " contains illegal file name that breaks out of the target directory: " + e.getName()); } diff --git a/test/src/test/java/hudson/FilePathTest.java b/test/src/test/java/hudson/FilePathTest.java index bc5dfd3f4d06..abf6d377ccc2 100644 --- a/test/src/test/java/hudson/FilePathTest.java +++ b/test/src/test/java/hudson/FilePathTest.java @@ -29,9 +29,14 @@ import static org.hamcrest.Matchers.*; import org.junit.Test; import static org.junit.Assert.*; +import static org.junit.Assume.*; import org.junit.Rule; import org.jvnet.hudson.test.Issue; import org.jvnet.hudson.test.JenkinsRule; +import org.jvnet.hudson.test.recipes.LocalData; +import java.io.File; +import java.io.IOException; +import java.lang.reflect.Method; public class FilePathTest { @@ -56,4 +61,160 @@ public void listGlob() throws Exception { } } + @Test + @Issue("JENKINS-32778") + @LocalData("zip_with_relative") + public void zipRelativePathHandledCorrectly() throws Exception { + assumeTrue(Functions.isWindows()); + FilePath zipFile = r.jenkins.getRootPath().child("zip-with-folder.zip"); + FilePath targetLocation = r.jenkins.getRootPath().child("unzip-target"); + + FilePath simple1 = targetLocation.child("simple1.txt"); + FilePath simple2 = targetLocation.child("child").child("simple2.txt"); + + assertThat(simple1.exists(), is(false)); + assertThat(simple2.exists(), is(false)); + + zipFile.unzip(targetLocation); + + assertThat(simple1.exists(), is(true)); + assertThat(simple2.exists(), is(true)); + } + + @Test + @Issue("JENKINS-32778") + @LocalData("zip_with_relative") + public void zipAbsolutePathHandledCorrectly_win() throws Exception { + assumeTrue(Functions.isWindows()); + + // this special zip contains a ..\..\ [..] \..\Temp\evil.txt + FilePath zipFile = r.jenkins.getRootPath().child("zip-slip-win.zip"); + + FilePath targetLocation = r.jenkins.getRootPath().child("unzip-target"); + + FilePath good = targetLocation.child("good.txt"); + + assertThat(good.exists(), is(false)); + + try { + zipFile.unzip(targetLocation); + fail("The evil.txt should have triggered an exception"); + } + catch(IOException e){ + assertThat(e.getMessage(), containsString("contains illegal file name that breaks out of the target directory")); + } + + // as the unzip operation failed, the good.txt was potentially unzipped + // but we need to make sure that the evil.txt is not there + File evil = new File(r.jenkins.getRootDir(), "..\\..\\..\\..\\..\\..\\..\\..\\..\\..\\..\\..\\..\\..\\..\\..\\..\\..\\..\\..\\..\\..\\..\\..\\..\\..\\..\\..\\..\\..\\..\\..\\..\\..\\..\\..\\..\\..\\..\\..\\Temp\\evil.txt"); + assertThat(evil.exists(), is(false)); + } + + @Test + @Issue("JENKINS-32778") + @LocalData("zip_with_relative") + public void zipAbsolutePathHandledCorrectly_unix() throws Exception { + assumeFalse(Functions.isWindows()); + + // this special zip contains a ../../../ [..] /../tmp/evil.txt + FilePath zipFile = r.jenkins.getRootPath().child("zip-slip.zip"); + + FilePath targetLocation = r.jenkins.getRootPath().child("unzip-target"); + + FilePath good = targetLocation.child("good.txt"); + + assertThat(good.exists(), is(false)); + + try { + zipFile.unzip(targetLocation); + fail("The evil.txt should have triggered an exception"); + } + catch(IOException e){ + assertThat(e.getMessage(), containsString("contains illegal file name that breaks out of the target directory")); + } + + // as the unzip operation failed, the good.txt was potentially unzipped + // but we need to make sure that the evil.txt is not there + File evil = new File(r.jenkins.getRootDir(), "../../../../../../../../../../../../../../../../../../../../../../../../../../../../../../../../../../../../../../../../tmp/evil.txt"); + assertThat(evil.exists(), is(false)); + } + + @Test + @Issue("JENKINS-32778") + @LocalData("zip_with_relative") + public void zipRelativePathHandledCorrectly_oneUp() throws Exception { + // internal structure: + // ../simple3.txt + // child/simple2.txt + // simple1.txt + FilePath zipFile = r.jenkins.getRootPath().child("zip-rel-one-up.zip"); + FilePath targetLocation = r.jenkins.getRootPath().child("unzip-target"); + + FilePath simple3 = targetLocation.getParent().child("simple3.txt"); + + assertThat(simple3.exists(), is(false)); + + try { + zipFile.unzip(targetLocation); + fail("The ../simple3.txt should have triggered an exception"); + } + catch(IOException e){ + assertThat(e.getMessage(), containsString("contains illegal file name that breaks out of the target directory")); + } + + assertThat(simple3.exists(), is(false)); + } + + @Test + @Issue("XXX") + @LocalData("zip_with_relative") + public void zipTarget_regular() throws Exception { + assumeTrue(Functions.isWindows()); + File zipFile = new File(r.jenkins.getRootDir(), "zip-with-folder.zip"); + File targetLocation = new File(r.jenkins.getRootDir(), "unzip-target"); + FilePath targetLocationFP = r.jenkins.getRootPath().child("unzip-target"); + + FilePath simple1 = targetLocationFP.child("simple1.txt"); + FilePath simple2 = targetLocationFP.child("child").child("simple2.txt"); + + assertThat(simple1.exists(), is(false)); + assertThat(simple2.exists(), is(false)); + + Method unzipPrivateMethod; + unzipPrivateMethod = FilePath.class.getDeclaredMethod("unzip", File.class, File.class); + unzipPrivateMethod.setAccessible(true); + + FilePath fp = new FilePath(new File(".")); + unzipPrivateMethod.invoke(fp, targetLocation, zipFile); + + assertThat(simple1.exists(), is(true)); + assertThat(simple2.exists(), is(true)); + } + + @Test + @Issue("XXX") + @LocalData("zip_with_relative") + public void zipTarget_relative() throws Exception { + assumeTrue(Functions.isWindows()); + File zipFile = new File(r.jenkins.getRootDir(), "zip-with-folder.zip"); + // the main difference is here, the ./ + File targetLocation = new File(r.jenkins.getRootDir(), "./unzip-target"); + FilePath targetLocationFP = r.jenkins.getRootPath().child("unzip-target"); + + FilePath simple1 = targetLocationFP.child("simple1.txt"); + FilePath simple2 = targetLocationFP.child("child").child("simple2.txt"); + + assertThat(simple1.exists(), is(false)); + assertThat(simple2.exists(), is(false)); + + Method unzipPrivateMethod; + unzipPrivateMethod = FilePath.class.getDeclaredMethod("unzip", File.class, File.class); + unzipPrivateMethod.setAccessible(true); + + FilePath fp = new FilePath(new File(".")); + unzipPrivateMethod.invoke(fp, targetLocation, zipFile); + + assertThat(simple1.exists(), is(true)); + assertThat(simple2.exists(), is(true)); + } } diff --git a/test/src/test/resources/hudson/FilePathTest/zip_with_relative/zip-rel-one-up.zip b/test/src/test/resources/hudson/FilePathTest/zip_with_relative/zip-rel-one-up.zip new file mode 100644 index 000000000000..7baedb7cf3ce Binary files /dev/null and b/test/src/test/resources/hudson/FilePathTest/zip_with_relative/zip-rel-one-up.zip differ diff --git a/test/src/test/resources/hudson/FilePathTest/zip_with_relative/zip-slip-win.zip b/test/src/test/resources/hudson/FilePathTest/zip_with_relative/zip-slip-win.zip new file mode 100644 index 000000000000..3474c88bec74 Binary files /dev/null and b/test/src/test/resources/hudson/FilePathTest/zip_with_relative/zip-slip-win.zip differ diff --git a/test/src/test/resources/hudson/FilePathTest/zip_with_relative/zip-slip.zip b/test/src/test/resources/hudson/FilePathTest/zip_with_relative/zip-slip.zip new file mode 100644 index 000000000000..38b3f499de01 Binary files /dev/null and b/test/src/test/resources/hudson/FilePathTest/zip_with_relative/zip-slip.zip differ diff --git a/test/src/test/resources/hudson/FilePathTest/zip_with_relative/zip-with-folder.zip b/test/src/test/resources/hudson/FilePathTest/zip_with_relative/zip-with-folder.zip new file mode 100644 index 000000000000..b76cfb679181 Binary files /dev/null and b/test/src/test/resources/hudson/FilePathTest/zip_with_relative/zip-with-folder.zip differ