diff --git a/AnkiDroid/src/main/java/com/ichi2/utils/FileUtil.kt b/AnkiDroid/src/main/java/com/ichi2/utils/FileUtil.kt index d053c2cb70c6..17488420c4a6 100644 --- a/AnkiDroid/src/main/java/com/ichi2/utils/FileUtil.kt +++ b/AnkiDroid/src/main/java/com/ichi2/utils/FileUtil.kt @@ -98,7 +98,7 @@ object FileUtil { // If we got a real file name, do a copy from it val inputStream: InputStream = try { - contentResolver.openInputStream(uri)!! + contentResolver.openInputStreamSafe(uri)!! } catch (e: Exception) { Timber.w(e, "internalizeUri() unable to open input stream from content resolver for Uri %s", uri) throw e @@ -125,21 +125,6 @@ object FileUtil { fun listFiles(dir: File): Array = dir.listFiles() ?: throw IOException("Failed to list the contents of '$dir'") - - /** - * Returns a sequence containing the provided file, and its parents - * up to the root of the filesystem. - */ - fun File.getParentsAndSelfRecursive() = - sequence { - var currentPath: File? = this@getParentsAndSelfRecursive.canonicalFile - while (currentPath != null) { - yield(currentPath) - currentPath = currentPath.parentFile?.canonicalFile - } - } - - fun File.isDescendantOf(ancestor: File) = this.getParentsAndSelfRecursive().drop(1).contains(ancestor) } /** diff --git a/lint-rules/src/main/java/com/ichi2/anki/lint/IssueRegistry.kt b/lint-rules/src/main/java/com/ichi2/anki/lint/IssueRegistry.kt index 9dd92052437d..27f0cda204a5 100644 --- a/lint-rules/src/main/java/com/ichi2/anki/lint/IssueRegistry.kt +++ b/lint-rules/src/main/java/com/ichi2/anki/lint/IssueRegistry.kt @@ -37,6 +37,7 @@ import com.ichi2.anki.lint.rules.InvalidStringFormatDetector import com.ichi2.anki.lint.rules.JUnitNullAssertionDetector import com.ichi2.anki.lint.rules.LocaleRootDetector import com.ichi2.anki.lint.rules.NonPositionalFormatSubstitutions +import com.ichi2.anki.lint.rules.OpenInputStreamSafeDetector import com.ichi2.anki.lint.rules.PrintStackTraceUsage import com.ichi2.anki.lint.rules.SentenceCaseConventions import com.ichi2.anki.lint.rules.TranslationTypo @@ -62,6 +63,7 @@ class IssueRegistry : IssueRegistry() { LocaleRootDetector.ISSUE, PrintStackTraceUsage.ISSUE, NonPositionalFormatSubstitutions.ISSUE, + OpenInputStreamSafeDetector.ISSUE, SentenceCaseConventions.ISSUE, TranslationTypo.ISSUE, FixedPreferencesTitleLength.PREFERENCES_ISSUE_MAX_LENGTH, diff --git a/lint-rules/src/main/java/com/ichi2/anki/lint/rules/OpenInputStreamSafeDetector.kt b/lint-rules/src/main/java/com/ichi2/anki/lint/rules/OpenInputStreamSafeDetector.kt new file mode 100644 index 000000000000..0948cdf37621 --- /dev/null +++ b/lint-rules/src/main/java/com/ichi2/anki/lint/rules/OpenInputStreamSafeDetector.kt @@ -0,0 +1,67 @@ +package com.ichi2.anki.lint.rules + +import com.android.tools.lint.detector.api.Category +import com.android.tools.lint.detector.api.Detector +import com.android.tools.lint.detector.api.Implementation +import com.android.tools.lint.detector.api.Issue +import com.android.tools.lint.detector.api.JavaContext +import com.android.tools.lint.detector.api.Scope +import com.android.tools.lint.detector.api.Severity +import com.android.tools.lint.detector.api.SourceCodeScanner +import com.intellij.psi.PsiMethod +import org.jetbrains.uast.UCallExpression +import org.jetbrains.uast.UMethod +import org.jetbrains.uast.getParentOfType + +/** + * Detector that ensures ContentResolver.openInputStream() is not called directly. + * Instead, developers should use the openInputStreamSafe() extension function + * which includes path traversal protection. + */ + +class OpenInputStreamSafeDetector : + Detector(), + SourceCodeScanner { + companion object { + private const val EXPLANATION = """ + Use openInputStreamSafe() instead of openInputStream() to prevent \ + path traversal vulnerabilities. The safe version normalizes paths and blocks \ + access to sensitive directories like /data. + """ + + val ISSUE = + Issue.create( + id = "UnsafeOpenInputStream", + briefDescription = "Use openInputStreamSafe() instead of openInputStream()", + explanation = EXPLANATION, + category = Category.SECURITY, + priority = 9, + severity = Severity.ERROR, + implementation = + Implementation( + OpenInputStreamSafeDetector::class.java, + Scope.JAVA_FILE_SCOPE, + ), + ) + } + + override fun getApplicableMethodNames(): List = listOf("openInputStream") + + override fun visitMethodCall( + context: JavaContext, + node: UCallExpression, + method: PsiMethod, + ) { + // Only warn on ContentResolver.openInputStream() + if (!context.evaluator.isMemberInClass(method, "android.content.ContentResolver")) return + if (node.enclosingMethodName == "openInputStreamSafe") return + context.report( + issue = ISSUE, + location = context.getNameLocation(node), + message = "Use openInputStreamSafe() instead of openInputStream()", + ) + } +} + +val UCallExpression.enclosingMethodName: String + get() = getParentOfType(UMethod::class.java)!!.name diff --git a/lint-rules/src/test/java/com/ichi2/anki/lint/rules/OpenInputStreamSafeDetectorTest.kt b/lint-rules/src/test/java/com/ichi2/anki/lint/rules/OpenInputStreamSafeDetectorTest.kt new file mode 100644 index 000000000000..7c429fef683c --- /dev/null +++ b/lint-rules/src/test/java/com/ichi2/anki/lint/rules/OpenInputStreamSafeDetectorTest.kt @@ -0,0 +1,86 @@ +package com.ichi2.anki.lint.rules + +import com.android.tools.lint.checks.infrastructure.TestFiles.java +import com.android.tools.lint.checks.infrastructure.TestFiles.kotlin +import com.android.tools.lint.checks.infrastructure.TestLintTask.lint +import org.junit.Test +import org.junit.runner.RunWith +import org.junit.runners.JUnit4 + +@RunWith(JUnit4::class) +class OpenInputStreamSafeDetectorTest { + @Test + fun testDirectOpenInputStreamCall() { + lint() + .allowMissingSdk() + .files( + kotlin( + """ + class MyClass { + fun loadData(resolver: android.content.ContentResolver, uri: android.net.Uri) { + val stream = resolver.openInputStream(uri) + } + } + """, + ).indented(), + ).issues(OpenInputStreamSafeDetector.ISSUE) + .run() + .expectContains("Use openInputStreamSafe() instead of openInputStream()") + } + + @Test + fun testOpenInputStreamSafeCall() { + lint() + .allowMissingSdk() + .files( + kotlin( + """ + class MyClass { + fun loadData(resolver: android.content.ContentResolver, uri: android.net.Uri) { + val stream = resolver.openInputStreamSafe(uri) + } + } + """, + ).indented(), + ).issues(OpenInputStreamSafeDetector.ISSUE) + .run() + .expectClean() + } + + @Test + fun testJavaDirectOpenInputStreamCall() { + lint() + .allowMissingSdk() + .files( + java( + """ + public class MyClass { + public void loadData(android.content.ContentResolver resolver, android.net.Uri uri) { + java.io.InputStream stream = resolver.openInputStream(uri); + } + } + """, + ).indented(), + ).issues(OpenInputStreamSafeDetector.ISSUE) + .run() + .expectContains("Use openInputStreamSafe() instead of openInputStream()") + } + + @Test + fun `openInputStreamSafe is not flagged`() { + lint() + .allowMissingSdk() + .files( + kotlin( + """ +@Suppress("UnusedReceiverParameter") +fun android.content.ContentResolver.openInputStreamSafe(uri: Uri): InputStream? { + return openInputStream(uri) +} + """, + ), + ).issues(OpenInputStreamSafeDetector.ISSUE) + .run() + .expectClean() + } +}