From eedb764786313401e1058a8eda4e653d7eb89e01 Mon Sep 17 00:00:00 2001 From: Marcin Grzejszczak Date: Tue, 16 Aug 2022 16:15:01 +0200 Subject: [PATCH] Prevents unzipping files outside of the target folder --- .../cloud/internal/ZipCategory.java | 22 +++++-- .../cloud/internal/ZipCategoryTests.java | 58 ++++++++++++++++++ train-docs/src/test/resources/file.zip | Bin 0 -> 171 bytes .../resources/zip/zip-malicious-traversal.zip | Bin 0 -> 545 bytes 4 files changed, 74 insertions(+), 6 deletions(-) create mode 100644 train-docs/src/test/java/org/springframework/cloud/internal/ZipCategoryTests.java create mode 100644 train-docs/src/test/resources/file.zip create mode 100644 train-docs/src/test/resources/zip/zip-malicious-traversal.zip 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 0000000000000000000000000000000000000000..3da3a912405f9967ebedebe02f41ce44b66ddd5e GIT binary patch literal 171 zcmWIWW@h1H0D)76LW?TkIfXruL1mY!)AQr+{R*11^Mh1AZvVqhw0-+y}b_a17 E0OX(`8~^|S literal 0 HcmV?d00001 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 0000000000000000000000000000000000000000..38b3f499de0163e62ca15ce18350a9d9a477a51b GIT binary patch literal 545 zcmWIWW@h1H0D=Au{XYEp{-1?`Y!K#PkYPyA&ri`SsVE5z;bdU8U359h4v0%DxEUB( zzA-W|u!sQFm1JZVD*#cV0!Xz&eqJh90MJm76a&LlprHwl)s`S02)6*So}T`Ippx7I z{nWC|9FT|Lj?Pm62|-=W$Rx*%D=;L0E@xl>dYWNLBZ!3v8dgZqpan~SHzSh>Gwx6T jnE?Vz8bg8PfCLE8QsgiR@MdKLxrhk}K_2A>d6oeH^pk5C literal 0 HcmV?d00001