Fix zip slip check in StructContext on Windows

[Miguel0888]

Summary

This PR fixes a Windows-specific bug in StructContext.addArchive that prevents Fernflower from decompiling any JAR/ZIP archives when running on Windows.

The issue is not caused by corrupted archives – it is triggered by the zip-slip protection using File.getCanonicalPath() on paths that are not real directories but zipFile\entry combinations. On Windows this can throw an IOException even for perfectly valid entries, which then causes Fernflower to treat the whole archive as "corrupted".

The fix replaces the filesystem-based zip-slip check with a string-based check on the normalized entry name. This preserves the security goal (rejecting absolute paths and .. escapes) while avoiding Windows-specific path resolution bugs.

---

Problem / Error message

On Windows, any decompilation that involves archives inside an input directory fails with:

ERROR: Corrupted archive file: C:\temp\Fernflower\disassembled\unpacked\com.softwareag.wsstack.ui.registration_10.1.0.0000-0357\etc\Concept-AAR.zip
java.io.IOException: Die Syntax für den Dateinamen, Verzeichnisnamen oder die Datenträgerbezeichnung ist falsch
        at java.base/java.io.WinNTFileSystem.canonicalize0(Native Method)
        at java.base/java.io.WinNTFileSystem.canonicalize(WinNTFileSystem.java:463)
        at java.base/java.io.File.getCanonicalPath(File.java:626)
        at org.jetbrains.java.decompiler.struct.StructContext.addArchive(StructContext.java:143)
        at org.jetbrains.java.decompiler.struct.StructContext.addSpace(StructContext.java:90)
        at org.jetbrains.java.decompiler.struct.StructContext.addSpace(StructContext.java:75)
        at org.jetbrains.java.decompiler.struct.StructContext.addSpace(StructContext.java:75)
        at org.jetbrains.java.decompiler.struct.StructContext.addSpace(StructContext.java:75)
        at org.jetbrains.java.decompiler.struct.StructContext.addSpace(StructContext.java:64)
        at org.jetbrains.java.decompiler.main.Fernflower.addSource(Fernflower.java:107)
        at org.jetbrains.java.decompiler.main.decompiler.ConsoleDecompiler.addSource(ConsoleDecompiler.java:125)
        at org.jetbrains.java.decompiler.main.decompiler.ConsoleDecompiler.main(ConsoleDecompiler.java:85)

This happens not only for complex plugin archives, but can be reproduced with a minimal example:

• Create a.zip containing a single empty file b (no subdirectories).
• Run Fernflower on a directory that contains this a.zip.
• On Windows, StructContext.addArchive throws the above IOException from getCanonicalPath(), and the archive is reported as "Corrupted archive file".

If the file b is removed from the ZIP (so the ZIP is technically valid but empty), the error disappears. The problem is therefore unrelated to the ZIP structure itself and entirely due to how the path check is implemented.

This also means that on Windows Fernflower cannot reliably "convert" lib JARs/ZIPs into decompiled versions: as soon as a problematic entry hits this check, processing aborts with the above error.

---

Root cause

The relevant code in StructContext.addArchive currently does:

String name = entry.getName();
File test = new File(file.getAbsolutePath(), name);
if (!test.getCanonicalPath().startsWith(file.getCanonicalPath() + File.separator)) { // check for zip slip exploit
  throw new RuntimeException("Zip entry '" + entry.getName() + "' tries to escape target directory");
}

Here, file is the ZIP/JAR itself, e.g.:

C:\temp\Fernflower\disassembled\unpacked\...\etc\Concept-AAR.zip

For an entry name = "b" (or e.g. "TAX/b"), this constructs:

C:\temp\Fernflower\...\etc\Concept-AAR.zip\b

Calling getCanonicalPath() on such a "zipFile\entry" path is not meaningful in this context (Fernflower does not actually extract entries to disk), and on Windows this can fail with:

Die Syntax für den Dateinamen, Verzeichnisnamen oder die Datenträgerbezeichnung ist falsch.

This IOException bubbles up to the addSpace method and is logged as:

ERROR: Corrupted archive file: <zip>

so the entire archive is discarded even though:

• the ZIP is structurally valid and
• all entry names are syntactically normal from a ZIP perspective.

The current zip-slip protection therefore has two problems:

1. It uses the ZIP file itself as a "base directory" and constructs zipFile\entry filesystem paths, which are not the actual extraction target.
2. It depends on File.getCanonicalPath() for these pseudo-paths, which is fragile and platform-specific (fails on Windows for combinations that are perfectly valid as ZIP entries).

Fernflower only reads entries from the archive, it does not actually extract them to the filesystem under file.getAbsolutePath(), so using canonical filesystem paths here is not necessary.

---

Fix

The fix keeps the idea of rejecting suspicious entry paths (zip slip protection), but changes the implementation to a normalized string-based check that does not depend on filesystem resolution:

