Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -85,11 +85,23 @@ class YuhaiinDocumentProvider : DocumentsProvider() {
}

override fun isChildDocument(parentDocumentId: String?, documentId: String?): Boolean {
if (documentId != null) {
return parentDocumentId?.let { documentId.startsWith(it) } ?: false
if (parentDocumentId == null || documentId == null) {
return false
}
return isChild(File(parentDocumentId), File(documentId))
}

return false
internal fun isChild(parent: File, child: File): Boolean {
return try {
val canonicalParent = parent.canonicalPath
val canonicalChild = child.canonicalPath
if (canonicalChild == canonicalParent) return true
val parentPathWithSeparator =
if (canonicalParent.endsWith(File.separator)) canonicalParent else canonicalParent + File.separator
canonicalChild.startsWith(parentPathWithSeparator)
} catch (e: IOException) {
false
}
}

override fun querySearchDocuments(
Expand Down Expand Up @@ -119,12 +131,7 @@ class YuhaiinDocumentProvider : DocumentsProvider() {
val file = pending.removeAt(0)
// Avoid directories outside the $HOME directory linked with symlinks (to avoid e.g. search
// through the whole SD card).
val isInsideHome: Boolean = try {
file.canonicalPath.startsWith(baseDir.toString())
} catch (_: IOException) {
true
}
if (isInsideHome) {
if (isChild(baseDir, file)) {
if (file.isDirectory) {
file.listFiles()?.let { pending.addAll(it) }
} else {
Expand All @@ -144,6 +151,7 @@ class YuhaiinDocumentProvider : DocumentsProvider() {
private fun getFileForDocId(docId: String): File {
val f = File(docId)
if (!f.exists()) throw FileNotFoundException(f.absolutePath + " not found")
if (!isChild(baseDir, f)) throw FileNotFoundException("Invalid document ID: $docId")
return f
}

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
package io.github.asutorufa.yuhaiin.docuemntprovider

import org.junit.Assert.assertFalse
import org.junit.Assert.assertTrue
import org.junit.Test
import java.io.File
import java.nio.file.Files

class PathTraversalTest {

@Test
fun testIsChild() {
val provider = YuhaiinDocumentProvider()
val tempDir = Files.createTempDirectory("yuhaiin_test").toFile().canonicalFile
val subDir = File(tempDir, "subdir")
subDir.mkdir()
val fileInSubDir = File(subDir, "file.txt")
fileInSubDir.createNewFile()

val otherDir = Files.createTempDirectory("yuhaiin_other").toFile().canonicalFile
val otherFile = File(otherDir, "other.txt")
otherFile.createNewFile()

// Construct a path that looks like it's inside tempDir but isn't after canonicalization
val traversalFile = File(tempDir, "../" + otherDir.name + "/other.txt")

assertTrue("Should be child of itself", provider.isChild(tempDir, tempDir))
assertTrue("Should be child of tempDir", provider.isChild(tempDir, subDir))
assertTrue("Should be child of tempDir", provider.isChild(tempDir, fileInSubDir))

assertFalse("Should not be child of otherDir", provider.isChild(tempDir, otherDir))
assertFalse("Should not be child of otherDir", provider.isChild(tempDir, otherFile))
assertFalse("Traversal should be blocked: ${traversalFile.path}", provider.isChild(tempDir, traversalFile))

val partialMatchDir = File(tempDir.parentFile, tempDir.name + "_extra")
partialMatchDir.mkdir()
assertFalse("Partial name match should be blocked: ${partialMatchDir.path}", provider.isChild(tempDir, partialMatchDir))

tempDir.deleteRecursively()
otherDir.deleteRecursively()
partialMatchDir.deleteRecursively()
}
Comment thread
Asutorufa marked this conversation as resolved.
}