CVE-2018-1002201: Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal') (CWE-22)
value: zt-zip before 1.13 is vulnerable to directory traversal, allowing attackers to write to arbitrary files via a ../ (dot dot slash) in a Zip archive entry that is mishandled during extraction. This vulnerability is also known as 'Zip-Slip'.
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: 5.5
Published: 2018-07-25T17:29Z
Repository: https://github.com/zeroturnaround/zt-zip
Language: Java
Files changed: src/main/java/org/zeroturnaround/zip/ZipUtil.java, src/test/java/org/zeroturnaround/zip/DirectoryTraversalMaliciousTest.java
Commit date: 2018-04-26 17:52:37 +0300
Commit message: Fixed potential security vulnerability reported by Snyk Security Research Team

This is an arbitrary file write vulnerability, that can be achieved using a specially crafted zip archive, that holds path traversal filenames. So when the filename gets concatenated to the target extraction directory, the final path ends up outside of the target folder.

Diff: diff --git a/src/main/java/org/zeroturnaround/zip/ZipUtil.java b/src/main/java/org/zeroturnaround/zip/ZipUtil.java
index 7ebc8dd..604866e 100644
--- a/src/main/java/org/zeroturnaround/zip/ZipUtil.java
+++ b/src/main/java/org/zeroturnaround/zip/ZipUtil.java
@@ -1148,10 +1148,19 @@ public final class ZipUtil {
 
     public void process(InputStream in, ZipEntry zipEntry) throws IOException {
       String name = mapper.map(zipEntry.getName());
       if (name != null) {
         File file = new File(outputDir, name);
+
+        /* If we see the relative traversal string of ".." we need to make sure
+         * that the outputdir + name doesn't leave the outputdir. See
+         * DirectoryTraversalMaliciousTest for details.
+         */
+        if (name.indexOf("..") != -1 && !file.getCanonicalPath().startsWith(outputDir.getCanonicalPath())) {
+          throw new ZipException("The file "+name+" is trying to leave the target output directory of "+outputDir+". Ignoring this file.");
+        }
+
         if (zipEntry.isDirectory()) {
           FileUtils.forceMkdir(file);
         }
         else {
           FileUtils.forceMkdir(file.getParentFile());
@@ -1216,15 +1225,33 @@ public final class ZipUtil {
               FileUtils.forceMkdir(file);
             }
             parentDirectory = file;
           }
           File destFile = new File(parentDirectory, dirs[dirs.length - 1]);
+
+          /* If we see the relative traversal string of ".." we need to make sure
+           * that the outputdir + name doesn't leave the outputdir. See
+           * DirectoryTraversalMaliciousTest for details.
+           */
+          if (name.indexOf("..") != -1 && !destFile.getCanonicalPath().startsWith(outputDir.getCanonicalPath())) {
+            throw new ZipException("The file "+name+" is trying to leave the target output directory of "+outputDir+". Ignoring this file.");
+          }
+
           FileUtils.copy(in, destFile);
         }
         // it could be that there are just top level files that the unpacker is used for
         else {
           File destFile = new File(outputDir, name);
+
+          /* If we see the relative traversal string of ".." we need to make sure
+           * that the outputdir + name doesn't leave the outputdir. See
+           * DirectoryTraversalMaliciousTest for details.
+           */
+          if (name.indexOf("..") != -1 && !destFile.getCanonicalPath().startsWith(outputDir.getCanonicalPath())) {
+            throw new ZipException("The file "+name+" is trying to leave the target output directory of "+outputDir+". Ignoring this file.");
+          }
+
           FileUtils.copy(in, destFile);
         }
       }
     }
   }
