From 62face8c26258c10e56db7020f17d11e8b4736f3 Mon Sep 17 00:00:00 2001
From: Odinn <odinn@Odinns-MacBook-Pro.local>
Date: Sun, 6 May 2018 01:16:55 +0300
Subject: [PATCH] fix: fail when trying to extract outside of dest dir

A well crafted zip file may cause the code to extract outside of the destination dir.
This PR fails when that happens so that no unexpected behaviour happens.
---
 .../plexus/archiver/zip/AbstractZipUnArchiver.java |   9 ++++++++
 .../plexus/archiver/zip/ZipUnArchiverTest.java     |  25 +++++++++++++++++++++
 src/test/zips/zip-slip.zip                         | Bin 0 -> 545 bytes
 3 files changed, 34 insertions(+)
 create mode 100644 src/test/zips/zip-slip.zip

diff --git a/src/main/java/org/codehaus/plexus/archiver/zip/AbstractZipUnArchiver.java b/src/main/java/org/codehaus/plexus/archiver/zip/AbstractZipUnArchiver.java
index fb8759e..d4c20aa 100644
--- a/src/main/java/org/codehaus/plexus/archiver/zip/AbstractZipUnArchiver.java
+++ b/src/main/java/org/codehaus/plexus/archiver/zip/AbstractZipUnArchiver.java
@@ -198,6 +198,15 @@ protected void extractFile( final File srcF, final File dir, final InputStream c
     {
         final File f = FileUtils.resolveFile( dir, entryName );
 
+        // Make sure that the resolved path of the extracted file doesn't escape the destination directory
+        String canonicalDirPath = dir.getCanonicalPath();
+        String canonicalDestPath = f.getCanonicalPath();
+
+        if ( !canonicalDestPath.startsWith( canonicalDirPath ) )
+        {
+            throw new ArchiverException( "Entry is outside of the target directory (" + entryName + ")" );
+        }
+
         try
         {
             if ( !isOverwrite() && f.exists() && ( f.lastModified() >= entryDate.getTime() ) )
diff --git a/src/test/java/org/codehaus/plexus/archiver/zip/ZipUnArchiverTest.java b/src/test/java/org/codehaus/plexus/archiver/zip/ZipUnArchiverTest.java
index 4d4d475..b22359c 100644
--- a/src/test/java/org/codehaus/plexus/archiver/zip/ZipUnArchiverTest.java
+++ b/src/test/java/org/codehaus/plexus/archiver/zip/ZipUnArchiverTest.java
@@ -121,4 +121,29 @@ public void testSelectors()
         fileSelector.setExcludes( new String[]{ "resources/artifactId/directory/test.properties" } );
         runUnarchiver( "", new FileSelector[]{ fileSelector }, new boolean[]{ true, false, false } );
     }
+
+    public void testExtractingZipWithEntryOutsideDestDirThrowsException()
+            throws Exception
+    {
+        Exception ex = null;
+        String s = "target/zip-unarchiver-slip-tests";
+        File testZip = new File( getBasedir(), "src/test/zips/zip-slip.zip" );
+        File outputDirectory = new File( getBasedir(), s );
+
+        FileUtils.deleteDirectory( outputDirectory );
+
+        try
+        {
+            ZipUnArchiver zu = (ZipUnArchiver) lookup( UnArchiver.ROLE, "zip" );
+            zu.setSourceFile( testZip );
+            zu.extract( "", outputDirectory );
+        }
+        catch ( Exception e )
+        {
+            ex = e;
+        }
+
+        assertNotNull( ex );
+        assertTrue( ex.getMessage().startsWith( "Entry is outside of the target directory" ) );
+    }
 }
diff --git a/src/test/zips/zip-slip.zip b/src/test/zips/zip-slip.zip
new file mode 100644
index 0000000..38b3f49
--- /dev/null
+++ b/src/test/zips/zip-slip.zip
@@ -0,0 +1,5 @@
+PK
+     °LoOó        good.txtUT	 =¢ÓZ>¢ÓZux ö     this is a good one
+PK     •°L`A{9      „   ../../../../../../../../../../../../../../../../../../../../../../../../../../../../../../../../../../../../../../../../tmp/evil.txtthis is an evil one
+PK
+     °LoOó               ¤    good.txtUT =¢ÓZux ö     PK     •°L`A{9      „           ¤U   ../../../../../../../../../../../../../../../../../../../../../../../../../../../../../../../../../../../../../../../../tmp/evil.txtPK             
\ No newline at end of file
-- 
2.14.3