[klib] Validate paths when unpacking zip archives

compiler/util-io/src/org/jetbrains/kotlin/konan/file/File.kt

@@ -13,9 +13,7 @@ import java.io.RandomAccessFile
 import java.nio.MappedByteBuffer
 import java.nio.channels.FileChannel
 import java.nio.file.*
-import java.nio.file.attribute.BasicFileAttributeView
 import java.nio.file.attribute.BasicFileAttributes
-import java.nio.file.attribute.FileTime
 
 data class File constructor(internal val javaPath: Path) {
     constructor(parent: Path, child: String): this(parent.resolve(child))
@@ -64,12 +62,6 @@ data class File constructor(internal val javaPath: Path) {
         Files.copy(javaPath, destination.javaPath, StandardCopyOption.REPLACE_EXISTING)
     }
 
-    fun recursiveCopyTo(destination: File, resetTimeAttributes: Boolean = false) {
-        val sourcePath = javaPath
-        val destPath = destination.javaPath
-        sourcePath.recursiveCopyTo(destPath, resetTimeAttributes = resetTimeAttributes)
-    }
-
     fun renameTo(destination: File) = javaPath.toFile().renameTo(destination.javaPath.toFile())
 
     fun mkdirs() = Files.createDirectories(javaPath)
@@ -198,30 +190,6 @@ fun createTempFile(name: String, suffix: String? = null)
 fun createTempDir(name: String): File
         = Files.createTempDirectory(name).File()
 
-fun Path.recursiveCopyTo(destPath: Path, resetTimeAttributes: Boolean = false) {
-    val sourcePath = this
-    Files.walk(sourcePath).forEach next@ { oldPath ->
-
-        val relative = sourcePath.relativize(oldPath)
-        val destFs = destPath.getFileSystem()
-        // We are copying files between file systems, 
-        // so pass the relative path through the String.
-        val newPath = destFs.getPath(destPath.toString(), relative.toString())
-
-        // File systems don't allow replacing an existing root.
-        if (newPath == newPath.getRoot()) return@next
-        if (Files.isDirectory(newPath)) {
-            Files.createDirectories(newPath)
-        } else {
-            Files.copy(oldPath, newPath, StandardCopyOption.REPLACE_EXISTING)
-        }
-        if (resetTimeAttributes) {
-            val zero = FileTime.fromMillis(0)
-            Files.getFileAttributeView(newPath, BasicFileAttributeView::class.java).setTimes(zero, zero, zero);
-        }
-    }
-}
-
 fun bufferedReader(errorStream: InputStream?) = BufferedReader(InputStreamReader(errorStream))
 
 // stdlib `use` function adapted for AutoCloseable.
@@ -241,4 +209,4 @@ inline fun <T : AutoCloseable?, R> T.use(block: (T) -> R): R {
             this?.close()
         }
     }
-}
+}

compiler/util-io/src/org/jetbrains/kotlin/konan/file/ZipUtil.kt

@@ -5,9 +5,11 @@
 
 package org.jetbrains.kotlin.konan.file
 
-import java.net.URI
 import java.nio.file.*
+import java.nio.file.attribute.BasicFileAttributeView
+import java.nio.file.attribute.FileTime
 import java.nio.file.spi.FileSystemProvider
+import java.util.zip.ZipException
 
 // Zip filesystem provider doesn't allow creating several instances of ZipFileSystem from the same URI,
 // so newFileSystem(URI, ...) throws a FileSystemAlreadyExistsException in this case.
@@ -15,7 +17,7 @@ import java.nio.file.spi.FileSystemProvider
 // See also:
 // https://bugs.java.com/bugdatabase/view_bug.do?bug_id=7001822
 // https://bugs.java.com/bugdatabase/view_bug.do?bug_id=6994161
