Dissallow traversal entry even for byte[]
The previous change prevents the transformer from writing a file outside of the working directory. However it still produced an entry for an errant file when producing just contents, and not writing to the file system. However, the errant path would be added to the message and might be used by subsequent components to write to the file system. This situation is present in the `UnZip2FileTests`. While this vulnerability is not directly exposed by the framework, user applications could be affected by it.
This commit is contained in:
committed by
Artem Bilan
parent
8c4e18fde9
commit
d10f537283
@@ -129,17 +129,7 @@ public class UnZipTransformer extends AbstractZipTransformer {
|
||||
}
|
||||
|
||||
if (ZipResultType.FILE.equals(zipResultType)) {
|
||||
final File tempDir = new File(workDirectory, message.getHeaders().getId().toString());
|
||||
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 (!destinationFile.getCanonicalPath().startsWith(workDirectory.getCanonicalPath())) {
|
||||
throw new ZipException("The file " + zipEntryName +
|
||||
" is trying to leave the target output directory of " + workDirectory);
|
||||
}
|
||||
final File destinationFile = checkPath(message, zipEntryName);
|
||||
|
||||
if (zipEntry.isDirectory()) {
|
||||
destinationFile.mkdirs(); //NOSONAR false positive
|
||||
@@ -151,6 +141,7 @@ public class UnZipTransformer extends AbstractZipTransformer {
|
||||
}
|
||||
else if (ZipResultType.BYTE_ARRAY.equals(zipResultType)) {
|
||||
if (!zipEntry.isDirectory()) {
|
||||
checkPath(message, zipEntryName);
|
||||
byte[] data = IOUtils.toByteArray(zipEntryInputStream);
|
||||
uncompressedData.put(zipEntryName, data);
|
||||
}
|
||||
@@ -159,6 +150,21 @@ public class UnZipTransformer extends AbstractZipTransformer {
|
||||
throw new IllegalStateException("Unsupported zipResultType " + zipResultType);
|
||||
}
|
||||
}
|
||||
|
||||
public File checkPath(final Message<?> message, final String zipEntryName) throws IOException {
|
||||
final File tempDir = new File(workDirectory, message.getHeaders().getId().toString());
|
||||
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 (!destinationFile.getCanonicalPath().startsWith(workDirectory.getCanonicalPath())) {
|
||||
throw new ZipException("The file " + zipEntryName +
|
||||
" is trying to leave the target output directory of " + workDirectory);
|
||||
}
|
||||
return destinationFile;
|
||||
}
|
||||
});
|
||||
|
||||
if (uncompressedData.isEmpty()) {
|
||||
|
||||
@@ -16,6 +16,10 @@
|
||||
|
||||
package org.springframework.integration.zip;
|
||||
|
||||
import static org.hamcrest.Matchers.containsString;
|
||||
import static org.hamcrest.Matchers.instanceOf;
|
||||
import static org.junit.Assert.fail;
|
||||
|
||||
import java.io.File;
|
||||
import java.io.IOException;
|
||||
import java.io.InputStream;
|
||||
@@ -28,6 +32,7 @@ import org.junit.Before;
|
||||
import org.junit.Rule;
|
||||
import org.junit.Test;
|
||||
import org.junit.rules.TemporaryFolder;
|
||||
import org.zeroturnaround.zip.ZipException;
|
||||
|
||||
import org.springframework.context.annotation.AnnotationConfigApplicationContext;
|
||||
import org.springframework.context.annotation.Bean;
|
||||
@@ -36,8 +41,10 @@ import org.springframework.context.annotation.ImportResource;
|
||||
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.MessageChannel;
|
||||
import org.springframework.messaging.MessageHandlingException;
|
||||
|
||||
/**
|
||||
*
|
||||
@@ -146,6 +153,25 @@ public class UnZip2FileTests {
|
||||
|
||||
}
|
||||
|
||||
@Test
|
||||
public void unZipTraversal() throws Exception {
|
||||
final Resource resource = this.resourceLoader.getResource("classpath:testzipdata/zip-malicious-traversal.zip");
|
||||
final InputStream is = resource.getInputStream();
|
||||
byte[] zipdata = IOUtils.toByteArray(is);
|
||||
final Message<byte[]> message = MessageBuilder.withPayload(zipdata).build();
|
||||
try {
|
||||
input.send(message);
|
||||
fail("Expected Exception");
|
||||
}
|
||||
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"));
|
||||
}
|
||||
}
|
||||
|
||||
@Configuration
|
||||
@ImportResource("classpath:org/springframework/integration/zip/UnZip2FileTests-context.xml")
|
||||
public static class ContextConfiguration {
|
||||
|
||||
Reference in New Issue
Block a user