Skip to content

Commit

Permalink
Fixes zip-slip vulnerability
Browse files Browse the repository at this point in the history
Fixes sbt#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.
  • Loading branch information
eed3si9n committed Oct 22, 2023
1 parent dee89c8 commit f928cdd
Show file tree
Hide file tree
Showing 3 changed files with 31 additions and 8 deletions.
24 changes: 16 additions & 8 deletions io/src/main/scala/sbt/io/IO.scala
Original file line number Diff line number Diff line change
Expand Up @@ -387,35 +387,43 @@ 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)
()
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 + "'")
}
Expand All @@ -424,7 +432,7 @@ object IO {
}
}
next()
Set() ++ set
(Set() ++ set).map(_.toFile)
}

// TODO: provide a better API to download things.
Expand Down
Binary file added io/src/test/resources/zip-slip.zip
Binary file not shown.
15 changes: 15 additions & 0 deletions io/src/test/scala/sbt/io/FileUtilitiesSpecification.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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")))

Expand Down

0 comments on commit f928cdd

Please sign in to comment.