From e27941558eb8b06046a196be641febb3bab46964 Mon Sep 17 00:00:00 2001 From: eliotcougar Date: Sat, 29 Aug 2026 11:03:57 +0300 Subject: [PATCH] fix: prevent ZIP path traversal during backup restore (#6134) * fix: harden ZIP backup extraction * fix: tune backup ZIP limits * refactor: simplify ZIP extraction validation --- .../v2ray/ang/ui/backup/BackupViewModel.kt | 25 +- .../main/java/com/v2ray/ang/util/ZipUtil.kt | 230 +++++++++++++++--- .../java/com/v2ray/ang/util/ZipUtilTest.kt | 135 ++++++++++ 3 files changed, 344 insertions(+), 46 deletions(-) create mode 100644 V2rayNG/app/src/test/java/com/v2ray/ang/util/ZipUtilTest.kt diff --git a/V2rayNG/app/src/main/java/com/v2ray/ang/ui/backup/BackupViewModel.kt b/V2rayNG/app/src/main/java/com/v2ray/ang/ui/backup/BackupViewModel.kt index 2db5ca87..2891637f 100644 --- a/V2rayNG/app/src/main/java/com/v2ray/ang/ui/backup/BackupViewModel.kt +++ b/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 { - val backupDir = cacheDir.absolutePath + "/${System.currentTimeMillis()}" + private suspend fun performRestore(cacheDir: File, zipFile: File): Boolean = + 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)) { - return false + val count = MMKV.restoreAllFromDirectory(backupDir.absolutePath) + SettingsChangeManager.makeSetupGroupTab() + SettingsChangeManager.makeRestartService() + count > 0 + } finally { + backupDir.deleteRecursively() + } } - - val count = MMKV.restoreAllFromDirectory(backupDir) - SettingsChangeManager.makeSetupGroupTab() - SettingsChangeManager.makeRestartService() - - return count > 0 - } } diff --git a/V2rayNG/app/src/main/java/com/v2ray/ang/util/ZipUtil.kt b/V2rayNG/app/src/main/java/com/v2ray/ang/util/ZipUtil.kt index ffeb5c93..273baab7 100644 --- a/V2rayNG/app/src/main/java/com/v2ray/ang/util/ZipUtil.kt +++ b/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 { - if (!exists()) { - mkdirs() - } - } - 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() - } - } - } - } + return try { + extractArchive(zipFile, File(destDirectory), DEFAULT_EXTRACTION_LIMITS) + true } 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 { + val entriesByTarget = LinkedHashMap() + 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 } - /** - * Extract a file from an input stream. - * - * @param inputStream The input stream to read from. - * @param destFilePath The destination file path. - * @throws IOException If an I/O error occurs. - */ - @Throws(IOException::class) - private fun extractFile(inputStream: InputStream, destFilePath: String) { - val bos = BufferedOutputStream(FileOutputStream(destFilePath)) - val bytesIn = ByteArray(BUFFER_SIZE) - var read: Int - while (inputStream.read(bytesIn).also { read = it } != -1) { - bos.write(bytesIn, 0, read) + private fun extractEntries( + zip: ZipFile, + entries: Map, + limits: ExtractionLimits, + ) { + var totalBytes = 0L + entries.forEach { (target, entry) -> + if (entry.isDirectory) { + if (!target.isDirectory && !target.mkdirs()) { + throw IOException("Unable to create ZIP entry directory") + } + return@forEach + } + + 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() } -} \ No newline at end of file + + 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 + } +} diff --git a/V2rayNG/app/src/test/java/com/v2ray/ang/util/ZipUtilTest.kt b/V2rayNG/app/src/test/java/com/v2ray/ang/util/ZipUtilTest.kt new file mode 100644 index 00000000..126d3443 --- /dev/null +++ b/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): 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, + ) +}