Fix zip slip check in StructContext on Windows #22

Miguel0888 · 2025-12-10T21:02:30Z

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:

It uses the ZIP file itself as a "base directory" and constructs zipFile\entry filesystem paths, which are not the actual extraction target.
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.

(cherry picked from commit 226a520)

Copilot

Pull request overview

This PR fixes a Windows-specific bug in the zip-slip protection logic of StructContext.addArchive that caused valid JAR/ZIP archives to be incorrectly rejected as corrupted. The issue occurred because the original implementation used File.getCanonicalPath() on synthetic zipFile\entry paths, which fails on Windows even for legitimate ZIP entries. The fix replaces the filesystem-based check with a normalized string-based validation that prevents path traversal while avoiding platform-specific path resolution issues.

Key changes:

Replaced File.getCanonicalPath() check with string-based path normalization
Changed zip-slip detection to check for absolute paths and .. sequences directly in entry names

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

Copilot · 2025-12-10T21:03:06Z

src/org/jetbrains/java/decompiler/struct/StructContext.java

-        File test = new File(file.getAbsolutePath(), name);
-        if (!test.getCanonicalPath().startsWith(file.getCanonicalPath() + File.separator)) { // check for zip slip exploit
+        String normalizedName = name.replace('\\', '/');
+        if (normalizedName.startsWith("/") || normalizedName.startsWith("../") || normalizedName.contains("/../")) {


The zip-slip protection is incomplete. It doesn't handle cases where a path ends with .. (e.g., foo/..) or contains multiple consecutive slashes that could be normalized away. Consider using a more robust path normalization approach or additional checks for these edge cases.

Copilot · 2025-12-10T21:03:06Z

src/org/jetbrains/java/decompiler/struct/StructContext.java

-        File test = new File(file.getAbsolutePath(), name);
-        if (!test.getCanonicalPath().startsWith(file.getCanonicalPath() + File.separator)) { // check for zip slip exploit
+        String normalizedName = name.replace('\\', '/');
+        if (normalizedName.startsWith("/") || normalizedName.startsWith("../") || normalizedName.contains("/../")) {


The check should also reject paths that start with .. without a slash (e.g., ..foo), as startsWith(\"../\") only catches cases with a trailing slash. Consider changing to startsWith(\"..\") to catch all cases where the path begins with two dots.

Suggested change

if (normalizedName.startsWith("/") || normalizedName.startsWith("../") || normalizedName.contains("/../")) {

if (normalizedName.startsWith("/") || normalizedName.startsWith("..") || normalizedName.contains("/../")) {

Phoenixcwx · 2025-12-10T21:03:16Z

您的邮件已被phoenix查收欢迎下次来信

uRyanxD · 2025-12-13T02:51:19Z

@Miguel0888 This is an unofficial mirror of the actual code; a PR here will have no effect, unfortunately. You should open a pull request at https://github.com/JetBrains/intellij-community/tree/master/plugins/java-decompiler/engine

Miguel0888 · 2025-12-21T16:43:40Z

@Miguel0888 This is an unofficial mirror of the actual code; a PR here will have no effect, unfortunately. You should open a pull request at https://github.com/JetBrains/intellij-community/tree/master/plugins/java-decompiler/engine

Miguel0888:fix/zip-entry-traversal-check

Fix zip slip vulnerability by normalizing entry names in StructContext

a3d9e89

(cherry picked from commit 226a520)

Copilot AI review requested due to automatic review settings December 10, 2025 21:02

Copilot AI reviewed Dec 10, 2025

View reviewed changes

Miguel0888 mentioned this pull request Dec 21, 2025

Fix false-positive ZipEntry traversal check (from fernflower#22) Miguel0888/intellij-community#1

Closed

Miguel0888 added a commit to Miguel0888/intellij-community that referenced this pull request Dec 21, 2025

Fix ZipEntry traversal check (from JetBrains/fernflower#22)

1a1fcf7

Miguel0888 closed this Dec 21, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Fix zip slip check in StructContext on Windows #22

Fix zip slip check in StructContext on Windows #22

Uh oh!

Miguel0888 commented Dec 10, 2025

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Dec 10, 2025

Uh oh!

Copilot AI Dec 10, 2025

Uh oh!

Phoenixcwx commented Dec 10, 2025 via email

Uh oh!

uRyanxD commented Dec 13, 2025

Uh oh!

Miguel0888 commented Dec 21, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

	if (normalizedName.startsWith("/") \|\| normalizedName.startsWith("../") \|\| normalizedName.contains("/../")) {
	if (normalizedName.startsWith("/") \|\| normalizedName.startsWith("..") \|\| normalizedName.contains("/../")) {

Fix zip slip check in StructContext on Windows #22

Fix zip slip check in StructContext on Windows #22

Uh oh!

Conversation

Miguel0888 commented Dec 10, 2025

Summary

Problem / Error message

Root cause

Fix

Security considerations

Impact

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Dec 10, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 10, 2025

Choose a reason for hiding this comment

Uh oh!

Phoenixcwx commented Dec 10, 2025 via email

Uh oh!

uRyanxD commented Dec 13, 2025

Uh oh!

Miguel0888 commented Dec 21, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants