From a5573eb232ff85199ff9bb28993df715d9a19a25 Mon Sep 17 00:00:00 2001 From: Artem Bilan Date: Wed, 2 May 2018 21:12:30 -0400 Subject: [PATCH] Disallow traversal entity in zip When the file name holds path traversal file names it gets concatenated to the target extraction directory, the final path ends up outside of the target folder. --- spring-integration-zip/build.gradle | 6 ++-- .../zip/transformer/UnZipTransformer.java | 21 +++++++++--- .../transformer/UnZipTransformerTests.java | 30 +++++++++++++++++- .../testzipdata/zip-malicious-traversal.zip | Bin 0 -> 545 bytes 4 files changed, 48 insertions(+), 9 deletions(-) create mode 100644 spring-integration-zip/src/test/resources/testzipdata/zip-malicious-traversal.zip diff --git a/spring-integration-zip/build.gradle b/spring-integration-zip/build.gradle index 33c20b7..9610e71 100644 --- a/spring-integration-zip/build.gradle +++ b/spring-integration-zip/build.gradle @@ -61,9 +61,9 @@ ext { linkScmConnection = 'https://github.com/spring-projects/spring-integration-extensions.git' linkScmDevConnection = 'git@github.com:spring-projects/spring-integration-extensions.git' - slf4jVersion = "1.7.21" - springIntegrationVersion = '4.3.10.RELEASE' - ztZipVersion = '1.11' + slf4jVersion = "1.7.25" + springIntegrationVersion = '4.3.15.RELEASE' + ztZipVersion = '1.13' idPrefix = 'zip' } diff --git a/spring-integration-zip/src/main/java/org/springframework/integration/zip/transformer/UnZipTransformer.java b/spring-integration-zip/src/main/java/org/springframework/integration/zip/transformer/UnZipTransformer.java index 6dd300e..aad8cd4 100644 --- a/spring-integration-zip/src/main/java/org/springframework/integration/zip/transformer/UnZipTransformer.java +++ b/spring-integration-zip/src/main/java/org/springframework/integration/zip/transformer/UnZipTransformer.java @@ -1,5 +1,5 @@ /* - * Copyright 2015-2016 the original author or authors. + * Copyright 2015-2018 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. @@ -29,6 +29,7 @@ import org.apache.commons.io.IOUtils; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; import org.zeroturnaround.zip.ZipEntryCallback; +import org.zeroturnaround.zip.ZipException; import org.zeroturnaround.zip.ZipUtil; import org.springframework.messaging.Message; @@ -41,6 +42,7 @@ import org.springframework.messaging.MessagingException; * * @author Gunnar Hillert * @author Artem Bilan + * * @since 1.0 * */ @@ -104,7 +106,7 @@ public class UnZipTransformer extends AbstractZipTransformer { } else { throw new IllegalArgumentException(String.format("Unsupported payload type '%s'. " + - "The only supported payload types are java.io.File, byte[] and java.io.InputStream", + "The only supported payload types are java.io.File, byte[] and java.io.InputStream", payload.getClass().getSimpleName())); } @@ -122,7 +124,7 @@ public class UnZipTransformer extends AbstractZipTransformer { if (logger.isInfoEnabled()) { logger.info(String.format("Unpacking Zip Entry - Name: '%s',Time: '%s', " + - "Compressed Size: '%s', Type: '%s'", + "Compressed Size: '%s', Type: '%s'", zipEntryName, zipEntryTime, zipEntryCompressedSize, type)); } @@ -131,6 +133,15 @@ public class UnZipTransformer extends AbstractZipTransformer { tempDir.mkdirs(); //NOSONAR false positive final File destinationFile = new File(tempDir, zipEntryName); + /* If we see the relative traversal string of ".." we need to make sure + * that the outputdir + name doesn't leave the outputdir. + */ + if (zipEntryName.contains("..") && + !destinationFile.getCanonicalPath().startsWith(workDirectory.getCanonicalPath())) { + throw new ZipException("The file " + zipEntryName + + " is trying to leave the target output directory of " + workDirectory); + } + if (zipEntry.isDirectory()) { destinationFile.mkdirs(); //NOSONAR false positive } @@ -165,8 +176,8 @@ public class UnZipTransformer extends AbstractZipTransformer { } else { throw new MessagingException(message, - String.format("The UnZip operation extracted %s " - + "result objects but expectSingleResult was 'true'.", uncompressedData.size())); + String.format("The UnZip operation extracted %s " + + "result objects but expectSingleResult was 'true'.", uncompressedData.size())); } } else { diff --git a/spring-integration-zip/src/test/java/org/springframework/integration/zip/transformer/UnZipTransformerTests.java b/spring-integration-zip/src/test/java/org/springframework/integration/zip/transformer/UnZipTransformerTests.java index 26864b7..4ce4685 100644 --- a/spring-integration-zip/src/test/java/org/springframework/integration/zip/transformer/UnZipTransformerTests.java +++ b/spring-integration-zip/src/test/java/org/springframework/integration/zip/transformer/UnZipTransformerTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2015-2016 the original author or authors. + * Copyright 2015-2018 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. @@ -16,6 +16,9 @@ package org.springframework.integration.zip.transformer; +import static org.hamcrest.Matchers.containsString; +import static org.hamcrest.Matchers.instanceOf; + import java.io.File; import java.io.FileOutputStream; import java.io.IOException; @@ -30,12 +33,15 @@ import org.junit.Rule; import org.junit.Test; import org.junit.rules.TemporaryFolder; import org.junit.runner.RunWith; +import org.zeroturnaround.zip.ZipException; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.core.io.Resource; import org.springframework.core.io.ResourceLoader; import org.springframework.integration.support.MessageBuilder; +import org.springframework.integration.transformer.MessageTransformationException; import org.springframework.messaging.Message; +import org.springframework.messaging.MessageHandlingException; import org.springframework.messaging.MessagingException; import org.springframework.test.context.ContextConfiguration; import org.springframework.test.context.junit4.SpringJUnit4ClassRunner; @@ -44,6 +50,7 @@ import org.springframework.test.context.junit4.SpringJUnit4ClassRunner; * * @author Gunnar Hillert * @author Artem Bilan + * * @since 1.0 * */ @@ -253,4 +260,25 @@ public class UnZipTransformerTests { } } + @Test + public void testUnzipMaliciousTraversalZipFile() throws IOException { + final Resource resource = this.resourceLoader.getResource("classpath:testzipdata/zip-malicious-traversal.zip"); + final InputStream is = resource.getInputStream(); + + final Message message = MessageBuilder.withPayload(is).build(); + + final UnZipTransformer unZipTransformer = new UnZipTransformer(); + unZipTransformer.afterPropertiesSet(); + + try { + unZipTransformer.transform(message); + } + catch (Exception e) { + Assert.assertThat(e, instanceOf(MessageTransformationException.class)); + Assert.assertThat(e.getCause(), instanceOf(MessageHandlingException.class)); + Assert.assertThat(e.getCause().getCause(), instanceOf(ZipException.class)); + Assert.assertThat(e.getCause().getCause().getMessage(), + containsString("is trying to leave the target output directory")); + } + } } diff --git a/spring-integration-zip/src/test/resources/testzipdata/zip-malicious-traversal.zip b/spring-integration-zip/src/test/resources/testzipdata/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