From 83f64ba85cb132abe3e29fd540408be2df2847e3 Mon Sep 17 00:00:00 2001 From: "google-labs-jules[bot]" <161369871+google-labs-jules[bot]@users.noreply.github.com> Date: Sat, 14 Feb 2026 16:39:34 +0000 Subject: [PATCH] security: fix path traversal in YuhaiinDocumentProvider - Added `isChild` helper to validate file parent-child relationships using canonical paths. - Updated `getFileForDocId` to ensure requested documents are within the base directory. - Updated `isChildDocument` and `querySearchDocuments` for consistent security. - Added unit tests to verify protection against traversal and partial name matches. Co-authored-by: Asutorufa <16442314+Asutorufa@users.noreply.github.com> --- .../YuhaiinDocumentProvider.kt | 26 +++++++---- .../docuemntprovider/PathTraversalTest.kt | 43 +++++++++++++++++++ 2 files changed, 60 insertions(+), 9 deletions(-) create mode 100644 app/src/test/kotlin/io/github/asutorufa/yuhaiin/docuemntprovider/PathTraversalTest.kt diff --git a/app/src/main/kotlin/io/github/asutorufa/yuhaiin/docuemntprovider/YuhaiinDocumentProvider.kt b/app/src/main/kotlin/io/github/asutorufa/yuhaiin/docuemntprovider/YuhaiinDocumentProvider.kt index d49a35b6..12b5c72d 100644 --- a/app/src/main/kotlin/io/github/asutorufa/yuhaiin/docuemntprovider/YuhaiinDocumentProvider.kt +++ b/app/src/main/kotlin/io/github/asutorufa/yuhaiin/docuemntprovider/YuhaiinDocumentProvider.kt @@ -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( @@ -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 { @@ -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 } diff --git a/app/src/test/kotlin/io/github/asutorufa/yuhaiin/docuemntprovider/PathTraversalTest.kt b/app/src/test/kotlin/io/github/asutorufa/yuhaiin/docuemntprovider/PathTraversalTest.kt new file mode 100644 index 00000000..ee51b638 --- /dev/null +++ b/app/src/test/kotlin/io/github/asutorufa/yuhaiin/docuemntprovider/PathTraversalTest.kt @@ -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() + } +}