diff --git a/train-docs/src/main/java/org/springframework/cloud/internal/ZipCategory.java b/train-docs/src/main/java/org/springframework/cloud/internal/ZipCategory.java index b7e19fe..dccf60b 100644 --- a/train-docs/src/main/java/org/springframework/cloud/internal/ZipCategory.java +++ b/train-docs/src/main/java/org/springframework/cloud/internal/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; @@ -53,7 +54,7 @@ final class ZipCategory { * will be unzipped to. * @return a {@link Collection} of unzipped {@link File} objects. */ - static Collection unzipTo(File self, File destination) { + public static Collection unzipTo(File self, File destination) { checkUnzipDestination(destination); // if destination directory is not given, we'll fall back to the parent directory // of 'self' @@ -65,14 +66,23 @@ 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/train-docs/src/test/java/org/springframework/cloud/internal/ZipCategoryTests.java b/train-docs/src/test/java/org/springframework/cloud/internal/ZipCategoryTests.java new file mode 100644 index 0000000..436de29 --- /dev/null +++ b/train-docs/src/test/java/org/springframework/cloud/internal/ZipCategoryTests.java @@ -0,0 +1,58 @@ +/* + * Copyright 2013-2020 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.springframework.cloud.internal; + +import java.io.File; +import java.nio.file.Files; + +import org.assertj.core.api.BDDAssertions; +import org.junit.jupiter.api.Assertions; +import org.junit.jupiter.api.Test; + +class ZipCategoryTests { + + @Test + void should_unzip_a_file_to_the_specified_location() throws Exception { + // given: + File zipFile = new File(ZipCategoryTests.class.getClassLoader().getResource("file.zip").toURI()); + File tempDir = Files.createTempDirectory("foo").toFile(); + tempDir.deleteOnExit(); + // when: + ZipCategory.unzipTo(zipFile, tempDir); + // then: + BDDAssertions.then(tempDir.listFiles()) + .hasOnlyOneElementSatisfying(file -> { + BDDAssertions.then(file).hasName("file.txt") + .hasContent("test"); + }); + } + + @Test + void should_not_allow_malicious_traversal() throws Exception { + // given: + File zipFile = new File(ZipCategoryTests.class.getClassLoader().getResource("zip/zip-malicious-traversal.zip").toURI()); + File tempDir = Files.createTempDirectory("foo").toFile(); + tempDir.deleteOnExit(); + // when: + try { + ZipCategory.unzipTo(zipFile, tempDir); + Assertions.fail("Should throw exception"); + } catch (Exception e) { + BDDAssertions.then(e.getCause()).hasMessageContaining("is trying to leave the target output directory"); + } + } +} diff --git a/train-docs/src/test/resources/file.zip b/train-docs/src/test/resources/file.zip new file mode 100644 index 0000000..3da3a91 Binary files /dev/null and b/train-docs/src/test/resources/file.zip differ diff --git a/train-docs/src/test/resources/zip/zip-malicious-traversal.zip b/train-docs/src/test/resources/zip/zip-malicious-traversal.zip new file mode 100644 index 0000000..38b3f49 Binary files /dev/null and b/train-docs/src/test/resources/zip/zip-malicious-traversal.zip differ