fix: prevent ZIP path traversal during backup restore (#6134)

* fix: harden ZIP backup extraction

* fix: tune backup ZIP limits

* refactor: simplify ZIP extraction validation

V2rayNG/app/src/main/java/com/v2ray/ang/ui/backup/BackupViewModel.kt

@@ -185,17 +185,20 @@ class BackupViewModel(application: Application) : BaseViewModel(application) {
         }
     }
 
-    private fun performRestore(cacheDir: File, zipFile: File): Boolean {
+    private suspend fun performRestore(cacheDir: File, zipFile: File): Boolean =
-        val backupDir = cacheDir.absolutePath + "/${System.currentTimeMillis()}"
+        withContext(Dispatchers.IO) {
+            val backupDir = File(cacheDir, "restore_${System.nanoTime()}")
+            try {
+                if (!ZipUtil.unzipToFolder(zipFile, backupDir.absolutePath)) {
+                    return@withContext false
+                }
 
-        if (!ZipUtil.unzipToFolder(zipFile, backupDir)) {
+                val count = MMKV.restoreAllFromDirectory(backupDir.absolutePath)
-            return false
+                SettingsChangeManager.makeSetupGroupTab()
+                SettingsChangeManager.makeRestartService()
+                count > 0
+            } finally {
+                backupDir.deleteRecursively()
+            }
         }
-
-        val count = MMKV.restoreAllFromDirectory(backupDir)
-        SettingsChangeManager.makeSetupGroupTab()
-        SettingsChangeManager.makeRestartService()
-
-        return count > 0
-    }
 }

V2rayNG/app/src/main/java/com/v2ray/ang/util/ZipUtil.kt

@@ -8,11 +8,37 @@ import java.io.FileOutputStream
 import java.io.IOException
 import java.io.InputStream
 import java.util.zip.ZipEntry