-fun File.zipFileSystem(create: Boolean = false): FileSystem {
+private fun File.zipFileSystem(create: Boolean = false): FileSystem {
     val attributes = hashMapOf("create" to create.toString())
 
     // There is no FileSystems.newFileSystem overload accepting the attribute map.
@@ -47,16 +49,67 @@ fun File.zipDirAs(unixFile: File) {
     }
 }
 
-fun Path.unzipTo(directory: Path) {
+/**
-    val zipUri = URI.create("jar:" + this.toUri())
+ * Unpacks the contents of a zip archive located in [this] into the [destinationDirectory].
-    FileSystems.newFileSystem(zipUri, emptyMap<String, Any?>(), null).use { zipfs ->
+ *
-        val zipPath = zipfs.getPath("/")
+ * @param destinationDirectory The directory to unpack the contents to.
-        zipPath.recursiveCopyTo(directory)
+ * @param resetTimeAttributes Whether to set the newly created files' time attributes
+ * (creation time, last access time, and last modification time) to zero.
+ * @param fromSubdirectory A subdirectory inside the archive to unpack. Specify "/" if you need to unpack the whole archive.
+ */
+fun File.unzipTo(destinationDirectory: File, fromSubdirectory: File = File("/"), resetTimeAttributes: Boolean = false) {
+    withZipFileSystem {
+        it.file(fromSubdirectory).recursiveCopyTo(destinationDirectory, resetTimeAttributes)
     }
 }
 
+/**
+ * Unpacks the contents of a zip archive located in [this] into the [destinationDirectory].
+ *
+ * @param destinationDirectory The directory to unpack the contents to.
+ * @param resetTimeAttributes Whether to set the newly created files' time attributes
+ * (creation time, last access time, and last modification time) to zero.
+ * @param fromSubdirectory A subdirectory inside the archive to unpack. Specify "/" if you need to unpack the whole archive.
+ */
+fun Path.unzipTo(destinationDirectory: Path, fromSubdirectory: Path = Paths.get("/"), resetTimeAttributes: Boolean = false) {
+    File(this).unzipTo(File(destinationDirectory), File(fromSubdirectory), resetTimeAttributes)
+}
+
 fun <T> File.withZipFileSystem(create: Boolean, action: (FileSystem) -> T): T {
     return this.zipFileSystem(create).use(action)
 }
 
 fun <T> File.withZipFileSystem(action: (FileSystem) -> T): T = this.withZipFileSystem(false, action)
+
+// TODO: Make this function private after boostrap advance
+fun File.recursiveCopyTo(destination: File, resetTimeAttributes: Boolean = false) {
+    val sourcePath = javaPath
+    val destPath = destination.javaPath
+    val destFs = destPath.fileSystem
+    val normalizedDestPath = destPath.normalize()
+    Files.walk(sourcePath).forEach next@{ oldPath ->
+
+        val relative = sourcePath.relativize(oldPath)
+
+        // We are copying files between file systems,
+        // so pass the relative path through the String.
+        val newPath = destFs.getPath(destPath.toString(), relative.toString())
+
+        // NOTE: this check is important, it prevents a potential ZipSlip vulnerability
+        if (!newPath.normalize().startsWith(normalizedDestPath)) {
+            throw ZipException("$relative attempted to escape the destination directory $destination")
+        }
+
+        // File systems don't allow replacing an existing root.
+        if (newPath == newPath.root) return@next
+        if (Files.isDirectory(newPath)) {
+            Files.createDirectories(newPath)
+        } else {
+            Files.copy(oldPath, newPath, StandardCopyOption.REPLACE_EXISTING)
+        }
+        if (resetTimeAttributes) {
+            val zero = FileTime.fromMillis(0)
+            Files.getFileAttributeView(newPath, BasicFileAttributeView::class.java).setTimes(zero, zero, zero);
+        }
+    }
+}

compiler/util-klib/src/org/jetbrains/kotlin/library/KotlinLibraryUtils.kt

@@ -2,6 +2,7 @@ package org.jetbrains.kotlin.library
 
 import org.jetbrains.kotlin.konan.file.File
 import org.jetbrains.kotlin.konan.file.file
+import org.jetbrains.kotlin.konan.file.recursiveCopyTo
 import org.jetbrains.kotlin.konan.file.withZipFileSystem
 import org.jetbrains.kotlin.library.impl.zippedKotlinLibraryChecks
 
@@ -23,6 +24,7 @@ fun File.unpackZippedKonanLibraryTo(newDir: File) {
             newDir.delete()
     }
 
+    // TODO: Replace this with this.unzipTo(newDir) after bootstrap advance
     this.withZipFileSystem {
         it.file("/").recursiveCopyTo(newDir)
     }
@@ -32,4 +34,4 @@ fun File.unpackZippedKonanLibraryTo(newDir: File) {
 val List<String>.toUnresolvedLibraries
     get() = this.map {
         UnresolvedLibrary(it, null)
-    }
+    }

compiler/util-klib/src/org/jetbrains/kotlin/library/impl/KotlinLibraryLayoutImpl.kt

@@ -2,6 +2,8 @@ package org.jetbrains.kotlin.library.impl
 
 import org.jetbrains.kotlin.konan.file.File
 import org.jetbrains.kotlin.konan.file.file
+import org.jetbrains.kotlin.konan.file.unzipTo
+import org.jetbrains.kotlin.konan.file.recursiveCopyTo
 import org.jetbrains.kotlin.konan.file.withZipFileSystem
 import org.jetbrains.kotlin.library.*
 import org.jetbrains.kotlin.util.removeSuffixIfPresent
@@ -109,6 +111,14 @@ private fun extract(zipFile: File, file: File) = zipFile.withZipFileSystem { zip
 
 fun KotlinLibraryLayoutImpl.extractDir(directory: File): File = extractDir(this.klib, directory)
 
+// TODO: Use this implementation after bootstrap advance
+//private fun extractDir(zipFile: File, directory: File): File {
+//    val temporary = org.jetbrains.kotlin.konan.file.createTempDir(directory.name)
+//    temporary.deleteOnExitRecursively()
+//    zipFile.unzipTo(temporary, fromSubdirectory = directory)
+//    return temporary
+//}
+
 private fun extractDir(zipFile: File, directory: File): File = zipFile.withZipFileSystem { zipFileSystem ->
     val temporary = org.jetbrains.kotlin.konan.file.createTempDir(directory.name)
     zipFileSystem.file(directory).recursiveCopyTo(temporary)
@@ -162,4 +172,4 @@ internal fun zippedKotlinLibraryChecks(klibFile: File) {
     check(extension.isEmpty() || extension == KLIB_FILE_EXTENSION || extension == "jar") {
         "KLIB path has unexpected extension: $klibFile"
     }
-}
+}

kotlin-native/klib/src/test/kotlin/org/jetbrains/kotlin/cli/klib/test/ZipTest.kt

@@ -0,0 +1,71 @@
+/*
+ * Copyright 2010-2022 JetBrains s.r.o. and Kotlin Programming Language contributors.
+ * Use of this source code is governed by the Apache 2.0 license that can be found in the license/LICENSE.txt file.
+ */
+
+package org.jetbrains.kotlin.cli.klib.test
+
+import org.jetbrains.kotlin.konan.file.File
+import org.jetbrains.kotlin.konan.file.unzipTo
+import org.jetbrains.kotlin.konan.file.use
+import org.jetbrains.kotlin.library.impl.javaFile
+import org.junit.After
+import org.junit.Before
+import org.junit.Rule
+import org.junit.Test
+import org.junit.rules.TestName
+import java.io.FileOutputStream
+import java.io.IOException
+import java.util.zip.ZipEntry
+import java.util.zip.ZipException
+import java.util.zip.ZipOutputStream
+import kotlin.test.assertFailsWith
+
+class ZipTest {
+
+    @Rule
+    @JvmField
+    val currentTestName = TestName()
+
+    private lateinit var tmpDir: File
+
+    @Before
+    fun setUp() {
+        tmpDir = org.jetbrains.kotlin.konan.file.createTempDir(currentTestName.methodName)
+        tmpDir.deleteOnExitRecursively()
+    }
+
+    @Test
+    fun testZipSlip() {
+        // https://security.snyk.io/research/zip-slip-vulnerability
+        val zipArchive = tmpDir.child("sneaky.klib")
+
+        createMaliciousArchive(zipArchive)
+
+        try {
+            zipArchive.unzipTo(tmpDir.child("unpacked"))
+        } catch (e: Exception) {
+            if (e !is IOException && e.cause !is IOException) throw e
+        }
+
+        assert(!tmpDir.child("definitelySafe.txt").exists) { "ZipSlip vulnerability found!" }
+    }
+
+    @Test
+    fun testZipSlipValidation() {
+        val zipArchive = tmpDir.child("sneaky.klib")
+
+        createMaliciousArchive(zipArchive)
+
+        assertFailsWith<ZipException> {
+            zipArchive.unzipTo(tmpDir.child("unpacked"))
+        }
+    }
+}
+
+private fun createMaliciousArchive(file: File) {
+    ZipOutputStream(FileOutputStream(file.javaFile())).use {
+        it.putNextEntry(ZipEntry("../definitelySafe.txt"))
+        it.closeEntry()
+    }
+}