Prevents unzipping files outside of the target folder

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<File> unzipTo(File self, File destination) {
+	public static Collection<File> 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());

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");
+		}
+	}
+}