Skip to content

Commit 4fe93f1

Browse files
committed
lint: Rule for openInputStreamSafe
1 parent abcfce6 commit 4fe93f1

File tree

4 files changed

+172
-17
lines changed

4 files changed

+172
-17
lines changed

AnkiDroid/src/main/java/com/ichi2/utils/FileUtil.kt

Lines changed: 2 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -98,7 +98,7 @@ object FileUtil {
9898
// If we got a real file name, do a copy from it
9999
val inputStream: InputStream =
100100
try {
101-
contentResolver.openInputStream(uri)!!
101+
contentResolver.openInputStreamSafe(uri)!!
102102
} catch (e: Exception) {
103103
Timber.w(e, "internalizeUri() unable to open input stream from content resolver for Uri %s", uri)
104104
throw e
@@ -125,21 +125,6 @@ object FileUtil {
125125
fun listFiles(dir: File): Array<File> =
126126
dir.listFiles()
127127
?: throw IOException("Failed to list the contents of '$dir'")
128-
129-
/**
130-
* Returns a sequence containing the provided file, and its parents
131-
* up to the root of the filesystem.
132-
*/
133-
fun File.getParentsAndSelfRecursive() =
134-
sequence {
135-
var currentPath: File? = this@getParentsAndSelfRecursive.canonicalFile
136-
while (currentPath != null) {
137-
yield(currentPath)
138-
currentPath = currentPath.parentFile?.canonicalFile
139-
}
140-
}
141-
142-
fun File.isDescendantOf(ancestor: File) = this.getParentsAndSelfRecursive().drop(1).contains(ancestor)
143128
}
144129

145130
/**
@@ -219,5 +204,5 @@ fun ContentResolver.openInputStreamSafe(uri: Uri): InputStream? {
219204
if (normalized.startsWith("/data")) {
220205
throw SecurityException("java/android/unsafe-content-uri-resolution")
221206
}
222-
return openInputStream(uri)
207+
return openInputStreamSafe(uri)
223208
}

lint-rules/src/main/java/com/ichi2/anki/lint/IssueRegistry.kt

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,7 @@ import com.ichi2.anki.lint.rules.InvalidStringFormatDetector
3737
import com.ichi2.anki.lint.rules.JUnitNullAssertionDetector
3838
import com.ichi2.anki.lint.rules.LocaleRootDetector
3939
import com.ichi2.anki.lint.rules.NonPositionalFormatSubstitutions
40+
import com.ichi2.anki.lint.rules.OpenInputStreamSafeDetector
4041
import com.ichi2.anki.lint.rules.PrintStackTraceUsage
4142
import com.ichi2.anki.lint.rules.SentenceCaseConventions
4243
import com.ichi2.anki.lint.rules.TranslationTypo
@@ -62,6 +63,7 @@ class IssueRegistry : IssueRegistry() {
6263
LocaleRootDetector.ISSUE,
6364
PrintStackTraceUsage.ISSUE,
6465
NonPositionalFormatSubstitutions.ISSUE,
66+
OpenInputStreamSafeDetector.ISSUE,
6567
SentenceCaseConventions.ISSUE,
6668
TranslationTypo.ISSUE,
6769
FixedPreferencesTitleLength.PREFERENCES_ISSUE_MAX_LENGTH,
Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,67 @@
1+
package com.ichi2.anki.lint.rules
2+
3+
import com.android.tools.lint.detector.api.Category
4+
import com.android.tools.lint.detector.api.Detector
5+
import com.android.tools.lint.detector.api.Implementation
6+
import com.android.tools.lint.detector.api.Issue
7+
import com.android.tools.lint.detector.api.JavaContext
8+
import com.android.tools.lint.detector.api.Scope
9+
import com.android.tools.lint.detector.api.Severity
10+
import com.android.tools.lint.detector.api.SourceCodeScanner
11+
import com.intellij.psi.PsiMethod
12+
import org.jetbrains.uast.UCallExpression
13+
14+
/**
15+
* Detector that ensures ContentResolver.openInputStream() is not called directly.
16+
* Instead, developers should use the openInputStreamSafe() extension function
17+
* which includes path traversal protection.
18+
*/
19+
20+
class OpenInputStreamSafeDetector :
21+
Detector(),
22+
SourceCodeScanner {
23+
companion object {
24+
private const val EXPLANATION = """
25+
Use openInputStreamSafe() instead of openInputStream() to prevent \
26+
path traversal vulnerabilities. The safe version normalizes paths and blocks \
27+
access to sensitive directories like /data.
28+
"""
29+
30+
val ISSUE =
31+
Issue.create(
32+
id = "UnsafeOpenInputStream",
33+
briefDescription = "Use openInputStreamSafe() instead of openInputStream()",
34+
explanation = EXPLANATION,
35+
category = Category.SECURITY,
36+
priority = 9,
37+
severity = Severity.ERROR,
38+
implementation =
39+
Implementation(
40+
OpenInputStreamSafeDetector::class.java,
41+
Scope.JAVA_FILE_SCOPE,
42+
),
43+
)
44+
}
45+
46+
override fun getApplicableMethodNames(): List<String> = listOf("openInputStream")
47+
48+
override fun visitMethodCall(
49+
context: JavaContext,
50+
node: UCallExpression,
51+
method: PsiMethod,
52+
) {
53+
// Check if this is ContentResolver.openInputStream()
54+
// The evaluator checks if this method is a member of "android.content.ContentResolver".
55+
// If it's not (e.g., it's some custom class's openInputStream), we ignore it and return early.
56+
val evaluator = context.evaluator
57+
if (!evaluator.isMemberInClass(method, "android.content.ContentResolver")) {
58+
return
59+
}
60+
61+
context.report(
62+
issue = ISSUE,
63+
location = context.getNameLocation(node),
64+
message = "Use openInputStreamSafe() instead of openInputStream() for security",
65+
)
66+
}
67+
}
Lines changed: 101 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,101 @@
1+
package com.ichi2.anki.lint.rules
2+
3+
import com.android.tools.lint.checks.infrastructure.TestFiles.java
4+
import com.android.tools.lint.checks.infrastructure.TestFiles.kotlin
5+
import com.android.tools.lint.checks.infrastructure.TestLintTask.lint
6+
import org.junit.Test
7+
import org.junit.runner.RunWith
8+
import org.junit.runners.JUnit4
9+
10+
@RunWith(JUnit4::class)
11+
class OpenInputStreamSafeDetectorTest {
12+
// Stub files for Android classes needed by the tests
13+
private val contentResolverStub =
14+
java(
15+
"""
16+
package android.content;
17+
18+
import android.net.Uri;
19+
import java.io.InputStream;
20+
21+
public abstract class ContentResolver {
22+
public final InputStream openInputStream(Uri uri) {
23+
return null;
24+
}
25+
}
26+
""",
27+
).indented()
28+
29+
private val uriStub =
30+
java(
31+
"""
32+
package android.net;
33+
34+
public abstract class Uri {
35+
}
36+
""",
37+
).indented()
38+
39+
@Test
40+
fun testDirectOpenInputStreamCall() {
41+
lint()
42+
.allowMissingSdk()
43+
.files(
44+
contentResolverStub,
45+
uriStub,
46+
kotlin(
47+
"""
48+
class MyClass {
49+
fun loadData(resolver: android.content.ContentResolver, uri: android.net.Uri) {
50+
val stream = resolver.openInputStream(uri)
51+
}
52+
}
53+
""",
54+
).indented(),
55+
).issues(OpenInputStreamSafeDetector.ISSUE)
56+
.run()
57+
.expectContains("Use openInputStreamSafe() instead of openInputStream() for security")
58+
}
59+
60+
@Test
61+
fun testOpenInputStreamSafeCall() {
62+
lint()
63+
.allowMissingSdk()
64+
.files(
65+
contentResolverStub,
66+
uriStub,
67+
kotlin(
68+
"""
69+
class MyClass {
70+
fun loadData(resolver: android.content.ContentResolver, uri: android.net.Uri) {
71+
val stream = resolver.openInputStreamSafe(uri)
72+
}
73+
}
74+
""",
75+
).indented(),
76+
).issues(OpenInputStreamSafeDetector.ISSUE)
77+
.run()
78+
.expectClean()
79+
}
80+
81+
@Test
82+
fun testJavaDirectOpenInputStreamCall() {
83+
lint()
84+
.allowMissingSdk()
85+
.files(
86+
contentResolverStub,
87+
uriStub,
88+
java(
89+
"""
90+
public class MyClass {
91+
public void loadData(android.content.ContentResolver resolver, android.net.Uri uri) {
92+
java.io.InputStream stream = resolver.openInputStream(uri);
93+
}
94+
}
95+
""",
96+
).indented(),
97+
).issues(OpenInputStreamSafeDetector.ISSUE)
98+
.run()
99+
.expectContains("Use openInputStreamSafe() instead of openInputStream() for security")
100+
}
101+
}

0 commit comments

Comments
 (0)