Additional check when unpacking archives. Contributed by Wilfred Spiegelenburg.

This commit is contained in:
Kihwal Lee 2018-05-29 14:15:12 -05:00
parent 9502b47bd2
commit e3236a9680
2 changed files with 52 additions and 0 deletions

View File

@ -117,12 +117,17 @@ public static void unJar(InputStream inputStream, File toDir,
throws IOException {
try (JarInputStream jar = new JarInputStream(inputStream)) {
int numOfFailedLastModifiedSet = 0;
String targetDirPath = toDir.getCanonicalPath() + File.separator;
for (JarEntry entry = jar.getNextJarEntry();
entry != null;
entry = jar.getNextJarEntry()) {
if (!entry.isDirectory() &&
unpackRegex.matcher(entry.getName()).matches()) {
File file = new File(toDir, entry.getName());
if (!file.getCanonicalPath().startsWith(targetDirPath)) {
throw new IOException("expanding " + entry.getName()
+ " would create file outside of " + toDir);
}
ensureDirectory(file.getParentFile());
try (OutputStream out = new FileOutputStream(file)) {
IOUtils.copyBytes(jar, out, BUFFER_SIZE);
@ -182,6 +187,7 @@ public static void unJar(File jarFile, File toDir, Pattern unpackRegex)
throws IOException {
try (JarFile jar = new JarFile(jarFile)) {
int numOfFailedLastModifiedSet = 0;
String targetDirPath = toDir.getCanonicalPath() + File.separator;
Enumeration<JarEntry> entries = jar.entries();
while (entries.hasMoreElements()) {
final JarEntry entry = entries.nextElement();
@ -189,6 +195,10 @@ public static void unJar(File jarFile, File toDir, Pattern unpackRegex)
unpackRegex.matcher(entry.getName()).matches()) {
try (InputStream in = jar.getInputStream(entry)) {
File file = new File(toDir, entry.getName());
if (!file.getCanonicalPath().startsWith(targetDirPath)) {
throw new IOException("expanding " + entry.getName()
+ " would create file outside of " + toDir);
}
ensureDirectory(file.getParentFile());
try (OutputStream out = new FileOutputStream(file)) {
IOUtils.copyBytes(in, out, BUFFER_SIZE);

View File

@ -21,6 +21,7 @@
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertTrue;
import static org.junit.Assert.fail;
import static org.mockito.Matchers.any;
import static org.mockito.Mockito.spy;
import static org.mockito.Mockito.times;
@ -32,6 +33,7 @@
import java.io.FileOutputStream;
import java.io.IOException;
import java.io.InputStream;
import java.nio.charset.StandardCharsets;
import java.util.Random;
import java.util.jar.JarEntry;
import java.util.jar.JarOutputStream;
@ -255,4 +257,44 @@ public void testClientClassLoaderSkipUnjar() throws Throwable {
// it should not throw an exception
verify(runJar, times(0)).unJar(any(File.class), any(File.class));
}
@Test
public void testUnJar2() throws IOException {
// make a simple zip
File jarFile = new File(TEST_ROOT_DIR, TEST_JAR_NAME);
JarOutputStream jstream =
new JarOutputStream(new FileOutputStream(jarFile));
JarEntry je = new JarEntry("META-INF/MANIFEST.MF");
byte[] data = "Manifest-Version: 1.0\nCreated-By: 1.8.0_1 (Manual)"
.getBytes(StandardCharsets.UTF_8);
je.setSize(data.length);
jstream.putNextEntry(je);
jstream.write(data);
jstream.closeEntry();
je = new JarEntry("../outside.path");
data = "any data here".getBytes(StandardCharsets.UTF_8);
je.setSize(data.length);
jstream.putNextEntry(je);
jstream.write(data);
jstream.closeEntry();
jstream.close();
File unjarDir = getUnjarDir("unjar-path");
// Unjar everything
try {
RunJar.unJar(jarFile, unjarDir, MATCH_ANY);
fail("unJar should throw IOException.");
} catch (IOException e) {
GenericTestUtils.assertExceptionContains(
"would create file outside of", e);
}
try {
RunJar.unJar(new FileInputStream(jarFile), unjarDir, MATCH_ANY);
fail("unJar should throw IOException.");
} catch (IOException e) {
GenericTestUtils.assertExceptionContains(
"would create file outside of", e);
}
}
}