diff --git a/spring-cloud-contract-stub-runner/src/main/java/org/springframework/cloud/contract/stubrunner/util/ZipCategory.java b/spring-cloud-contract-stub-runner/src/main/java/org/springframework/cloud/contract/stubrunner/util/ZipCategory.java index dde85eaa75..7340e75f54 100644 --- a/spring-cloud-contract-stub-runner/src/main/java/org/springframework/cloud/contract/stubrunner/util/ZipCategory.java +++ b/spring-cloud-contract-stub-runner/src/main/java/org/springframework/cloud/contract/stubrunner/util/ZipCategory.java @@ -25,6 +25,7 @@ import java.util.ArrayList; import java.util.Collection; import java.util.List; import java.util.zip.ZipEntry; +import java.util.zip.ZipException; import java.util.zip.ZipInputStream; import org.springframework.util.StreamUtils; @@ -65,14 +66,23 @@ public final class ZipCategory { try (ZipInputStream zipInput = new ZipInputStream(fileInputStream)) { for (ZipEntry entry = zipInput.getNextEntry(); entry != null; entry = zipInput.getNextEntry()) { if (!entry.isDirectory()) { - final File file = new File(destination, entry.getName()); - if (file.getParentFile() != null) { - file.getParentFile().mkdirs(); + final File destinationFile = new File(destination, entry.getName()); + /* + * If we see the relative traversal string of ".." we need to make sure + * that the outputdir + name doesn't leave the outputdir. + */ + String zipEntryName = entry.getName(); + if (!destinationFile.toPath().normalize().startsWith(destination.toPath())) { + throw new ZipException("The file " + zipEntryName + + " is trying to leave the target output directory of " + destination); } - try (OutputStream output = Files.newOutputStream(file.toPath())) { + if (destinationFile.getParentFile() != null) { + destinationFile.getParentFile().mkdirs(); + } + try (OutputStream output = Files.newOutputStream(destinationFile.toPath())) { StreamUtils.copy(zipInput, output); } - unzippedFiles.add(file); + unzippedFiles.add(destinationFile); } else { final File dir = new File(destination, entry.getName()); diff --git a/spring-cloud-contract-stub-runner/src/test/groovy/org/springframework/cloud/contract/stubrunner/util/ZipCategorySpec.groovy b/spring-cloud-contract-stub-runner/src/test/groovy/org/springframework/cloud/contract/stubrunner/util/ZipCategorySpec.groovy index 047a3ea4e4..9544779d7e 100644 --- a/spring-cloud-contract-stub-runner/src/test/groovy/org/springframework/cloud/contract/stubrunner/util/ZipCategorySpec.groovy +++ b/spring-cloud-contract-stub-runner/src/test/groovy/org/springframework/cloud/contract/stubrunner/util/ZipCategorySpec.groovy @@ -36,4 +36,17 @@ class ZipCategorySpec extends Specification { }?.text?.trim() == 'test' } + def 'should not allow malicious traversal'() throws Exception { + given: + File zipFile = new File(ZipCategorySpec.classLoader.getResource('zip/zip-malicious-traversal.zip').toURI()) + File tempDir = File.createTempDir() + tempDir.deleteOnExit() + when: + use(ZipCategory) { + zipFile.unzipTo(tempDir) + } + then: + Exception e = thrown() + e.getCause().getMessage().contains("is trying to leave the target output directory") + } } diff --git a/spring-cloud-contract-stub-runner/src/test/resources/zip/zip-malicious-traversal.zip b/spring-cloud-contract-stub-runner/src/test/resources/zip/zip-malicious-traversal.zip new file mode 100644 index 0000000000..38b3f499de Binary files /dev/null and b/spring-cloud-contract-stub-runner/src/test/resources/zip/zip-malicious-traversal.zip differ