Skip to content

Commit a5573eb

Browse files
artembilangaryrussell
authored andcommitted
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.
1 parent c5602c4 commit a5573eb

File tree

4 files changed

+48
-9
lines changed

4 files changed

+48
-9
lines changed

spring-integration-zip/build.gradle

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -61,9 +61,9 @@ ext {
6161
linkScmConnection = 'https://github.com/spring-projects/spring-integration-extensions.git'
6262
linkScmDevConnection = '[email protected]:spring-projects/spring-integration-extensions.git'
6363

64-
slf4jVersion = "1.7.21"
65-
springIntegrationVersion = '4.3.10.RELEASE'
66-
ztZipVersion = '1.11'
64+
slf4jVersion = "1.7.25"
65+
springIntegrationVersion = '4.3.15.RELEASE'
66+
ztZipVersion = '1.13'
6767

6868
idPrefix = 'zip'
6969
}

spring-integration-zip/src/main/java/org/springframework/integration/zip/transformer/UnZipTransformer.java

Lines changed: 16 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2015-2016 the original author or authors.
2+
* Copyright 2015-2018 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -29,6 +29,7 @@
2929
import org.apache.commons.logging.Log;
3030
import org.apache.commons.logging.LogFactory;
3131
import org.zeroturnaround.zip.ZipEntryCallback;
32+
import org.zeroturnaround.zip.ZipException;
3233
import org.zeroturnaround.zip.ZipUtil;
3334

3435
import org.springframework.messaging.Message;
@@ -41,6 +42,7 @@
4142
*
4243
* @author Gunnar Hillert
4344
* @author Artem Bilan
45+
*
4446
* @since 1.0
4547
*
4648
*/
@@ -104,7 +106,7 @@ else if (payload instanceof byte[]) {
104106
}
105107
else {
106108
throw new IllegalArgumentException(String.format("Unsupported payload type '%s'. " +
107-
"The only supported payload types are java.io.File, byte[] and java.io.InputStream",
109+
"The only supported payload types are java.io.File, byte[] and java.io.InputStream",
108110
payload.getClass().getSimpleName()));
109111
}
110112

@@ -122,7 +124,7 @@ public void process(InputStream zipEntryInputStream, ZipEntry zipEntry) throws I
122124

123125
if (logger.isInfoEnabled()) {
124126
logger.info(String.format("Unpacking Zip Entry - Name: '%s',Time: '%s', " +
125-
"Compressed Size: '%s', Type: '%s'",
127+
"Compressed Size: '%s', Type: '%s'",
126128
zipEntryName, zipEntryTime, zipEntryCompressedSize, type));
127129
}
128130

@@ -131,6 +133,15 @@ public void process(InputStream zipEntryInputStream, ZipEntry zipEntry) throws I
131133
tempDir.mkdirs(); //NOSONAR false positive
132134
final File destinationFile = new File(tempDir, zipEntryName);
133135

136+
/* If we see the relative traversal string of ".." we need to make sure
137+
* that the outputdir + name doesn't leave the outputdir.
138+
*/
139+
if (zipEntryName.contains("..") &&
140+
!destinationFile.getCanonicalPath().startsWith(workDirectory.getCanonicalPath())) {
141+
throw new ZipException("The file " + zipEntryName +
142+
" is trying to leave the target output directory of " + workDirectory);
143+
}
144+
134145
if (zipEntry.isDirectory()) {
135146
destinationFile.mkdirs(); //NOSONAR false positive
136147
}
@@ -165,8 +176,8 @@ else if (ZipResultType.BYTE_ARRAY.equals(zipResultType)) {
165176
}
166177
else {
167178
throw new MessagingException(message,
168-
String.format("The UnZip operation extracted %s "
169-
+ "result objects but expectSingleResult was 'true'.", uncompressedData.size()));
179+
String.format("The UnZip operation extracted %s "
180+
+ "result objects but expectSingleResult was 'true'.", uncompressedData.size()));
170181
}
171182
}
172183
else {

spring-integration-zip/src/test/java/org/springframework/integration/zip/transformer/UnZipTransformerTests.java

Lines changed: 29 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2015-2016 the original author or authors.
2+
* Copyright 2015-2018 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -16,6 +16,9 @@
1616

1717
package org.springframework.integration.zip.transformer;
1818

19+
import static org.hamcrest.Matchers.containsString;
20+
import static org.hamcrest.Matchers.instanceOf;
21+
1922
import java.io.File;
2023
import java.io.FileOutputStream;
2124
import java.io.IOException;
@@ -30,12 +33,15 @@
3033
import org.junit.Test;
3134
import org.junit.rules.TemporaryFolder;
3235
import org.junit.runner.RunWith;
36+
import org.zeroturnaround.zip.ZipException;
3337

3438
import org.springframework.beans.factory.annotation.Autowired;
3539
import org.springframework.core.io.Resource;
3640
import org.springframework.core.io.ResourceLoader;
3741
import org.springframework.integration.support.MessageBuilder;
42+
import org.springframework.integration.transformer.MessageTransformationException;
3843
import org.springframework.messaging.Message;
44+
import org.springframework.messaging.MessageHandlingException;
3945
import org.springframework.messaging.MessagingException;
4046
import org.springframework.test.context.ContextConfiguration;
4147
import org.springframework.test.context.junit4.SpringJUnit4ClassRunner;
@@ -44,6 +50,7 @@
4450
*
4551
* @author Gunnar Hillert
4652
* @author Artem Bilan
53+
*
4754
* @since 1.0
4855
*
4956
*/
@@ -253,4 +260,25 @@ public void unzipInvalidZipFile() throws IOException, InterruptedException {
253260
}
254261
}
255262

263+
@Test
264+
public void testUnzipMaliciousTraversalZipFile() throws IOException {
265+
final Resource resource = this.resourceLoader.getResource("classpath:testzipdata/zip-malicious-traversal.zip");
266+
final InputStream is = resource.getInputStream();
267+
268+
final Message<InputStream> message = MessageBuilder.withPayload(is).build();
269+
270+
final UnZipTransformer unZipTransformer = new UnZipTransformer();
271+
unZipTransformer.afterPropertiesSet();
272+
273+
try {
274+
unZipTransformer.transform(message);
275+
}
276+
catch (Exception e) {
277+
Assert.assertThat(e, instanceOf(MessageTransformationException.class));
278+
Assert.assertThat(e.getCause(), instanceOf(MessageHandlingException.class));
279+
Assert.assertThat(e.getCause().getCause(), instanceOf(ZipException.class));
280+
Assert.assertThat(e.getCause().getCause().getMessage(),
281+
containsString("is trying to leave the target output directory"));
282+
}
283+
}
256284
}
Binary file not shown.

0 commit comments

Comments
 (0)