String name = entry.getName();
String normalizedName = name.replace('\', '/');
if (normalizedName.startsWith("/") ||
    normalizedName.startsWith("../") ||
    normalizedName.contains("/../")) {
  throw new RuntimeException("Zip entry '" + entry.getName() + "' tries to escape target directory");
}

Everything else in addArchive remains unchanged:

• .class entries are still loaded and decompiled as before.
• Non-class entries are still passed through as otherEntry / dirEntry.
• The behavior for "normal" entries is unchanged on non-Windows platforms.

On Windows, the crucial difference is:

• We no longer call File.getCanonicalPath() on zipFile\entry paths.
• Therefore, no spurious IOException is thrown for valid ZIP entries.
• Archives that previously triggered "Corrupted archive file" errors are processed successfully.

---

Security considerations

The original intention of the code is to prevent zip-slip style path traversal attacks. In this context:

• Fernflower does not extract entries to the filesystem under file.getAbsolutePath().
• It only reads entries from a ZIP/JAR that is already present on disk and writes decompiled .java files into the configured destination via IResultSaver.

Given that, the important safety property is:

Do not allow entries whose logical path tries to escape the logical root (e.g. ../.., absolute paths).

The new check still enforces exactly this:

• It rejects:
  • entries starting with / (absolute paths),
  • entries starting with ../,
  • entries containing /../ in the middle.
• It allows:
  • regular relative paths like com/example/Foo.class,
  • simple names like b, TAX/b, etc.

This preserves the zip-slip protection at the logical entry-name level, while removing the dependency on platform-specific canonical path resolution that caused valid archives to fail on Windows.

In other words:

• The security mechanism is not disabled.
• It is made more correct and platform-independent by enforcing the same policy on the entry path itself, instead of on a synthetic zipFile\entry filesystem path.

---

Impact

• Fixes a Windows-only bug that prevented decompilation of JAR/ZIP archives (especially when archives were nested inside plugin / plugins/ directories).
• Does not change behavior on logically invalid/suspicious entry names (they are still rejected).
• Keeps the conceptual security goal (zip slip prevention) intact, while avoiding reliance on File.getCanonicalPath() for paths that are not real extraction targets.

[Copilot]

Pull request overview

This PR addresses a Windows-specific bug in the zip slip protection mechanism within StructContext.addArchive. The issue occurs when File.getCanonicalPath() is called on synthetic filesystem paths (combining ZIP file paths with entry names), which causes IOException on Windows and incorrectly marks valid ZIP archives as corrupted. The fix replaces the filesystem-based check with a string-based validation approach.

Key Changes:

• Replaced File.getCanonicalPath() check with string normalization and pattern matching
• Changed from constructing synthetic filesystem paths to directly validating ZIP entry names
• Preserved the security goal of preventing zip slip attacks while fixing Windows compatibility

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

The string concatenation using the + operator should be replaced with string literals for better performance and readability. Instead of ".." + "/" and "/" + ".." + "/", use "../" and "/../" directly. While the compiler may optimize simple concatenations, using string literals is clearer and avoids any potential runtime concatenation overhead.

Suggested change

	if (normalizedName.startsWith("/") || normalizedName.startsWith(".." + "/") || normalizedName.contains("/" + ".." + "/")) { // check for zip slip exploit
	if (normalizedName.startsWith("/") || normalizedName.startsWith("../") || normalizedName.contains("/../")) { // check for zip slip exploit

[Copilot]

The zip slip check may not catch all path traversal attempts. Consider these edge cases:

1. Entry names that are exactly .. (would not be caught by startsWith("../"))
2. Entry names ending with /.. like somedir/.. (would not be caught by contains("/../"))
3. Entry names ending with .. without a slash like foo/bar.. should be allowed, but foo/bar/.. should be rejected

The check should handle these cases to be more comprehensive. Consider using a pattern that checks for .. as a complete path component, such as checking if the normalized name equals ".." or checking for the pattern "/.." at the end of the string or followed by /.

Suggested change

	if (normalizedName.startsWith("/") || normalizedName.startsWith(".." + "/") || normalizedName.contains("/" + ".." + "/")) { // check for zip slip exploit
	throw new RuntimeException("Zip entry '" + entry.getName() + "' tries to escape target directory");
	}
	if (normalizedName.startsWith("/")) { // check for zip slip exploit: absolute path
	throw new RuntimeException("Zip entry '" + entry.getName() + "' tries to escape target directory");
	}
	for (String component : normalizedName.split("/")) { // check for zip slip exploit: parent directory traversal
	if ("..".equals(component)) {
	throw new RuntimeException("Zip entry '" + entry.getName() + "' tries to escape target directory");
	}
	}

[BartvHelvert]

Hi, could you please squash your changes and create a bug report at https://youtrack.jetbrains.com/issues/IDEA