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 33c20b73..9610e71d 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 6dd300e0..aad8cd46 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.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 @@ * * @author Gunnar Hillert * @author Artem Bilan + * * @since 1.0 * */ @@ -104,7 +106,7 @@ else if (payload instanceof byte[]) { } 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 void process(InputStream zipEntryInputStream, ZipEntry zipEntry) throws I 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 void process(InputStream zipEntryInputStream, ZipEntry zipEntry) throws I 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 @@ else if (ZipResultType.BYTE_ARRAY.equals(zipResultType)) { } 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 26864b77..4ce46850 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.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 @@ * * @author Gunnar Hillert * @author Artem Bilan + * * @since 1.0 * */ @@ -253,4 +260,25 @@ public void unzipInvalidZipFile() throws IOException, InterruptedException { } } + @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