Skip to content

Commit 43f1e24

Browse files
committed
Fixes zip-slip vulnerability
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.
1 parent dee89c8 commit 43f1e24

File tree

3 files changed

+27
-8
lines changed

3 files changed

+27
-8
lines changed

io/src/main/scala/sbt/io/IO.scala

Lines changed: 16 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -387,35 +387,43 @@ object IO {
387387
preserveLastModified: Boolean = true
388388
): Set[File] = {
389389
createDirectory(toDirectory)
390-
zipInputStream(from)(zipInput => extract(zipInput, toDirectory, filter, preserveLastModified))
390+
zipInputStream(from)(zipInput =>
391+
extract(zipInput, toDirectory.toPath, filter, preserveLastModified)
392+
)
391393
}
392394

393395
private def extract(
394396
from: ZipInputStream,
395-
toDirectory: File,
397+
toDirectory: NioPath,
396398
filter: NameFilter,
397399
preserveLastModified: Boolean
398400
) = {
399-
val set = new HashSet[File]
401+
val set = new HashSet[NioPath]
402+
val canonicalDirPath = toDirectory.normalize().toString
403+
def validateExtractPath(name: String, target: NioPath): Unit =
404+
if (!target.normalize().toString.startsWith(canonicalDirPath)) {
405+
throw new RuntimeException(s"Entry ($name) is outside of the target directory")
406+
}
400407
@tailrec def next(): Unit = {
401408
val entry = from.getNextEntry
402409
if (entry == null)
403410
()
404411
else {
405412
val name = entry.getName
406413
if (filter.accept(name)) {
407-
val target = new File(toDirectory, name)
414+
val target = toDirectory.resolve(name)
415+
validateExtractPath(name, target)
408416
// log.debug("Extracting zip entry '" + name + "' to '" + target + "'")
409417
if (entry.isDirectory)
410-
createDirectory(target)
418+
createDirectory(target.toFile)
411419
else {
412420
set += target
413421
translate("Error extracting zip entry '" + name + "' to '" + target + "': ") {
414-
fileOutputStream(false)(target)(out => transfer(from, out))
422+
fileOutputStream(false)(target.toFile)(out => transfer(from, out))
415423
}
416424
}
417425
if (preserveLastModified)
418-
setModifiedTimeOrFalse(target, entry.getTime)
426+
setModifiedTimeOrFalse(target.toFile, entry.getTime)
419427
} else {
420428
// log.debug("Ignoring zip entry '" + name + "'")
421429
}
@@ -424,7 +432,7 @@ object IO {
424432
}
425433
}
426434
next()
427-
Set() ++ set
435+
(Set() ++ set).map(_.toFile)
428436
}
429437

430438
// TODO: provide a better API to download things.

io/src/test/resources/zip-slip.zip

545 Bytes
Binary file not shown.

io/src/test/scala/sbt/io/FileUtilitiesSpecification.scala

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ object WriteContentSpecification extends Properties("Write content") {
2424
property("Append string appends") = forAll(appendAndCheckStrings _)
2525
property("Append bytes appends") = forAll(appendAndCheckBytes _)
2626
property("Unzip doesn't stack overflow") = largeUnzip()
27+
property("Unzip errors given parent traversal") = testZipSlip()
2728

2829
implicit lazy val validChar: Arbitrary[Char] = Arbitrary(
2930
for (i <- Gen.choose(0, 0xd7ff)) yield i.toChar
@@ -39,6 +40,16 @@ object WriteContentSpecification extends Properties("Write content") {
3940
true
4041
}
4142

43+
private def testZipSlip() = {
44+
val badFile = new File("io/src/test/resources/zip-slip.zip")
45+
try {
46+
unzipFile(badFile)
47+
false
48+
} catch {
49+
case e: RuntimeException => e.getMessage.contains("outside of the target directory")
50+
}
51+
}
52+
4253
private def testUnzip[T](implicit mf: Manifest[T]) =
4354
unzipFile(IO.classLocationFileOption(mf.runtimeClass).getOrElse(sys.error(s"$mf")))
4455

0 commit comments

Comments
 (0)