From f928cdd8aebc5a2b85c326cc267e698229e0b7b2 Mon Sep 17 00:00:00 2001
From: Eugene Yokota <eed3si9n@gmail.com>
Date: Sun, 22 Oct 2023 04:42:16 -0400
Subject: [PATCH] Fixes zip-slip vulnerability

Fixes https://github.com/sbt/io/issues/358
Ref codehaus-plexus/plexus-archiver 87

**Problem**
IO.unzip currently has zip-slip vulnerability, which can write arbitrary
files on the machine using specially crafted zip archive that holds path
traversal file names.

**Solution**
This replicates the fix originally sent to plex-archiver by Snyk Team.
---
 io/src/main/scala/sbt/io/IO.scala             |  24 ++++++++++++------
 io/src/test/resources/zip-slip.zip            | Bin 0 -> 545 bytes
 .../sbt/io/FileUtilitiesSpecification.scala   |  15 +++++++++++
 3 files changed, 31 insertions(+), 8 deletions(-)
 create mode 100644 io/src/test/resources/zip-slip.zip

diff --git a/io/src/main/scala/sbt/io/IO.scala b/io/src/main/scala/sbt/io/IO.scala
index 3172fac5..bdd28035 100644
--- a/io/src/main/scala/sbt/io/IO.scala
+++ b/io/src/main/scala/sbt/io/IO.scala
@@ -387,16 +387,23 @@ object IO {
       preserveLastModified: Boolean = true
   ): Set[File] = {
     createDirectory(toDirectory)
-    zipInputStream(from)(zipInput => extract(zipInput, toDirectory, filter, preserveLastModified))
+    zipInputStream(from)(zipInput =>
+      extract(zipInput, toDirectory.toPath, filter, preserveLastModified)
+    )
   }
 
   private def extract(
       from: ZipInputStream,
-      toDirectory: File,
+      toDirectory: NioPath,
       filter: NameFilter,
       preserveLastModified: Boolean
   ) = {
-    val set = new HashSet[File]
+    val set = new HashSet[NioPath]
+    val canonicalDirPath = toDirectory.normalize().toString
+    def validateExtractPath(name: String, target: NioPath): Unit =
+      if (!target.normalize().toString.startsWith(canonicalDirPath)) {
+        throw new RuntimeException(s"Entry ($name) is outside of the target directory")
+      }
     @tailrec def next(): Unit = {
       val entry = from.getNextEntry
       if (entry == null)
@@ -404,18 +411,19 @@ object IO {
       else {
         val name = entry.getName
         if (filter.accept(name)) {
-          val target = new File(toDirectory, name)
+          val target = toDirectory.resolve(name)
+          validateExtractPath(name, target)
           // log.debug("Extracting zip entry '" + name + "' to '" + target + "'")
           if (entry.isDirectory)
-            createDirectory(target)
+            createDirectory(target.toFile)
           else {
             set += target
             translate("Error extracting zip entry '" + name + "' to '" + target + "': ") {
-              fileOutputStream(false)(target)(out => transfer(from, out))
+              fileOutputStream(false)(target.toFile)(out => transfer(from, out))
             }
           }
           if (preserveLastModified)
-            setModifiedTimeOrFalse(target, entry.getTime)
+            setModifiedTimeOrFalse(target.toFile, entry.getTime)
         } else {
           // log.debug("Ignoring zip entry '" + name + "'")
         }
@@ -424,7 +432,7 @@ object IO {
       }
     }
     next()
-    Set() ++ set
+    (Set() ++ set).map(_.toFile)
   }
 
   // TODO: provide a better API to download things.
diff --git a/io/src/test/resources/zip-slip.zip b/io/src/test/resources/zip-slip.zip
new file mode 100644
index 0000000000000000000000000000000000000000..38b3f499de0163e62ca15ce18350a9d9a477a51b
GIT binary patch
literal 545
zcmWIWW@h1H0D=Au{XYEp{-1?`Y!K#PkYPyA&ri`SsVE5z;bdU8U359h4v0%DxEUB(
zzA-W|u!sQFm1JZVD*#cV0!Xz&eqJh90MJm76a&LlprHwl)s`S02)6*So}T`Ippx7I
z{nWC|9FT|Lj?Pm62|-=W$Rx*%D=;L0E@xl>dYWNLBZ!3v8dgZqpan~SHzSh>Gwx6T
jnE?Vz8bg8PfCLE8QsgiR@MdKLxrhk}K_2A>d6oeH^pk5C

literal 0
HcmV?d00001

diff --git a/io/src/test/scala/sbt/io/FileUtilitiesSpecification.scala b/io/src/test/scala/sbt/io/FileUtilitiesSpecification.scala
index 394aacb6..d7c378ca 100644
--- a/io/src/test/scala/sbt/io/FileUtilitiesSpecification.scala
+++ b/io/src/test/scala/sbt/io/FileUtilitiesSpecification.scala
@@ -24,6 +24,7 @@ object WriteContentSpecification extends Properties("Write content") {
   property("Append string appends") = forAll(appendAndCheckStrings _)
   property("Append bytes appends") = forAll(appendAndCheckBytes _)
   property("Unzip doesn't stack overflow") = largeUnzip()
+  property("Unzip errors given parent traversal") = testZipSlip()
 
   implicit lazy val validChar: Arbitrary[Char] = Arbitrary(
     for (i <- Gen.choose(0, 0xd7ff)) yield i.toChar
@@ -39,6 +40,20 @@ object WriteContentSpecification extends Properties("Write content") {
     true
   }
 
+  private def testZipSlip() = {
+    val badFile0 = new File("io/src/test/resources/zip-slip.zip")
+    val badFile1 = new File("src/test/resources/zip-slip.zip")
+    val badFile =
+      if (badFile0.exists()) badFile0
+      else badFile1
+    try {
+      unzipFile(badFile)
+      false
+    } catch {
+      case e: RuntimeException => e.getMessage.contains("outside of the target directory")
+    }
+  }
+
   private def testUnzip[T](implicit mf: Manifest[T]) =
     unzipFile(IO.classLocationFileOption(mf.runtimeClass).getOrElse(sys.error(s"$mf")))