CVE-2023-40828: Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal') (CWE-22)
value: An issue in pf4j pf4j v.3.9.0 and before allows a remote attacker to obtain sensitive information and execute arbitrary code via the expandIfZip method in the extract function.
CWE description: The product uses external input to construct a pathname that is intended to identify a file or directory that is located underneath a restricted parent directory, but the product does not properly neutralize special elements within the pathname that can cause the pathname to resolve to a location that is outside of the restricted directory.
CVSS v3 base score: 7.5
Published: 2023-08-28T22:15Z
Repository: https://github.com/pf4j/pf4j
Language: Java
Files changed: pf4j/src/main/java/org/pf4j/util/Unzip.java, pf4j/src/test/java/org/pf4j/util/UnzipTest.java
Commit date: 2023-08-16 21:00:59 +0300
Commit message: Add security checks to prevent directory traversal when decompressing (#538)


Diff: diff --git a/pf4j/src/main/java/org/pf4j/util/Unzip.java b/pf4j/src/main/java/org/pf4j/util/Unzip.java
index 198cf7d..a78cf83 100644
--- a/pf4j/src/main/java/org/pf4j/util/Unzip.java
+++ b/pf4j/src/main/java/org/pf4j/util/Unzip.java
@@ -13,21 +13,21 @@
  * See the License for the specific language governing permissions and
  * limitations under the License.
  */
 package org.pf4j.util;
 
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
 import java.io.File;
 import java.io.FileInputStream;
-import java.io.FileNotFoundException;
 import java.io.FileOutputStream;
 import java.io.IOException;
 import java.util.zip.ZipEntry;
+import java.util.zip.ZipException;
 import java.util.zip.ZipInputStream;
 
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
-
 /**
  * This class extracts the content of the plugin zip into a directory.
  * It's a class for only the internal use.
  *
  * @author Decebal Suiu
@@ -73,15 +73,21 @@ public class Unzip {
         // delete destination directory if exists
         if (destination.exists() && destination.isDirectory()) {
             FileUtils.delete(destination.toPath());
         }
 
+        String destinationCanonicalPath = destination.getCanonicalPath();
         try (ZipInputStream zipInputStream = new ZipInputStream(new FileInputStream(source))) {
             ZipEntry zipEntry;
             while ((zipEntry = zipInputStream.getNextEntry()) != null) {
                 File file = new File(destination, zipEntry.getName());
 
+                String fileCanonicalPath = file.getCanonicalPath();
+                if (!fileCanonicalPath.startsWith(destinationCanonicalPath)) {
+                    throw new ZipException("The file "+ zipEntry.getName() + " is trying to leave the target output directory of "+ destination);
+                }
+
                 // create intermediary directories - sometimes zip don't add them
                 File dir = new File(file.getParent());
 
                 mkdirsOrThrow(dir);
 
diff --git a/pf4j/src/test/java/org/pf4j/util/UnzipTest.java b/pf4j/src/test/java/org/pf4j/util/UnzipTest.java
new file mode 100644
index 0000000..202e1b0
--- /dev/null
+++ b/pf4j/src/test/java/org/pf4j/util/UnzipTest.java
@@ -0,0 +1,60 @@
+/*
+ * Copyright (C) 2012-present the original author or authors.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.pf4j.util;
+
+import org.junit.jupiter.api.Test;
+
+import java.io.File;
+import java.io.FileOutputStream;
+import java.io.IOException;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.util.zip.ZipEntry;
+import java.util.zip.ZipException;
+import java.util.zip.ZipOutputStream;
+
+import static org.junit.jupiter.api.Assertions.assertThrows;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+public class UnzipTest {
+
+    @Test
+    public void zipSlip() throws IOException {
+        File zipFile = createMaliciousZipFile();
+        Path destination = Files.createTempDirectory("zipSlip");
+
+        Unzip unzip = new Unzip();
+        unzip.setSource(zipFile);
+        unzip.setDestination(destination.toFile());
+
+        Exception exception = assertThrows(ZipException.class, unzip::extract);
+        assertTrue(exception.getMessage().contains("is trying to leave the target output directory"));
+    }
+
+    private File createMaliciousZipFile() throws IOException {
+        File zipFile = File.createTempFile("malicious", ".zip");
+        String maliciousFileName = "../malicious.sh";
+        try (ZipOutputStream zipOutputStream = new ZipOutputStream(new FileOutputStream(zipFile))) {
+            ZipEntry entry = new ZipEntry(maliciousFileName);
+            zipOutputStream.putNextEntry(entry);
+            zipOutputStream.write("Malicious content".getBytes());
+            zipOutputStream.closeEntry();
+        }
+
+        return zipFile;
+    }
+
+}

Vulnerable code: import java.io.FileNotFoundException;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

Fixed code: import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

import java.util.zip.ZipException;
        String destinationCanonicalPath = destination.getCanonicalPath();
                String fileCanonicalPath = file.getCanonicalPath();
                if (!fileCanonicalPath.startsWith(destinationCanonicalPath)) {
                    throw new ZipException("The file "+ zipEntry.getName() + " is trying to leave the target output directory of "+ destination);
                }

/*
 * Copyright (C) 2012-present the original author or authors.
 *
 * Licensed under the Apache License, Version 2.0 (the "License");
 * you may not use this file except in compliance with the License.
 * You may obtain a copy of the License at
 *
 *     http://www.apache.org/licenses/LICENSE-2.0
 *
 * Unless required by applicable law or agreed to in writing, software
 * distributed under the License is distributed on an "AS IS" BASIS,
 * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
 * See the License for the specific language governing permissions and
 * limitations under the License.
 */
package org.pf4j.util;

import org.junit.jupiter.api.Test;

import java.io.File;
import java.io.FileOutputStream;
import java.io.IOException;
import java.nio.file.Files;
import java.nio.file.Path;
import java.util.zip.ZipEntry;
import java.util.zip.ZipException;
import java.util.zip.ZipOutputStream;

import static org.junit.jupiter.api.Assertions.assertThrows;
import static org.junit.jupiter.api.Assertions.assertTrue;

public class UnzipTest {

    @Test
    public void zipSlip() throws IOException {
        File zipFile = createMaliciousZipFile();
        Path destination = Files.createTempDirectory("zipSlip");

        Unzip unzip = new Unzip();
        unzip.setSource(zipFile);
        unzip.setDestination(destination.toFile());

        Exception exception = assertThrows(ZipException.class, unzip::extract);
        assertTrue(exception.getMessage().contains("is trying to leave the target output directory"));
    }

    private File createMaliciousZipFile() throws IOException {
        File zipFile = File.createTempFile("malicious", ".zip");
        String maliciousFileName = "../malicious.sh";
        try (ZipOutputStream zipOutputStream = new ZipOutputStream(new FileOutputStream(zipFile))) {
            ZipEntry entry = new ZipEntry(maliciousFileName);
            zipOutputStream.putNextEntry(entry);
            zipOutputStream.write("Malicious content".getBytes());
            zipOutputStream.closeEntry();
        }

        return zipFile;
    }

}