Apache
/
druid
mirror of https://github.com/apache/druid.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Fix Zip Slip vulnerability (#5850) 

* Fix evil zip exploit

* PR comment, checkstyle

* PR comments

* Add link to vulnerability report

* Fix test
This commit is contained in:
Jonathan Wei 2018-06-12 10:03:08 -07:00 committed by GitHub
parent 2feec44a55
commit bc9da54e12
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
7 changed files with 148 additions and 11 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

30
indexing-hadoop/src/main/java/io/druid/indexer/JobHelper.java
View File

@ -25,6 +25,7 @@ import com.google.common.base.Strings;
import com.google.common.base.Throwables;
import com.google.common.io.Files;
import io.druid.indexer.updater.HadoopDruidConverterConfig;
import io.druid.java.util.common.CompressionUtils;
import io.druid.java.util.common.DateTimes;
import io.druid.java.util.common.FileUtils;
import io.druid.java.util.common.IAE;
@ -43,6 +44,7 @@ import org.apache.hadoop.fs.FileSystem;
import org.apache.hadoop.fs.LocalFileSystem;
import org.apache.hadoop.fs.Path;
import org.apache.hadoop.io.retry.RetryPolicies;
import org.apache.hadoop.io.retry.RetryPolicy;
import org.apache.hadoop.io.retry.RetryProxy;
import org.apache.hadoop.mapreduce.Job;
import org.apache.hadoop.mapreduce.MRJobConfig;
@ -676,9 +678,21 @@ public class JobHelper
final Path zip,
final Configuration configuration,
final File outDir,
final Progressable progressable
final Progressable progressable,
final RetryPolicy retryPolicy
) throws IOException
{
final RetryPolicy effectiveRetryPolicy;
if (retryPolicy == null) {
effectiveRetryPolicy = RetryPolicies.exponentialBackoffRetry(
NUM_RETRIES,
SECONDS_BETWEEN_RETRIES,
TimeUnit.SECONDS
);
} else {
effectiveRetryPolicy = retryPolicy;
}
final DataPusher zipPusher = (DataPusher) RetryProxy.create(
DataPusher.class, new DataPusher()
{
@ -693,13 +707,11 @@ public class JobHelper
try (ZipInputStream in = new ZipInputStream(fileSystem.open(zip, 1 << 13))) {
for (ZipEntry entry = in.getNextEntry(); entry != null; entry = in.getNextEntry()) {
final String fileName = entry.getName();
try (final OutputStream out = new BufferedOutputStream(
new FileOutputStream(
outDir.getAbsolutePath()
+ File.separator
+ fileName
), 1 << 13
)) {
final String outputPath = new File(outDir, fileName).getAbsolutePath();
CompressionUtils.validateZipOutputFile(zip.getName(), new File(outputPath), outDir);
try (final OutputStream out = new BufferedOutputStream(new FileOutputStream(outputPath))) {
for (int len = in.read(buffer); len >= 0; len = in.read(buffer)) {
progressable.progress();
if (len == 0) {
@ -721,7 +733,7 @@ public class JobHelper
}
}
},
RetryPolicies.exponentialBackoffRetry(NUM_RETRIES, SECONDS_BETWEEN_RETRIES, TimeUnit.SECONDS)
effectiveRetryPolicy
);
return zipPusher.push();
}

2
indexing-hadoop/src/main/java/io/druid/indexer/hadoop/DatasourceRecordReader.java
View File

@ -90,7 +90,7 @@ public class DatasourceRecordReader extends RecordReader<NullWritable, InputRow>
tmpSegmentDirs.add(dir);
logger.info("Locally storing fetched segment at [%s]", dir);
JobHelper.unzipNoGuava(path, context.getConfiguration(), dir, context);
JobHelper.unzipNoGuava(path, context.getConfiguration(), dir, context, null);
logger.info("finished fetching segment files");
QueryableIndex index = HadoopDruidIndexerConfig.INDEX_IO.loadIndex(dir);

2
indexing-hadoop/src/main/java/io/druid/indexer/updater/HadoopConverterJob.java
View File

@ -520,7 +520,7 @@ public class HadoopConverterJob
log.warn("Unable to make directory");
}
final long inSize = JobHelper.unzipNoGuava(inPath, context.getConfiguration(), inDir, context);
final long inSize = JobHelper.unzipNoGuava(inPath, context.getConfiguration(), inDir, context, null);
log.debug("Loaded %d bytes into [%s] for converting", inSize, inDir.getAbsolutePath());
context.getCounter(COUNTER_GROUP, COUNTER_LOADED).increment(inSize);

43
indexing-hadoop/src/test/java/io/druid/indexer/JobHelperTest.java
View File

@ -25,6 +25,8 @@ import io.druid.data.input.impl.CSVParseSpec;
import io.druid.data.input.impl.DimensionsSpec;
import io.druid.data.input.impl.StringInputRowParser;
import io.druid.data.input.impl.TimestampSpec;
import io.druid.java.util.common.CompressionUtils;
import io.druid.java.util.common.ISE;
import io.druid.java.util.common.Intervals;
import io.druid.java.util.common.granularity.Granularities;
import io.druid.query.aggregation.AggregatorFactory;
@ -34,7 +36,10 @@ import io.druid.segment.indexing.granularity.UniformGranularitySpec;
import io.druid.timeline.DataSegment;
import io.druid.timeline.partition.NoneShardSpec;
import org.apache.hadoop.conf.Configuration;
import org.apache.hadoop.fs.Path;
import org.apache.hadoop.io.retry.RetryPolicies;
import org.apache.hadoop.mapreduce.Job;
import org.apache.hadoop.util.Progressable;
import org.joda.time.Interval;
import org.junit.Assert;
import org.junit.Before;
@ -43,8 +48,10 @@ import org.junit.Test;
import org.junit.rules.TemporaryFolder;
import java.io.File;
import java.io.IOException;
import java.net.URI;
import java.net.URISyntaxException;
import java.nio.file.Files;
import java.util.HashMap;
import java.util.Map;
@ -178,6 +185,42 @@ public class JobHelperTest
);
}
@Test
public void testEvilZip() throws IOException
{
final File tmpDir = temporaryFolder.newFolder("testEvilZip");
final File evilResult = new File("/tmp/evil.txt");
Files.deleteIfExists(evilResult.toPath());
File evilZip = new File(tmpDir, "evil.zip");
Files.deleteIfExists(evilZip.toPath());
CompressionUtils.makeEvilZip(evilZip);
try {
JobHelper.unzipNoGuava(
new Path(evilZip.getCanonicalPath()),
new Configuration(),
tmpDir,
new Progressable()
{
@Override
public void progress()
{
}
},
RetryPolicies.TRY_ONCE_THEN_FAIL
);
}
catch (ISE ise) {
Assert.assertTrue(ise.getMessage().contains("does not start with outDir"));
Assert.assertFalse("Zip exploit triggered, /tmp/evil.txt was written.", evilResult.exists());
return;
}
Assert.fail("Exception was not thrown for malicious zip file");
}
private static class HadoopDruidIndexerConfigSpy extends HadoopDruidIndexerConfig
{

BIN
indexing-hadoop/src/test/resources/evil.zip Normal file
View File

Binary file not shown.

36
java-util/src/main/java/io/druid/java/util/common/CompressionUtils.java
View File

@ -232,6 +232,9 @@ public class CompressionUtils
while (enumeration.hasMoreElements()) {
final ZipEntry entry = enumeration.nextElement();
final File outFile = new File(outDir, entry.getName());
validateZipOutputFile(pulledFile.getCanonicalPath(), outFile, outDir);
result.addFiles(
FileUtils.retryCopy(
new ByteSource()
@ -252,6 +255,25 @@ public class CompressionUtils
return result;
}
public static void validateZipOutputFile(
String sourceFilename,
final File outFile,
final File outDir
) throws IOException
{
// check for evil zip exploit that allows writing output to arbitrary directories
final File canonicalOutFile = outFile.getCanonicalFile();
final String canonicalOutDir = outDir.getCanonicalPath();
if (!canonicalOutFile.toPath().startsWith(canonicalOutDir)) {
throw new ISE(
"Unzipped output path[%s] of sourceFile[%s] does not start with outDir[%s].",
canonicalOutFile,
sourceFilename,
canonicalOutDir
);
}
}
/**
* Unzip from the input stream to the output directory, using the entry's file name as the file name in the output directory.
* The behavior of directories in the input stream's zip is undefined.
@ -272,6 +294,8 @@ public class CompressionUtils
while ((entry = zipIn.getNextEntry()) != null) {
final File file = new File(outDir, entry.getName());
validateZipOutputFile("", file, outDir);
NativeIO.chunkedCopy(zipIn, file);
result.addFile(file);
@ -562,4 +586,16 @@ public class CompressionUtils
return in;
}
}
// Helper method for unit tests (for checking that we fixed https://snyk.io/research/zip-slip-vulnerability)
public static void makeEvilZip(File outputFile) throws IOException
{
ZipOutputStream zipOutputStream = new ZipOutputStream(new FileOutputStream(outputFile));
ZipEntry zipEntry = new ZipEntry("../../../../../../../../../../../../../../../tmp/evil.txt");
zipOutputStream.putNextEntry(zipEntry);
byte[] output = StringUtils.toUtf8("evil text");
zipOutputStream.write(output);
zipOutputStream.closeEntry();
zipOutputStream.close();
}
}

46
java-util/src/test/java/io/druid/java/util/common/CompressionUtilsTest.java
View File

@ -313,6 +313,52 @@ public class CompressionUtilsTest
}
}
@Test
public void testEvilZip() throws IOException
{
final File tmpDir = temporaryFolder.newFolder("testEvilZip");
final File evilResult = new File("/tmp/evil.txt");
java.nio.file.Files.deleteIfExists(evilResult.toPath());
File evilZip = new File(tmpDir, "evil.zip");
java.nio.file.Files.deleteIfExists(evilZip.toPath());
CompressionUtils.makeEvilZip(evilZip);
try {
CompressionUtils.unzip(evilZip, tmpDir);
}
catch (ISE ise) {
Assert.assertTrue(ise.getMessage().contains("does not start with outDir"));
Assert.assertFalse("Zip exploit triggered, /tmp/evil.txt was written.", evilResult.exists());
return;
}
Assert.fail("Exception was not thrown for malicious zip file");
}
@Test
public void testEvilZipInputStream() throws IOException
{
final File tmpDir = temporaryFolder.newFolder("testEvilZip");
final File evilResult = new File("/tmp/evil.txt");
java.nio.file.Files.deleteIfExists(evilResult.toPath());
File evilZip = new File(tmpDir, "evil.zip");
java.nio.file.Files.deleteIfExists(evilZip.toPath());
CompressionUtils.makeEvilZip(evilZip);
try {
CompressionUtils.unzip(new FileInputStream(evilZip), tmpDir);
}
catch (ISE ise) {
Assert.assertTrue(ise.getMessage().contains("does not start with outDir"));
Assert.assertFalse("Zip exploit triggered, /tmp/evil.txt was written.", evilResult.exists());
return;
}
Assert.fail("Exception was not thrown for malicious zip file");
}
@Test
// Sanity check to make sure the test class works as expected
public void testZeroRemainingInputStream() throws IOException