mirror of
https://github.com/TeamAmaze/AmazeFileManager.git
synced 2026-10-17 05:08:00 +08:00
Merge pull request #4591 from TeamAmaze/bugfix/extract-entry
Fixes for path traversal vulnerabilities
This commit is contained in:
+2
-2
@@ -58,7 +58,7 @@ class SevenZipHelperCallable(
|
||||
entries.addAll(
|
||||
consolidate(
|
||||
entriesMap.keys.filter {
|
||||
it.startsWith(relativePath)
|
||||
CompressedHelper.isEntryPathValid(it) && it.startsWith(relativePath)
|
||||
},
|
||||
if (relativePath == "") {
|
||||
0
|
||||
@@ -101,7 +101,7 @@ class SevenZipHelperCallable(
|
||||
} catch (e: PasswordRequiredException) {
|
||||
// this is so that the caller can use onError to ask the user for the password
|
||||
throw e
|
||||
} catch (e: IOException) {
|
||||
} catch (_: IOException) {
|
||||
throw ArchiveException(String.format("7zip archive %s is corrupt", filePath))
|
||||
}
|
||||
}
|
||||
|
||||
@@ -373,8 +373,7 @@ public class ExtractService extends AbstractProgressiveService {
|
||||
} else {
|
||||
LOG.error("Error while extracting file " + compressedPath, e);
|
||||
AppConfig.toast(getApplicationContext(), extractService.getString(R.string.error));
|
||||
paused = true;
|
||||
publishProgress(e);
|
||||
return false;
|
||||
}
|
||||
} catch (Throwable unhandledException) {
|
||||
LOG.error("Unhandled exception thrown", unhandledException);
|
||||
|
||||
+21
-6
@@ -164,10 +164,6 @@ public abstract class CompressedHelper {
|
||||
// without the compression extension
|
||||
decompressor = new UnknownCompressedFileDecompressor(context);
|
||||
} else {
|
||||
if (BuildConfig.DEBUG) {
|
||||
throw new IllegalArgumentException("The compressed file has no way of opening it: " + file);
|
||||
}
|
||||
|
||||
LOG.error("The compressed file has no way of opening it: " + file);
|
||||
decompressor = null;
|
||||
}
|
||||
@@ -228,8 +224,27 @@ public abstract class CompressedHelper {
|
||||
}
|
||||
}
|
||||
|
||||
public static final boolean isEntryPathValid(String entryPath) {
|
||||
return !entryPath.startsWith("..\\") && !entryPath.startsWith("../") && !entryPath.equals("..");
|
||||
public static boolean isEntryPathValid(String entryPath) {
|
||||
if (entryPath == null || entryPath.isEmpty()) {
|
||||
return false;
|
||||
}
|
||||
// Normalize path separators to handle both Unix and Windows-style paths.
|
||||
String normalized = entryPath.replace('\\', '/');
|
||||
// Walk the path segments, tracking depth to detect escaping the archive root.
|
||||
// A path like "dir/sub/.." is valid (it resolves to "dir"), but "../../evil" is not.
|
||||
int depth = 0;
|
||||
for (String segment : normalized.split("/")) {
|
||||
if ("..".equals(segment)) {
|
||||
depth--;
|
||||
if (depth < 0) {
|
||||
// Path would escape the archive root.
|
||||
return false;
|
||||
}
|
||||
} else if (!segment.isEmpty() && !".".equals(segment)) {
|
||||
depth++;
|
||||
}
|
||||
}
|
||||
return true;
|
||||
}
|
||||
|
||||
private static boolean isZip(String type) {
|
||||
|
||||
+22
@@ -23,6 +23,7 @@ package com.amaze.filemanager.filesystem.compressed.extractcontents;
|
||||
import static com.amaze.filemanager.filesystem.compressed.CompressedHelper.SEPARATOR;
|
||||
import static com.amaze.filemanager.filesystem.compressed.CompressedHelper.SEPARATOR_CHAR;
|
||||
|
||||
import java.io.File;
|
||||
import java.io.IOException;
|
||||
import java.util.ArrayList;
|
||||
import java.util.Collections;
|
||||
@@ -113,6 +114,27 @@ public abstract class Extractor {
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Verifies that {@code outputFile} is contained within {@code outputDir}, guarding against
|
||||
* zip-slip / path-traversal attacks.
|
||||
*
|
||||
* @throws IOException if the resolved canonical path of {@code outputFile} would land outside
|
||||
* {@code outputDir}.
|
||||
*/
|
||||
protected static void checkEntryPath(File outputFile, String outputDir) throws IOException {
|
||||
String canonicalOutput = outputFile.getCanonicalPath();
|
||||
String canonicalDir = new File(outputDir).getCanonicalPath() + File.separator;
|
||||
if (!canonicalOutput.startsWith(canonicalDir)
|
||||
&& !canonicalOutput.equals(new File(outputDir).getCanonicalPath())) {
|
||||
throw new IOException(
|
||||
"Refusing to extract entry '"
|
||||
+ outputFile.getName()
|
||||
+ "' outside target directory '"
|
||||
+ canonicalDir
|
||||
+ "'");
|
||||
}
|
||||
}
|
||||
|
||||
public static class EmptyArchiveNotice extends IOException {}
|
||||
|
||||
public static class BadArchiveNotice extends IOException {
|
||||
|
||||
+3
-2
@@ -67,7 +67,7 @@ abstract class AbstractCommonsArchiveExtractor(
|
||||
}
|
||||
}
|
||||
}
|
||||
if (archiveEntries.size > 0) {
|
||||
if (archiveEntries.isNotEmpty()) {
|
||||
listener.onStart(totalBytes, archiveEntries[0].name)
|
||||
inputStream.close()
|
||||
inputStream = createFrom(FileInputStream(filePath))
|
||||
@@ -96,11 +96,12 @@ abstract class AbstractCommonsArchiveExtractor(
|
||||
entry: ArchiveEntry,
|
||||
outputDir: String,
|
||||
) {
|
||||
val outputFile = File(outputDir, entry.name)
|
||||
checkEntryPath(outputFile, outputDir)
|
||||
if (entry.isDirectory) {
|
||||
MakeDirectoryOperation.mkdir(File(outputDir, entry.name), context)
|
||||
return
|
||||
}
|
||||
val outputFile = File(outputDir, entry.name)
|
||||
if (false == outputFile.parentFile?.exists()) {
|
||||
MakeDirectoryOperation.mkdir(outputFile.parentFile, context)
|
||||
}
|
||||
|
||||
+3
-3
@@ -105,12 +105,12 @@ class SevenZipExtractor(
|
||||
entry: SevenZArchiveEntry,
|
||||
outputDir: String,
|
||||
) {
|
||||
val name = entry.name
|
||||
val outputFile = File(outputDir, entry.name)
|
||||
checkEntryPath(outputFile, outputDir)
|
||||
if (entry.isDirectory) {
|
||||
MakeDirectoryOperation.mkdir(File(outputDir, name), context)
|
||||
MakeDirectoryOperation.mkdir(File(outputDir, entry.name), context)
|
||||
return
|
||||
}
|
||||
val outputFile = File(outputDir, name)
|
||||
if (!outputFile.parentFile.exists()) {
|
||||
MakeDirectoryOperation.mkdir(outputFile.parentFile, context)
|
||||
}
|
||||
|
||||
+1
-8
@@ -21,7 +21,6 @@
|
||||
package com.amaze.filemanager.filesystem.compressed.extractcontents.helpers
|
||||
|
||||
import android.content.Context
|
||||
import android.os.Build
|
||||
import com.amaze.filemanager.R
|
||||
import com.amaze.filemanager.application.AppConfig
|
||||
import com.amaze.filemanager.fileoperations.filesystem.compressed.ArchivePasswordCache
|
||||
@@ -47,8 +46,6 @@ class ZipExtractor(
|
||||
listener: OnUpdate,
|
||||
updatePosition: UpdatePosition,
|
||||
) : Extractor(context, filePath, outputPath, listener, updatePosition) {
|
||||
private val isRobolectricTest = Build.HARDWARE == "robolectric"
|
||||
|
||||
@Throws(IOException::class)
|
||||
override fun extractWithFilter(filter: Filter) {
|
||||
var totalBytes: Long = 0
|
||||
@@ -110,11 +107,7 @@ class ZipExtractor(
|
||||
outputDir: String,
|
||||
) {
|
||||
val outputFile = File(outputDir, fixEntryName(entry.fileName))
|
||||
if (!outputFile.canonicalPath.startsWith(outputDir) &&
|
||||
(isRobolectricTest && !outputFile.canonicalPath.startsWith("/private$outputDir"))
|
||||
) {
|
||||
throw IOException("Incorrect ZipEntry path!")
|
||||
}
|
||||
checkEntryPath(outputFile, outputDir)
|
||||
if (entry.isDirectory) {
|
||||
// zip entry is a directory, return after creating new directory
|
||||
MakeDirectoryOperation.mkdir(outputFile, context)
|
||||
|
||||
+9
-6
@@ -77,9 +77,11 @@ class RarExtractor(
|
||||
MainHeaderNullException::class.java.isAssignableFrom(it::class.java) -> {
|
||||
throw BadArchiveNotice(it)
|
||||
}
|
||||
|
||||
UnsupportedRarV5Exception::class.java.isAssignableFrom(it::class.java) -> {
|
||||
throw it
|
||||
}
|
||||
|
||||
else -> {
|
||||
throw PasswordRequiredException(filePath)
|
||||
}
|
||||
@@ -144,11 +146,7 @@ class RarExtractor(
|
||||
CompressedHelper.SEPARATOR,
|
||||
)
|
||||
val outputFile = File(outputDir, name)
|
||||
if (!outputFile.canonicalPath.startsWith(outputDir) &&
|
||||
(isRobolectricTest && !outputFile.canonicalPath.startsWith("/private$outputDir"))
|
||||
) {
|
||||
throw IOException("Incorrect RAR FileHeader path!")
|
||||
}
|
||||
checkEntryPath(outputFile, outputDir)
|
||||
if (entry.isDirectory) {
|
||||
MakeDirectoryOperation.mkdir(outputFile, context)
|
||||
outputFile.setLastModified(entry.mTime.time)
|
||||
@@ -232,7 +230,12 @@ class RarExtractor(
|
||||
"\\\\".toRegex(),
|
||||
CompressedHelper.SEPARATOR,
|
||||
)
|
||||
extractEntry(context, archive, header, context.externalCacheDir!!.absolutePath)
|
||||
extractEntry(
|
||||
context,
|
||||
archive,
|
||||
header,
|
||||
context.externalCacheDir!!.absolutePath,
|
||||
)
|
||||
return "${context.externalCacheDir!!.absolutePath}/$filename"
|
||||
}
|
||||
}
|
||||
|
||||
+43
@@ -0,0 +1,43 @@
|
||||
/*
|
||||
* Copyright (C) 2014-2021 Arpit Khurana <arpitkh96@gmail.com>, Vishal Nehra <vishalmeham2@gmail.com>,
|
||||
* Emmanuel Messulam<emmanuelbendavid@gmail.com>, Raymond Lai <airwave209gt at gmail.com> and Contributors.
|
||||
*
|
||||
* This file is part of Amaze File Manager.
|
||||
*
|
||||
* Amaze File Manager is free software: you can redistribute it and/or modify
|
||||
* it under the terms of the GNU General Public License as published by
|
||||
* the Free Software Foundation, either version 3 of the License, or
|
||||
* (at your option) any later version.
|
||||
*
|
||||
* This program is distributed in the hope that it will be useful,
|
||||
* but WITHOUT ANY WARRANTY; without even the implied warranty of
|
||||
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
|
||||
* GNU General Public License for more details.
|
||||
*
|
||||
* You should have received a copy of the GNU General Public License
|
||||
* along with this program. If not, see <http://www.gnu.org/licenses/>.
|
||||
*/
|
||||
|
||||
package com.amaze.filemanager.asynchronous.asynctasks.compress
|
||||
|
||||
import android.os.Environment
|
||||
import org.junit.Assert.assertFalse
|
||||
import org.junit.Test
|
||||
import java.io.File
|
||||
|
||||
/**
|
||||
* Tests for [SevenZipHelperCallable].
|
||||
*/
|
||||
class SevenZipHelperCallableMaliciousTest : AbstractCompressedHelperCallableTest() {
|
||||
/**
|
||||
* Test to ensure that the root of the archive does not expose parent folder entries
|
||||
* (e.g., ".." or "../").
|
||||
*/
|
||||
@Test
|
||||
fun testRootDoesNotExposeParentFolderEntries() {
|
||||
val archive = File(Environment.getExternalStorageDirectory(), "malicious.7z")
|
||||
val result = SevenZipHelperCallable(archive.absolutePath, "", false).call()
|
||||
|
||||
assertFalse(result.any { it.name == ".." || it.name.startsWith("../") })
|
||||
}
|
||||
}
|
||||
@@ -49,6 +49,7 @@ import org.awaitility.Awaitility.await
|
||||
import org.junit.After
|
||||
import org.junit.Assert.assertEquals
|
||||
import org.junit.Assert.assertNull
|
||||
import org.junit.Assert.assertTrue
|
||||
import org.junit.Assert.fail
|
||||
import org.junit.Before
|
||||
import org.junit.Ignore
|
||||
@@ -95,6 +96,11 @@ class ExtractServiceTest {
|
||||
private val tarXzfile: File
|
||||
private val tarBz2file: File
|
||||
private val sevenZipfile: File
|
||||
private val maliciousTarGzFile: File
|
||||
private val maliciousTarBz2File: File
|
||||
private val maliciousTarXzFile: File
|
||||
private val maliciousTarLzmaFile: File
|
||||
private val maliciousSevenZipFile: File
|
||||
private val passwordProtectedZipfile: File
|
||||
private val passwordProtected7Zipfile: File
|
||||
private val listPasswordProtected7Zipfile: File
|
||||
@@ -126,6 +132,11 @@ class ExtractServiceTest {
|
||||
tarXzfile = File(this, "test-archive.tar.xz")
|
||||
tarBz2file = File(this, "test-archive.tar.bz2")
|
||||
sevenZipfile = File(this, "test-archive.7z")
|
||||
maliciousTarGzFile = File(this, "malicious.tar.gz")
|
||||
maliciousTarBz2File = File(this, "malicious.tar.bz2")
|
||||
maliciousTarXzFile = File(this, "malicious.tar.xz")
|
||||
maliciousTarLzmaFile = File(this, "malicious.tar.lzma")
|
||||
maliciousSevenZipFile = File(this, "malicious.7z")
|
||||
passwordProtectedZipfile = File(this, "test-archive-encrypted.zip")
|
||||
passwordProtected7Zipfile = File(this, "test-archive-encrypted.7z")
|
||||
listPasswordProtected7Zipfile = File(this, "test-archive-encrypted-list.7z")
|
||||
@@ -163,6 +174,11 @@ class ExtractServiceTest {
|
||||
getResourceAsStream("test-archive.tar.xz").copyTo(FileOutputStream(tarXzfile))
|
||||
getResourceAsStream("test-archive.tar.bz2").copyTo(FileOutputStream(tarBz2file))
|
||||
getResourceAsStream("test-archive.7z").copyTo(FileOutputStream(sevenZipfile))
|
||||
getResourceAsStream("malicious.tar.gz").copyTo(FileOutputStream(maliciousTarGzFile))
|
||||
getResourceAsStream("malicious.tar.bz2").copyTo(FileOutputStream(maliciousTarBz2File))
|
||||
getResourceAsStream("malicious.tar.xz").copyTo(FileOutputStream(maliciousTarXzFile))
|
||||
getResourceAsStream("malicious.tar.lzma").copyTo(FileOutputStream(maliciousTarLzmaFile))
|
||||
getResourceAsStream("malicious.7z").copyTo(FileOutputStream(maliciousSevenZipFile))
|
||||
getResourceAsStream("test-archive-encrypted.zip")
|
||||
.copyTo(FileOutputStream(passwordProtectedZipfile))
|
||||
getResourceAsStream("test-archive-encrypted.7z")
|
||||
@@ -326,6 +342,72 @@ class ExtractServiceTest {
|
||||
assertNull(ShadowToast.getTextOfLatestToast())
|
||||
}
|
||||
|
||||
/**
|
||||
* Test malicious 7z extraction exits with a regular error toast.
|
||||
*/
|
||||
@Test
|
||||
fun testExtractMalicious7z() {
|
||||
performTest(maliciousSevenZipFile)
|
||||
ShadowLooper.idleMainLooper()
|
||||
await().atMost(10, TimeUnit.SECONDS).until { ShadowToast.getLatestToast() != null }
|
||||
assertErrorOrInvalidEntriesToast()
|
||||
}
|
||||
|
||||
/**
|
||||
* Test malicious tar.gz extraction exits with a regular error toast.
|
||||
*/
|
||||
@Test
|
||||
fun testExtractMaliciousTarGz() {
|
||||
performTest(maliciousTarGzFile)
|
||||
ShadowLooper.idleMainLooper()
|
||||
await().atMost(10, TimeUnit.SECONDS).until { ShadowToast.getLatestToast() != null }
|
||||
assertErrorOrInvalidEntriesToast()
|
||||
}
|
||||
|
||||
/**
|
||||
* Test malicious tar.bz2 extraction exits with a regular error toast.
|
||||
*/
|
||||
@Test
|
||||
fun testExtractMaliciousTarBz2() {
|
||||
performTest(maliciousTarBz2File)
|
||||
ShadowLooper.idleMainLooper()
|
||||
await().atMost(10, TimeUnit.SECONDS).until { ShadowToast.getLatestToast() != null }
|
||||
assertErrorOrInvalidEntriesToast()
|
||||
}
|
||||
|
||||
/**
|
||||
* Test malicious tar.xz extraction exits with a regular error toast.
|
||||
*/
|
||||
@Test
|
||||
fun testExtractMaliciousTarXz() {
|
||||
performTest(maliciousTarXzFile)
|
||||
ShadowLooper.idleMainLooper()
|
||||
await().atMost(10, TimeUnit.SECONDS).until { ShadowToast.getLatestToast() != null }
|
||||
assertErrorOrInvalidEntriesToast()
|
||||
}
|
||||
|
||||
/**
|
||||
* Test malicious tar.lzma extraction exits with a regular error toast.
|
||||
*/
|
||||
@Test
|
||||
fun testExtractMaliciousTarLzma() {
|
||||
performTest(maliciousTarLzmaFile)
|
||||
ShadowLooper.idleMainLooper()
|
||||
await().atMost(10, TimeUnit.SECONDS).until { ShadowToast.getLatestToast() != null }
|
||||
assertErrorOrInvalidEntriesToast()
|
||||
}
|
||||
|
||||
private fun assertErrorOrInvalidEntriesToast() {
|
||||
val context = ApplicationProvider.getApplicationContext<Context>()
|
||||
val latestToastText = ShadowToast.getTextOfLatestToast()
|
||||
assertTrue(
|
||||
setOf(
|
||||
context.getString(R.string.error),
|
||||
context.getString(R.string.multiple_invalid_archive_entries),
|
||||
).contains(latestToastText),
|
||||
)
|
||||
}
|
||||
|
||||
/**
|
||||
* Test password-protected zip.
|
||||
*/
|
||||
|
||||
+42
@@ -279,4 +279,46 @@ public class CompressedHelperTest {
|
||||
// no path
|
||||
assertEquals("", CompressedHelper.getFileName(""));
|
||||
}
|
||||
|
||||
/**
|
||||
* isEntryPathValid() tests.
|
||||
*
|
||||
* <p>Validates that paths which resolve within the archive root are accepted, and that paths
|
||||
* which would escape the archive root are rejected — including cases where {@code ..} components
|
||||
* appear in the middle of an otherwise valid path.
|
||||
*/
|
||||
@Test
|
||||
public void isEntryPathValidTest() {
|
||||
// null / empty are always invalid
|
||||
assertFalse(CompressedHelper.isEntryPathValid(null));
|
||||
assertFalse(CompressedHelper.isEntryPathValid(""));
|
||||
|
||||
// simple valid paths
|
||||
assertTrue(CompressedHelper.isEntryPathValid("test.txt"));
|
||||
assertTrue(CompressedHelper.isEntryPathValid("dir/file.txt"));
|
||||
assertTrue(CompressedHelper.isEntryPathValid("dir/sub/file.txt"));
|
||||
assertTrue(CompressedHelper.isEntryPathValid("dir/"));
|
||||
|
||||
// ".." that resolves back inside the root is VALID
|
||||
// e.g. "dir/sub/.." resolves to "dir" — still inside the archive
|
||||
assertTrue(CompressedHelper.isEntryPathValid("dir/sub/.."));
|
||||
assertTrue(CompressedHelper.isEntryPathValid("dir/sub/../file.txt"));
|
||||
assertTrue(CompressedHelper.isEntryPathValid("a/b/c/../../file.txt"));
|
||||
|
||||
// paths that escape the archive root are INVALID
|
||||
assertFalse(CompressedHelper.isEntryPathValid("../evil.txt"));
|
||||
assertFalse(CompressedHelper.isEntryPathValid("../../evil.txt"));
|
||||
assertFalse(CompressedHelper.isEntryPathValid("foo/../../evil.txt"));
|
||||
assertFalse(CompressedHelper.isEntryPathValid("foo/../../../evil.txt"));
|
||||
|
||||
// Windows-style separators are normalised first
|
||||
assertFalse(CompressedHelper.isEntryPathValid("..\\evil.txt"));
|
||||
assertFalse(CompressedHelper.isEntryPathValid("foo\\..\\..\\evil.txt"));
|
||||
assertTrue(CompressedHelper.isEntryPathValid("dir\\sub\\file.txt"));
|
||||
assertTrue(CompressedHelper.isEntryPathValid("dir\\sub\\..\\file.txt"));
|
||||
|
||||
// "." segments are ignored (current directory)
|
||||
assertTrue(CompressedHelper.isEntryPathValid("./test.txt"));
|
||||
assertTrue(CompressedHelper.isEntryPathValid("dir/./file.txt"));
|
||||
}
|
||||
}
|
||||
|
||||
+56
@@ -20,10 +20,66 @@
|
||||
|
||||
package com.amaze.filemanager.filesystem.compressed.extractcontents
|
||||
|
||||
import android.os.Environment
|
||||
import androidx.test.core.app.ApplicationProvider
|
||||
import com.amaze.filemanager.asynchronous.management.ServiceWatcherUtil
|
||||
import com.amaze.filemanager.filesystem.compressed.extractcontents.helpers.SevenZipExtractor
|
||||
import org.junit.Assert.assertFalse
|
||||
import org.junit.Assert.fail
|
||||
import org.junit.Test
|
||||
import java.io.File
|
||||
import java.io.IOException
|
||||
|
||||
open class SevenZipExtractorTest : AbstractArchiveExtractorTest() {
|
||||
override val archiveType: String = "7z"
|
||||
|
||||
override fun extractorClass(): Class<out Extractor?> = SevenZipExtractor::class.java
|
||||
|
||||
/**
|
||||
* Verify that a 7-Zip archive carrying a path-traversal entry
|
||||
* (../POC_7Z_PROOF.txt) is blocked by the canonical-path guard:
|
||||
* - extractEverything() must throw IOException
|
||||
* - no file is written outside the designated output directory
|
||||
*/
|
||||
@Test
|
||||
fun testExtractMalicious7z() {
|
||||
val maliciousArchive = File(Environment.getExternalStorageDirectory(), "malicious.7z")
|
||||
val outputDir = Environment.getExternalStorageDirectory()
|
||||
val extractor =
|
||||
SevenZipExtractor(
|
||||
ApplicationProvider.getApplicationContext(),
|
||||
maliciousArchive.absolutePath,
|
||||
outputDir.absolutePath,
|
||||
object : Extractor.OnUpdate {
|
||||
override fun onStart(
|
||||
totalBytes: Long,
|
||||
firstEntryName: String,
|
||||
) = Unit
|
||||
|
||||
override fun onUpdate(entryPath: String) = Unit
|
||||
|
||||
override fun isCancelled(): Boolean = false
|
||||
|
||||
override fun onFinish() = Unit
|
||||
},
|
||||
ServiceWatcherUtil.UPDATE_POSITION,
|
||||
)
|
||||
|
||||
try {
|
||||
extractor.extractEverything()
|
||||
fail("Expected IOException: canonical-path guard must reject the traversal entry")
|
||||
} catch (e: IOException) {
|
||||
// Confirm the guard fired (not a generic bad-archive error)
|
||||
assertFalse(
|
||||
"Exception must not be a BadArchiveNotice",
|
||||
e is Extractor.BadArchiveNotice,
|
||||
)
|
||||
}
|
||||
|
||||
// The malicious file must NOT have been written outside the output directory
|
||||
assertFalse(
|
||||
"Malicious file must not escape the output directory",
|
||||
File(outputDir.parentFile, "POC_7Z_PROOF.txt").exists(),
|
||||
)
|
||||
}
|
||||
}
|
||||
|
||||
+54
@@ -20,10 +20,64 @@
|
||||
|
||||
package com.amaze.filemanager.filesystem.compressed.extractcontents
|
||||
|
||||
import android.os.Environment
|
||||
import androidx.test.core.app.ApplicationProvider
|
||||
import com.amaze.filemanager.asynchronous.management.ServiceWatcherUtil
|
||||
import com.amaze.filemanager.filesystem.compressed.extractcontents.helpers.TarBzip2Extractor
|
||||
import org.junit.Assert.assertFalse
|
||||
import org.junit.Assert.fail
|
||||
import org.junit.Test
|
||||
import java.io.File
|
||||
import java.io.IOException
|
||||
|
||||
/**
|
||||
* Tests for [TarBzip2Extractor].
|
||||
*/
|
||||
open class TarBzip2ExtractorTest : AbstractArchiveExtractorTest() {
|
||||
override val archiveType: String = "tar.bz2"
|
||||
|
||||
override fun extractorClass(): Class<out Extractor?> = TarBzip2Extractor::class.java
|
||||
|
||||
/**
|
||||
* Test extracting a malicious tar.bz2 archive does not allow path traversal.
|
||||
*/
|
||||
@Test
|
||||
fun testExtractMaliciousTarBzip2() {
|
||||
val maliciousArchive = File(Environment.getExternalStorageDirectory(), "malicious.tar.bz2")
|
||||
val outputDir = Environment.getExternalStorageDirectory()
|
||||
val extractor =
|
||||
TarBzip2Extractor(
|
||||
ApplicationProvider.getApplicationContext(),
|
||||
maliciousArchive.absolutePath,
|
||||
outputDir.absolutePath,
|
||||
object : Extractor.OnUpdate {
|
||||
override fun onStart(
|
||||
totalBytes: Long,
|
||||
firstEntryName: String,
|
||||
) = Unit
|
||||
|
||||
override fun onUpdate(entryPath: String) = Unit
|
||||
|
||||
override fun isCancelled(): Boolean = false
|
||||
|
||||
override fun onFinish() = Unit
|
||||
},
|
||||
ServiceWatcherUtil.UPDATE_POSITION,
|
||||
)
|
||||
|
||||
try {
|
||||
extractor.extractEverything()
|
||||
fail("Expected IOException: canonical-path guard must reject the traversal entry")
|
||||
} catch (e: IOException) {
|
||||
assertFalse(
|
||||
"Exception must not be a BadArchiveNotice",
|
||||
e is Extractor.BadArchiveNotice,
|
||||
)
|
||||
}
|
||||
|
||||
assertFalse(
|
||||
"Malicious file must not escape the output directory",
|
||||
File(outputDir.parentFile, "POC_ZIPSLIP_PROOF.txt").exists(),
|
||||
)
|
||||
}
|
||||
}
|
||||
|
||||
+3
@@ -20,6 +20,9 @@
|
||||
|
||||
package com.amaze.filemanager.filesystem.compressed.extractcontents
|
||||
|
||||
/**
|
||||
* Tests for [TarBzip2Extractor], but with .tbz extension instead of .tar.bz2.
|
||||
*/
|
||||
class TarBzip2ExtractorTest2 : TarBzip2ExtractorTest() {
|
||||
override val archiveType: String = "tbz"
|
||||
}
|
||||
|
||||
+57
@@ -20,10 +20,67 @@
|
||||
|
||||
package com.amaze.filemanager.filesystem.compressed.extractcontents
|
||||
|
||||
import android.os.Environment
|
||||
import androidx.test.core.app.ApplicationProvider
|
||||
import com.amaze.filemanager.asynchronous.management.ServiceWatcherUtil
|
||||
import com.amaze.filemanager.filesystem.compressed.extractcontents.helpers.TarGzExtractor
|
||||
import org.junit.Assert.assertFalse
|
||||
import org.junit.Assert.fail
|
||||
import org.junit.Test
|
||||
import java.io.File
|
||||
import java.io.IOException
|
||||
|
||||
open class TarGzExtractorTest : AbstractArchiveExtractorTest() {
|
||||
override val archiveType: String = "tar.gz"
|
||||
|
||||
override fun extractorClass(): Class<out Extractor?> = TarGzExtractor::class.java
|
||||
|
||||
/**
|
||||
* Verify that a tar.gz archive carrying a path-traversal entry
|
||||
* (../POC_ZIPSLIP_PROOF.txt) is blocked by the canonical-path guard
|
||||
* in AbstractCommonsArchiveExtractor:
|
||||
* - extractEverything() must throw IOException
|
||||
* - no file is written outside the designated output directory
|
||||
*/
|
||||
@Test
|
||||
fun testExtractMaliciousTarGz() {
|
||||
val maliciousArchive = File(Environment.getExternalStorageDirectory(), "malicious.tar.gz")
|
||||
val outputDir = Environment.getExternalStorageDirectory()
|
||||
val extractor =
|
||||
TarGzExtractor(
|
||||
ApplicationProvider.getApplicationContext(),
|
||||
maliciousArchive.absolutePath,
|
||||
outputDir.absolutePath,
|
||||
object : Extractor.OnUpdate {
|
||||
override fun onStart(
|
||||
totalBytes: Long,
|
||||
firstEntryName: String,
|
||||
) = Unit
|
||||
|
||||
override fun onUpdate(entryPath: String) = Unit
|
||||
|
||||
override fun isCancelled(): Boolean = false
|
||||
|
||||
override fun onFinish() = Unit
|
||||
},
|
||||
ServiceWatcherUtil.UPDATE_POSITION,
|
||||
)
|
||||
|
||||
try {
|
||||
extractor.extractEverything()
|
||||
fail("Expected IOException: canonical-path guard must reject the traversal entry")
|
||||
} catch (e: IOException) {
|
||||
// Confirm the guard fired (not a generic bad-archive error)
|
||||
assertFalse(
|
||||
"Exception must not be a BadArchiveNotice",
|
||||
e is Extractor.BadArchiveNotice,
|
||||
)
|
||||
}
|
||||
|
||||
// The malicious file must NOT have been written outside the output directory
|
||||
assertFalse(
|
||||
"Malicious file must not escape the output directory",
|
||||
File(outputDir.parentFile, "POC_ZIPSLIP_PROOF.txt").exists(),
|
||||
)
|
||||
}
|
||||
}
|
||||
|
||||
+54
@@ -20,10 +20,64 @@
|
||||
|
||||
package com.amaze.filemanager.filesystem.compressed.extractcontents
|
||||
|
||||
import android.os.Environment
|
||||
import androidx.test.core.app.ApplicationProvider
|
||||
import com.amaze.filemanager.asynchronous.management.ServiceWatcherUtil
|
||||
import com.amaze.filemanager.filesystem.compressed.extractcontents.helpers.TarLzmaExtractor
|
||||
import org.junit.Assert.assertFalse
|
||||
import org.junit.Assert.fail
|
||||
import org.junit.Test
|
||||
import java.io.File
|
||||
import java.io.IOException
|
||||
|
||||
/**
|
||||
* Tests for [TarLzmaExtractor].
|
||||
*/
|
||||
class TarLzmaExtractorTest : AbstractArchiveExtractorTest() {
|
||||
override val archiveType: String = "tar.lzma"
|
||||
|
||||
override fun extractorClass(): Class<out Extractor?> = TarLzmaExtractor::class.java
|
||||
|
||||
/**
|
||||
* Test extracting a malicious tar.lzma archive does not allow path traversal.
|
||||
*/
|
||||
@Test
|
||||
fun testExtractMaliciousTarLzma() {
|
||||
val maliciousArchive = File(Environment.getExternalStorageDirectory(), "malicious.tar.lzma")
|
||||
val outputDir = Environment.getExternalStorageDirectory()
|
||||
val extractor =
|
||||
TarLzmaExtractor(
|
||||
ApplicationProvider.getApplicationContext(),
|
||||
maliciousArchive.absolutePath,
|
||||
outputDir.absolutePath,
|
||||
object : Extractor.OnUpdate {
|
||||
override fun onStart(
|
||||
totalBytes: Long,
|
||||
firstEntryName: String,
|
||||
) = Unit
|
||||
|
||||
override fun onUpdate(entryPath: String) = Unit
|
||||
|
||||
override fun isCancelled(): Boolean = false
|
||||
|
||||
override fun onFinish() = Unit
|
||||
},
|
||||
ServiceWatcherUtil.UPDATE_POSITION,
|
||||
)
|
||||
|
||||
try {
|
||||
extractor.extractEverything()
|
||||
fail("Expected IOException: canonical-path guard must reject the traversal entry")
|
||||
} catch (e: IOException) {
|
||||
assertFalse(
|
||||
"Exception must not be a BadArchiveNotice",
|
||||
e is Extractor.BadArchiveNotice,
|
||||
)
|
||||
}
|
||||
|
||||
assertFalse(
|
||||
"Malicious file must not escape the output directory",
|
||||
File(outputDir.parentFile, "POC_ZIPSLIP_PROOF.txt").exists(),
|
||||
)
|
||||
}
|
||||
}
|
||||
|
||||
+54
@@ -20,10 +20,64 @@
|
||||
|
||||
package com.amaze.filemanager.filesystem.compressed.extractcontents
|
||||
|
||||
import android.os.Environment
|
||||
import androidx.test.core.app.ApplicationProvider
|
||||
import com.amaze.filemanager.asynchronous.management.ServiceWatcherUtil
|
||||
import com.amaze.filemanager.filesystem.compressed.extractcontents.helpers.TarXzExtractor
|
||||
import org.junit.Assert.assertFalse
|
||||
import org.junit.Assert.fail
|
||||
import org.junit.Test
|
||||
import java.io.File
|
||||
import java.io.IOException
|
||||
|
||||
/**
|
||||
* Tests for [TarXzExtractor].
|
||||
*/
|
||||
class TarXzExtractorTest : AbstractArchiveExtractorTest() {
|
||||
override val archiveType: String = "tar.xz"
|
||||
|
||||
override fun extractorClass(): Class<out Extractor?> = TarXzExtractor::class.java
|
||||
|
||||
/**
|
||||
* Test extracting a malicious tar.xz archive does not allow path traversal.
|
||||
*/
|
||||
@Test
|
||||
fun testExtractMaliciousTarXz() {
|
||||
val maliciousArchive = File(Environment.getExternalStorageDirectory(), "malicious.tar.xz")
|
||||
val outputDir = Environment.getExternalStorageDirectory()
|
||||
val extractor =
|
||||
TarXzExtractor(
|
||||
ApplicationProvider.getApplicationContext(),
|
||||
maliciousArchive.absolutePath,
|
||||
outputDir.absolutePath,
|
||||
object : Extractor.OnUpdate {
|
||||
override fun onStart(
|
||||
totalBytes: Long,
|
||||
firstEntryName: String,
|
||||
) = Unit
|
||||
|
||||
override fun onUpdate(entryPath: String) = Unit
|
||||
|
||||
override fun isCancelled(): Boolean = false
|
||||
|
||||
override fun onFinish() = Unit
|
||||
},
|
||||
ServiceWatcherUtil.UPDATE_POSITION,
|
||||
)
|
||||
|
||||
try {
|
||||
extractor.extractEverything()
|
||||
fail("Expected IOException: canonical-path guard must reject the traversal entry")
|
||||
} catch (e: IOException) {
|
||||
assertFalse(
|
||||
"Exception must not be a BadArchiveNotice",
|
||||
e is Extractor.BadArchiveNotice,
|
||||
)
|
||||
}
|
||||
|
||||
assertFalse(
|
||||
"Malicious file must not escape the output directory",
|
||||
File(outputDir.parentFile, "POC_ZIPSLIP_PROOF.txt").exists(),
|
||||
)
|
||||
}
|
||||
}
|
||||
|
||||
+62
@@ -20,10 +20,72 @@
|
||||
|
||||
package com.amaze.filemanager.filesystem.compressed.extractcontents
|
||||
|
||||
import android.os.Environment
|
||||
import androidx.test.core.app.ApplicationProvider
|
||||
import com.amaze.filemanager.asynchronous.management.ServiceWatcherUtil
|
||||
import com.amaze.filemanager.filesystem.compressed.extractcontents.helpers.ZipExtractor
|
||||
import org.junit.Assert.assertFalse
|
||||
import org.junit.Assert.assertTrue
|
||||
import org.junit.Test
|
||||
import java.io.File
|
||||
|
||||
class ZipExtractorTest : AbstractArchiveExtractorTest() {
|
||||
override val archiveType: String = "zip"
|
||||
|
||||
override fun extractorClass(): Class<out Extractor?> = ZipExtractor::class.java
|
||||
|
||||
/**
|
||||
* Verify that a ZIP archive carrying a path-traversal entry
|
||||
* (foo/../../POC_ZIP_PROOF.txt) is handled safely:
|
||||
* - extraction completes without an exception
|
||||
* - the offending entry is recorded in invalidArchiveEntries
|
||||
* - no file is written outside the designated output directory
|
||||
*/
|
||||
@Test
|
||||
fun testExtractMaliciousZip() {
|
||||
val maliciousArchive = File(Environment.getExternalStorageDirectory(), "malicious.zip")
|
||||
val outputDir = Environment.getExternalStorageDirectory()
|
||||
val extractor =
|
||||
ZipExtractor(
|
||||
ApplicationProvider.getApplicationContext(),
|
||||
maliciousArchive.absolutePath,
|
||||
outputDir.absolutePath,
|
||||
object : Extractor.OnUpdate {
|
||||
override fun onStart(
|
||||
totalBytes: Long,
|
||||
firstEntryName: String,
|
||||
) = Unit
|
||||
|
||||
override fun onUpdate(entryPath: String) = Unit
|
||||
|
||||
override fun isCancelled(): Boolean = false
|
||||
|
||||
override fun onFinish() = Unit
|
||||
},
|
||||
ServiceWatcherUtil.UPDATE_POSITION,
|
||||
)
|
||||
|
||||
// Extraction must succeed — path-traversal entries are quarantined, not thrown
|
||||
extractor.extractEverything()
|
||||
|
||||
// The traversal entry must be recorded as invalid …
|
||||
assertTrue(
|
||||
"Malicious path-traversal entry must be captured in invalidArchiveEntries",
|
||||
extractor.invalidArchiveEntries.isNotEmpty(),
|
||||
)
|
||||
assertTrue(
|
||||
"invalidArchiveEntries must contain the POC entry",
|
||||
extractor.invalidArchiveEntries.any { "POC_ZIP_PROOF" in it },
|
||||
)
|
||||
// … and must NOT have been written outside the output directory
|
||||
val escapedFile = File(outputDir, "foo/../../POC_ZIP_PROOF.txt").canonicalFile
|
||||
assertFalse(
|
||||
"Malicious file must not escape the output directory",
|
||||
escapedFile.exists(),
|
||||
)
|
||||
assertFalse(
|
||||
"Escaped file canonical path must not reside under output directory",
|
||||
escapedFile.canonicalPath.startsWith(outputDir.canonicalPath),
|
||||
)
|
||||
}
|
||||
}
|
||||
|
||||
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
+52
@@ -27,6 +27,8 @@ import com.amaze.filemanager.asynchronous.management.ServiceWatcherUtil
|
||||
import com.amaze.filemanager.filesystem.compressed.extractcontents.helpers.RarExtractor
|
||||
import com.github.junrar.Archive
|
||||
import org.junit.Assert.assertEquals
|
||||
import org.junit.Assert.assertFalse
|
||||
import org.junit.Assert.assertTrue
|
||||
import org.junit.Ignore
|
||||
import org.junit.Test
|
||||
import java.io.File
|
||||
@@ -89,6 +91,56 @@ class RarExtractorTest : AbstractArchiveExtractorTest() {
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Verify that a RAR4 archive carrying a path-traversal entry
|
||||
* (foo//../../POC_RAR_PROOF.txt/POC_RAR_PROOF.txt) is handled safely:
|
||||
* - extraction completes without an exception
|
||||
* - the offending entry is recorded in invalidArchiveEntries
|
||||
* - no file is written outside the designated output directory
|
||||
*/
|
||||
@Test
|
||||
fun testExtractMaliciousRar() {
|
||||
val maliciousArchive = File(Environment.getExternalStorageDirectory(), "malicious.rar")
|
||||
val outputDir = Environment.getExternalStorageDirectory()
|
||||
val extractor =
|
||||
RarExtractor(
|
||||
ApplicationProvider.getApplicationContext(),
|
||||
maliciousArchive.absolutePath,
|
||||
outputDir.absolutePath,
|
||||
object : Extractor.OnUpdate {
|
||||
override fun onStart(
|
||||
totalBytes: Long,
|
||||
firstEntryName: String,
|
||||
) = Unit
|
||||
|
||||
override fun onUpdate(entryPath: String) = Unit
|
||||
|
||||
override fun isCancelled(): Boolean = false
|
||||
|
||||
override fun onFinish() = Unit
|
||||
},
|
||||
ServiceWatcherUtil.UPDATE_POSITION,
|
||||
)
|
||||
|
||||
// Extraction must succeed — path-traversal entries are quarantined, not thrown
|
||||
extractor.extractEverything()
|
||||
|
||||
// The traversal entry must be recorded as invalid …
|
||||
assertTrue(
|
||||
"Malicious path-traversal entry must be captured in invalidArchiveEntries",
|
||||
extractor.invalidArchiveEntries.isNotEmpty(),
|
||||
)
|
||||
assertTrue(
|
||||
"invalidArchiveEntries must contain the POC entry",
|
||||
extractor.invalidArchiveEntries.any { "POC_RAR_PROOF" in it },
|
||||
)
|
||||
// … and must NOT have been written outside the output directory
|
||||
assertFalse(
|
||||
"Malicious file must not escape the output directory",
|
||||
File(outputDir.parentFile, "POC_RAR_PROOF.txt").exists(),
|
||||
)
|
||||
}
|
||||
|
||||
@Test @Ignore
|
||||
override fun testExtractBadArchive() = Unit
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user