+import java.util.zip.ZipException
 import java.util.zip.ZipFile
 import java.util.zip.ZipOutputStream
 
 object ZipUtil {
     private const val BUFFER_SIZE = 4096
+    private const val MEBIBYTE = 1024L * 1024L
+
+    private val DEFAULT_EXTRACTION_LIMITS = ExtractionLimits(
+        maxArchiveBytes = 128L * MEBIBYTE,
+        maxEntries = 32,
+        maxEntryBytes = 128L * MEBIBYTE,
+        maxTotalBytes = 256L * MEBIBYTE,
+        maxCompressionRatio = 1000.0,
+    )
+
+    internal class ExtractionLimits(
+        val maxArchiveBytes: Long,
+        val maxEntries: Int,
+        val maxEntryBytes: Long,
+        val maxTotalBytes: Long,
+        val maxCompressionRatio: Double,
+    ) {
+        init {
+            require(maxArchiveBytes > 0)
+            require(maxEntries > 0)
+            require(maxEntryBytes > 0)
+            require(maxTotalBytes > 0)
+            require(maxCompressionRatio >= 1.0)
+        }
+    }
 
     /**
      * Zip the contents of a folder.
@@ -78,47 +104,181 @@ object ZipUtil {
      */
     @Throws(IOException::class)
     fun unzipToFolder(zipFile: File, destDirectory: String): Boolean {
-        File(destDirectory).run {
+        return try {
-            if (!exists()) {
+            extractArchive(zipFile, File(destDirectory), DEFAULT_EXTRACTION_LIMITS)
-                mkdirs()
+            true
-            }
-        }
-        try {
-            ZipFile(zipFile).use { zip ->
-                zip.entries().asSequence().forEach { entry ->
-                    zip.getInputStream(entry).use { input ->
-                        val filePath = destDirectory + File.separator + entry.name
-                        if (!entry.isDirectory) {
-                            extractFile(input, filePath)
-                        } else {
-                            val dir = File(filePath)
-                            dir.mkdir()
-                        }
-                    }
-                }
-            }
         } catch (e: Exception) {
             LogUtil.e(AppConfig.TAG, "Failed to unzip file", e)
+            false
+        }
+    }
+
+    internal fun extractArchive(
+        zipFile: File,
+        destination: File,
+        limits: ExtractionLimits,
+    ) {
+        if (!zipFile.isFile || zipFile.length() > limits.maxArchiveBytes) {
+            throw ZipException("ZIP archive is missing or exceeds the compressed-size limit")
+        }
+
+        val root = destination.canonicalFile
+        ZipFile(zipFile).use { zip ->
+            val entries = validateEntries(zip, root, limits)
+            val rootCreated = prepareDestination(root)
+            try {
+                extractEntries(zip, entries, limits)
+            } catch (error: Throwable) {
+                root.listFiles()?.forEach { it.deleteRecursively() }
+                if (rootCreated) root.delete()
+                throw error
+            }
+        }
+    }
+
+    private fun validateEntries(
+        zip: ZipFile,
+        root: File,
+        limits: ExtractionLimits,
+    ): Map<File, ZipEntry> {
+        val entriesByTarget = LinkedHashMap<File, ZipEntry>()
+        var totalBytes = 0L
+        val entries = zip.entries()
+
+        while (entries.hasMoreElements()) {
+            if (entriesByTarget.size >= limits.maxEntries) {
+                throw ZipException("ZIP archive exceeds the entry-count limit")
+            }
+
+            val entry = entries.nextElement()
+            if (entry.name.isEmpty()) {
+                throw ZipException("ZIP archive contains an empty entry name")
+            }
+            if (entry.method != ZipEntry.STORED && entry.method != ZipEntry.DEFLATED) {
+                throw ZipException("ZIP archive contains an unsupported compression method")
+            }
+
+            val target = resolveEntryTarget(root, entry)
+            if (entriesByTarget.put(target, entry) != null) {
+                throw ZipException("ZIP archive contains duplicate entry destinations")
+            }
+
+            val entryBytes = entry.size
+            val compressedBytes = entry.compressedSize
+            if (entryBytes < 0L || compressedBytes < 0L) {
+                throw ZipException("ZIP archive contains an entry with an unknown size")
+            }
+            if (entry.isDirectory && entryBytes != 0L) {
+                throw ZipException("ZIP archive contains a non-empty directory entry")
+            }
+            if (entryBytes > limits.maxEntryBytes) {
+                throw ZipException("ZIP archive entry exceeds the expanded-size limit")
+            }
+            if (totalBytes > limits.maxTotalBytes - entryBytes) {
+                throw ZipException("ZIP archive exceeds the total expanded-size limit")
+            }
+            if (
+                entryBytes > 0L && (
+                    compressedBytes == 0L ||
+                        entryBytes.toDouble() / compressedBytes > limits.maxCompressionRatio
+                )
+            ) {
+                throw ZipException("ZIP archive entry exceeds the compression-ratio limit")
+            }
+
+            totalBytes += entryBytes
+        }
+
+        entriesByTarget.forEach { (target, _) ->
+            var parent = target.parentFile
+            while (parent != null && parent != root) {
+                if (entriesByTarget[parent]?.isDirectory == false) {
+                    throw ZipException("ZIP archive places an entry below a file")
+                }
+                parent = parent.parentFile
+            }
+        }
+        return entriesByTarget
+    }
+
+    private fun resolveEntryTarget(root: File, entry: ZipEntry): File {
+        val target = File(root, entry.name).canonicalFile
+        val rootPrefix = root.path + File.separator
+        if (target == root || !target.path.startsWith(rootPrefix)) {
+            throw ZipException("ZIP entry resolves outside the destination directory")
+        }
+        return target
+    }
+
+    private fun prepareDestination(root: File): Boolean {
+        if (root.exists()) {
+            if (!root.isDirectory || root.listFiles()?.isNotEmpty() != false) {
+                throw IOException("ZIP destination must be an empty directory")
+            }
             return false
         }
+        if (!root.mkdirs()) {
+            throw IOException("Unable to create ZIP destination directory")
+        }
         return true
     }
 
-    /**
+    private fun extractEntries(
-     * Extract a file from an input stream.
+        zip: ZipFile,
-     *
+        entries: Map<File, ZipEntry>,
-     * @param inputStream The input stream to read from.
+        limits: ExtractionLimits,
-     * @param destFilePath The destination file path.
+    ) {
-     * @throws IOException If an I/O error occurs.
+        var totalBytes = 0L
-     */
+        entries.forEach { (target, entry) ->
-    @Throws(IOException::class)
+            if (entry.isDirectory) {
-    private fun extractFile(inputStream: InputStream, destFilePath: String) {
+                if (!target.isDirectory && !target.mkdirs()) {
-        val bos = BufferedOutputStream(FileOutputStream(destFilePath))
+                    throw IOException("Unable to create ZIP entry directory")
-        val bytesIn = ByteArray(BUFFER_SIZE)
+                }
-        var read: Int
+                return@forEach
-        while (inputStream.read(bytesIn).also { read = it } != -1) {
+            }
-            bos.write(bytesIn, 0, read)
+
+            val parent = target.parentFile
+                ?: throw ZipException("ZIP entry has no destination parent")
+            if (!parent.isDirectory && !parent.mkdirs()) {
+                throw IOException("Unable to create ZIP entry parent directory")
+            }
+            if (target.exists()) {
+                throw ZipException("ZIP entry would overwrite an existing file")
+            }
+
+            zip.getInputStream(entry).use { input ->
+                BufferedOutputStream(FileOutputStream(target)).use { output ->
+                    totalBytes += copyEntry(input, output, entry.size, totalBytes, limits)
+                }
+            }
         }
-        bos.close()
     }
-}
+
+    private fun copyEntry(
+        input: InputStream,
+        output: BufferedOutputStream,
+        expectedBytes: Long,
+        currentTotalBytes: Long,
+        limits: ExtractionLimits,
+    ): Long {
+        val buffer = ByteArray(BUFFER_SIZE)
+        var entryBytes = 0L
+        while (true) {
+            val read = input.read(buffer)
+            if (read < 0) break
+            val readBytes = read.toLong()
+            val remainingEntryBytes = limits.maxEntryBytes - entryBytes
+            val remainingTotalBytes = limits.maxTotalBytes - currentTotalBytes - entryBytes
+            if (readBytes > remainingEntryBytes || readBytes > remainingTotalBytes
+            ) {
+                throw ZipException("ZIP entry exceeds the expanded-size limit while extracting")
+            }
+            output.write(buffer, 0, read)
+            entryBytes += readBytes
+        }
+        if (entryBytes != expectedBytes) {
+            throw ZipException("ZIP entry expanded size differs from its declared size")
+        }
+        return entryBytes
+    }
+}

V2rayNG/app/src/test/java/com/v2ray/ang/util/ZipUtilTest.kt

@@ -0,0 +1,135 @@
+package com.v2ray.ang.util
+
+import org.junit.Assert.assertEquals
+import org.junit.Assert.assertFalse
+import org.junit.Assert.assertThrows
+import org.junit.Assert.assertTrue
+import org.junit.Rule
+import org.junit.Test
+import org.junit.rules.TemporaryFolder
+import java.io.File
+import java.util.zip.ZipEntry
+import java.util.zip.ZipException
+import java.util.zip.ZipOutputStream
+
+class ZipUtilTest {
+
+    @get:Rule
+    val temporaryFolder = TemporaryFolder()
+
+    @Test
+    fun unzipToFolderExtractsNestedFile() {
+        val archive = createArchive("nested/config" to "value".toByteArray())
+        val destination = File(temporaryFolder.root, "destination")
+
+        assertTrue(ZipUtil.unzipToFolder(archive, destination.absolutePath))
+        assertEquals("value", File(destination, "nested/config").readText())
+    }
+
+    @Test
+    fun extractArchiveRejectsPathTraversal() {
+        val archive = createArchive("../outside" to "overwrite".toByteArray())
+        val outside = File(temporaryFolder.root, "outside")
+
+        assertArchiveRejected(archive)
+
+        assertFalse(outside.exists())
+    }
+
+    @Test
+    fun extractArchiveRejectsOversizedArchive() {
+        val archive = createArchive("config" to byteArrayOf(1))
+
+        assertArchiveRejected(
+            archive,
+            extractionLimits(maxArchiveBytes = archive.length() - 1),
+        )
+    }
+
+    @Test
+    fun extractArchiveRejectsTooManyEntries() {
+        val archive = createArchive(
+            "first" to byteArrayOf(1),
+            "second" to byteArrayOf(2),
+        )
+        assertArchiveRejected(archive, extractionLimits(maxEntries = 1))
+    }
+
+    @Test
+    fun extractArchiveRejectsOversizedEntry() {
+        val archive = createArchive("config" to ByteArray(5))
+
+        assertArchiveRejected(archive, extractionLimits(maxEntryBytes = 4))
+    }
+
+    @Test
+    fun extractArchiveRejectsExcessiveTotalSize() {
+        val archive = createArchive(
+            "first" to ByteArray(3),
+            "second" to ByteArray(3),
+        )
+        assertArchiveRejected(archive, extractionLimits(maxTotalBytes = 5))
+    }
+
+    @Test
+    fun extractArchiveRejectsExcessiveCompressionRatio() {
+        val archive = createArchive("config" to ByteArray(4096))
+
+        assertArchiveRejected(archive, extractionLimits(maxCompressionRatio = 2.0))
+    }
+
+    @Test
+    fun extractArchiveRejectsDuplicateCanonicalDestination() {
+        val archive = createArchive(
+            "config" to byteArrayOf(1),
+            "nested/../config" to byteArrayOf(2),
+        )
+        assertArchiveRejected(archive)
+    }
+
+    @Test
+    fun extractArchiveRejectsEntryBelowFile() {
+        val archive = createArchive(
+            "parent" to byteArrayOf(1),
+            "parent/child" to byteArrayOf(2),
+        )
+        assertArchiveRejected(archive)
+    }
+
+    private fun assertArchiveRejected(
+        archive: File,
+        limits: ZipUtil.ExtractionLimits = extractionLimits(),
+    ) {
+        val destination = File(temporaryFolder.root, "destination")
+        assertThrows(ZipException::class.java) {
+            ZipUtil.extractArchive(archive, destination, limits)
+        }
+        assertFalse(destination.exists())
+    }
+
+    private fun createArchive(vararg entries: Pair<String, ByteArray>): File {
+        val archive = temporaryFolder.newFile()
+        ZipOutputStream(archive.outputStream()).use { output ->
+            entries.forEach { (name, contents) ->
+                output.putNextEntry(ZipEntry(name))
+                output.write(contents)
+                output.closeEntry()
+            }
+        }
+        return archive
+    }
+
+    private fun extractionLimits(
+        maxArchiveBytes: Long = 1024L * 1024L,
+        maxEntries: Int = 10,
+        maxEntryBytes: Long = 16L * 1024L,
+        maxTotalBytes: Long = 32L * 1024L,
+        maxCompressionRatio: Double = 1000.0,
+    ) = ZipUtil.ExtractionLimits(
+        maxArchiveBytes = maxArchiveBytes,
+        maxEntries = maxEntries,
+        maxEntryBytes = maxEntryBytes,
+        maxTotalBytes = maxTotalBytes,
+        maxCompressionRatio = maxCompressionRatio,
+    )
+}