@@ -1256,10 +1283,19 @@ public final class ZipUtil {
       }
 
       String name = mapper.map(getUnrootedName(root, zipEntry.getName()));
       if (name != null) {
         File file = new File(outputDir, name);
+
+        /* If we see the relative traversal string of ".." we need to make sure
+         * that the outputdir + name doesn't leave the outputdir. See
+         * DirectoryTraversalMaliciousTest for details.
+         */
+        if (name.indexOf("..") != -1 && !file.getCanonicalPath().startsWith(outputDir.getCanonicalPath())) {
+          throw new ZipException("The file "+name+" is trying to leave the target output directory of "+outputDir+". Ignoring this file.");
+        }
+
         if (zipEntry.isDirectory()) {
           FileUtils.forceMkdir(file);
         }
         else {
           FileUtils.forceMkdir(file.getParentFile());
diff --git a/src/test/java/org/zeroturnaround/zip/DirectoryTraversalMaliciousTest.java b/src/test/java/org/zeroturnaround/zip/DirectoryTraversalMaliciousTest.java
new file mode 100644
index 0000000..8eede3d
--- /dev/null
+++ b/src/test/java/org/zeroturnaround/zip/DirectoryTraversalMaliciousTest.java
@@ -0,0 +1,61 @@
+package org.zeroturnaround.zip;
+
+/**
+ *    Copyright (C) 2012 ZeroTurnaround LLC <support@zeroturnaround.com>
+ *
+ *    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.
+ */
+import java.io.File;
+
+import junit.framework.TestCase;
+
+public class DirectoryTraversalMaliciousTest extends TestCase {
+  /*
+   * This is the contents of the file. There is one evil file that tries to get out of the
+   * target.
+   *
+   * $ unzip -t zip-slip.zip
+   * Archive: zip-slip.zip
+   * testing: good.txt OK
+   * testing: ../../../../../../../../../../../../../../../../../../../../../../../../../../../../../../../../../../../../../../../../tmp/evil.txt OK
+   * No errors detected in compressed data of zip-slip.zip.
+   */
+  private static final File badFile = new File("src/test/resources/zip-malicious-traversal.zip");
+  private static final File badFileBackslashes = new File("src/test/resources/zip-malicious-traversal-backslashes.zip");
+
+  public void testUnpackDoesntLeaveTarget() throws Exception {
+    File file = File.createTempFile("temp", null);
+    File tmpDir = file.getParentFile();
+
+    try {
+      ZipUtil.unpack(badFile, tmpDir);
+      fail();
+    }
+    catch (ZipException e) {
+      assertTrue(true);
+    }
+  }
+
+  public void testUnwrapDoesntLeaveTarget() throws Exception {
+    File file = File.createTempFile("temp", null);
+    File tmpDir = file.getParentFile();
+
+    try {
+      ZipUtil.iterate(badFileBackslashes, new ZipUtil.BackslashUnpacker(tmpDir));
+      fail();
+    }
+    catch (ZipException e) {
+      assertTrue(true);
+    }
+  }
+}
diff --git a/src/test/resources/zip-malicious-traversal-backslashes.zip b/src/test/resources/zip-malicious-traversal-backslashes.zip
new file mode 100644
index 0000000..a67ff8b
Binary files /dev/null and b/src/test/resources/zip-malicious-traversal-backslashes.zip differ
diff --git a/src/test/resources/zip-malicious-traversal.zip b/src/test/resources/zip-malicious-traversal.zip
new file mode 100644
index 0000000..38b3f49
Binary files /dev/null and b/src/test/resources/zip-malicious-traversal.zip differ

Fixed code: 
        /* If we see the relative traversal string of ".." we need to make sure
         * that the outputdir + name doesn't leave the outputdir. See
         * DirectoryTraversalMaliciousTest for details.
         */
        if (name.indexOf("..") != -1 && !file.getCanonicalPath().startsWith(outputDir.getCanonicalPath())) {
          throw new ZipException("The file "+name+" is trying to leave the target output directory of "+outputDir+". Ignoring this file.");
        }


          /* If we see the relative traversal string of ".." we need to make sure
           * that the outputdir + name doesn't leave the outputdir. See
           * DirectoryTraversalMaliciousTest for details.
           */
          if (name.indexOf("..") != -1 && !destFile.getCanonicalPath().startsWith(outputDir.getCanonicalPath())) {
            throw new ZipException("The file "+name+" is trying to leave the target output directory of "+outputDir+". Ignoring this file.");
          }


          /* If we see the relative traversal string of ".." we need to make sure
           * that the outputdir + name doesn't leave the outputdir. See
           * DirectoryTraversalMaliciousTest for details.
           */
          if (name.indexOf("..") != -1 && !destFile.getCanonicalPath().startsWith(outputDir.getCanonicalPath())) {
            throw new ZipException("The file "+name+" is trying to leave the target output directory of "+outputDir+". Ignoring this file.");
          }


        /* If we see the relative traversal string of ".." we need to make sure
         * that the outputdir + name doesn't leave the outputdir. See
         * DirectoryTraversalMaliciousTest for details.
         */
        if (name.indexOf("..") != -1 && !file.getCanonicalPath().startsWith(outputDir.getCanonicalPath())) {
          throw new ZipException("The file "+name+" is trying to leave the target output directory of "+outputDir+". Ignoring this file.");
        }

package org.zeroturnaround.zip;

/**
 *    Copyright (C) 2012 ZeroTurnaround LLC <support@zeroturnaround.com>
 *
 *    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.
 */
import java.io.File;

import junit.framework.TestCase;

public class DirectoryTraversalMaliciousTest extends TestCase {
  /*
   * This is the contents of the file. There is one evil file that tries to get out of the
   * target.
   *
   * $ unzip -t zip-slip.zip
   * Archive: zip-slip.zip
   * testing: good.txt OK
   * testing: ../../../../../../../../../../../../../../../../../../../../../../../../../../../../../../../../../../../../../../../../tmp/evil.txt OK
   * No errors detected in compressed data of zip-slip.zip.
   */
  private static final File badFile = new File("src/test/resources/zip-malicious-traversal.zip");
  private static final File badFileBackslashes = new File("src/test/resources/zip-malicious-traversal-backslashes.zip");

  public void testUnpackDoesntLeaveTarget() throws Exception {
    File file = File.createTempFile("temp", null);
    File tmpDir = file.getParentFile();

    try {
      ZipUtil.unpack(badFile, tmpDir);
      fail();
    }
    catch (ZipException e) {
      assertTrue(true);
    }
  }

  public void testUnwrapDoesntLeaveTarget() throws Exception {
    File file = File.createTempFile("temp", null);
    File tmpDir = file.getParentFile();

    try {
      ZipUtil.iterate(badFileBackslashes, new ZipUtil.BackslashUnpacker(tmpDir));
      fail();
    }
    catch (ZipException e) {
      assertTrue(true);
    }
  }
}