Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add security checks to prevent directory traversal when decompressing… #537

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions pf4j/src/main/java/org/pf4j/util/FileUtils.java
Original file line number Diff line number Diff line change
Expand Up @@ -197,6 +197,7 @@ public static Path expandIfZip(Path filePath) throws IOException {
return pluginDirectory;
}


/**
* Return true only if path is a zip file.
*
Expand Down
10 changes: 10 additions & 0 deletions pf4j/src/main/java/org/pf4j/util/Unzip.java
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,10 @@
import java.io.FileNotFoundException;
import java.io.FileOutputStream;
import java.io.IOException;
import java.nio.file.Path;
import java.nio.file.Paths;
import java.util.zip.ZipEntry;
import java.util.zip.ZipException;
import java.util.zip.ZipInputStream;

import org.slf4j.Logger;
Expand Down Expand Up @@ -80,6 +83,11 @@ public void extract() throws IOException {
while ((zipEntry = zipInputStream.getNextEntry()) != null) {
File file = new File(destination, zipEntry.getName());

// add check
if (zipEntry.getName().indexOf("..") != -1 && !file.getCanonicalPath().startsWith(destination.getCanonicalPath())) {
throw new ZipException("The file "+zipEntry.getName()+" is trying to leave the target output directory of "+destination+". Ignoring this file.");
}

// create intermediary directories - sometimes zip don't add them
File dir = new File(file.getParent());

Expand All @@ -100,6 +108,8 @@ public void extract() throws IOException {
}
}



private static void mkdirsOrThrow(File dir) throws IOException {
if (!dir.exists() && !dir.mkdirs()) {
throw new IOException("Failed to create directory " + dir);
Expand Down
5 changes: 5 additions & 0 deletions pf4j/src/test/java/org/pf4j/LoadPluginsTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -20,10 +20,14 @@
import org.junit.jupiter.api.io.TempDir;
import org.pf4j.test.PluginZip;

import java.io.File;
import java.io.FileOutputStream;
import java.nio.file.Files;
import java.nio.file.Path;
import java.nio.file.Paths;
import java.util.Collections;
import java.util.zip.ZipEntry;
import java.util.zip.ZipOutputStream;

import static org.hamcrest.CoreMatchers.containsString;
import static org.hamcrest.CoreMatchers.equalTo;
Expand All @@ -36,6 +40,7 @@
import static org.junit.jupiter.api.Assertions.assertThrows;
import static org.junit.jupiter.api.Assertions.assertTrue;
import static org.junit.jupiter.api.Assertions.fail;
import static org.pf4j.util.FileUtils.expandIfZip;

public class LoadPluginsTest {

Expand Down
47 changes: 47 additions & 0 deletions pf4j/src/test/java/org/pf4j/util/FileUtilsTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -24,9 +24,12 @@
import java.nio.file.Path;
import java.nio.file.Paths;
import java.util.List;
import java.util.zip.ZipEntry;
import java.util.zip.ZipOutputStream;

import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertTrue;
import static org.pf4j.util.FileUtils.expandIfZip;

public class FileUtilsTest {

Expand Down Expand Up @@ -95,4 +98,48 @@ public File createSampleFile(String fileName) throws IOException {
}
return file;
}

/**
* Using the zipslip vulnerability, create a zip file.
*
* Save the created zip file in the D:/code/pf4j directory, if you do not have this path on your computer D drive, create it
* @throws Exception
*/
@Test
public void creatFile() throws Exception {

String maliciousFileName = "../../../../../../../malicious.sh";

// Build a malicious ZIP file
try (ZipOutputStream zipOutputStream = new ZipOutputStream(new FileOutputStream("malicious.zip"))) {
ZipEntry entry = new ZipEntry(maliciousFileName);
zipOutputStream.putNextEntry(entry);
zipOutputStream.write("Malicious content".getBytes());
zipOutputStream.closeEntry();
}
}

/**
* Try to extract malicious.zip to the root directory of drive D on your computer.
* @throws Exception
*/
@Test
public void loadFile() throws Exception {
// Save the created zip file in the D:/code/pf4j directory, if you do not have this path on your computer D drive, create it
String maliciousZipPath = "D:\\code\\pf4j\\malicious.zip";

// Unzip file
expandIfZip(Paths.get(maliciousZipPath));
//loadPluginFromPath(Paths.get(maliciousZipPath));

// Check whether the specified file exists in the directory
File file = new File("D:\\malicious.sh");
if (file.exists()) {
System.out.println("file exists: Directory traversal successful!");
} else {
System.out.println("file does not exist!");
}
}


}