diff options
3 files changed, 223 insertions, 0 deletions
diff --git a/tools/lint/framework/checks/src/main/java/com/google/android/lint/AndroidFrameworkIssueRegistry.kt b/tools/lint/framework/checks/src/main/java/com/google/android/lint/AndroidFrameworkIssueRegistry.kt index 935badecf8d5..624a1987638e 100644 --- a/tools/lint/framework/checks/src/main/java/com/google/android/lint/AndroidFrameworkIssueRegistry.kt +++ b/tools/lint/framework/checks/src/main/java/com/google/android/lint/AndroidFrameworkIssueRegistry.kt @@ -20,6 +20,7 @@ import com.android.tools.lint.client.api.IssueRegistry import com.android.tools.lint.client.api.Vendor import com.android.tools.lint.detector.api.CURRENT_API import com.google.android.lint.parcel.SaferParcelChecker +import com.google.android.lint.aidl.PermissionAnnotationDetector import com.google.auto.service.AutoService @AutoService(IssueRegistry::class) @@ -37,6 +38,7 @@ class AndroidFrameworkIssueRegistry : IssueRegistry() { SaferParcelChecker.ISSUE_UNSAFE_API_USAGE, // TODO: Currently crashes due to OOM issue // PackageVisibilityDetector.ISSUE_PACKAGE_NAME_NO_PACKAGE_VISIBILITY_FILTERS, + PermissionAnnotationDetector.ISSUE_MISSING_PERMISSION_ANNOTATION, PermissionMethodDetector.ISSUE_PERMISSION_METHOD_USAGE, PermissionMethodDetector.ISSUE_CAN_BE_PERMISSION_METHOD, ) diff --git a/tools/lint/framework/checks/src/main/java/com/google/android/lint/PermissionAnnotationDetector.kt b/tools/lint/framework/checks/src/main/java/com/google/android/lint/PermissionAnnotationDetector.kt new file mode 100644 index 000000000000..6b50cfd9e5ab --- /dev/null +++ b/tools/lint/framework/checks/src/main/java/com/google/android/lint/PermissionAnnotationDetector.kt @@ -0,0 +1,87 @@ +/* + * Copyright (C) 2023 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.google.android.lint.aidl + +import com.android.tools.lint.detector.api.Category +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 org.jetbrains.uast.UBlockExpression +import org.jetbrains.uast.UMethod + +/** + * Ensures all AIDL-generated methods are annotated. + * + * This detector is run on system_server to validate that any method that may + * be exposed via an AIDL interface is permission-annotated. That is, it must + * have one of the following annotation: + * - @EnforcePermission + * - @RequiresNoPermission + * - @PermissionManuallyEnforced + */ +class PermissionAnnotationDetector : AidlImplementationDetector() { + + override fun visitAidlMethod( + context: JavaContext, + node: UMethod, + interfaceName: String, + body: UBlockExpression + ) { + if (context.evaluator.isAbstract(node)) return + + if (AIDL_PERMISSION_ANNOTATIONS.any { node.hasAnnotation(it) }) return + + context.report( + ISSUE_MISSING_PERMISSION_ANNOTATION, + node, + context.getLocation(node), + "The method ${node.name} is not permission-annotated." + ) + } + + companion object { + + private val EXPLANATION_MISSING_PERMISSION_ANNOTATION = """ + Interfaces that are exposed by system_server are required to have an annotation which + denotes the type of permission enforced. There are 3 possible options: + - @EnforcePermission + - @RequiresNoPermission + - @PermissionManuallyEnforced + See the documentation of each annotation for further details. + + The annotation on the Java implementation must be the same that the AIDL interface + definition. This is verified by a lint in the build system. + """.trimIndent() + + @JvmField + val ISSUE_MISSING_PERMISSION_ANNOTATION = Issue.create( + id = "MissingPermissionAnnotation", + briefDescription = "No permission annotation on exposed AIDL interface.", + explanation = EXPLANATION_MISSING_PERMISSION_ANNOTATION, + category = Category.CORRECTNESS, + priority = 5, + severity = Severity.ERROR, + implementation = Implementation( + PermissionAnnotationDetector::class.java, + Scope.JAVA_FILE_SCOPE + ), + enabledByDefault = false + ) + } +} diff --git a/tools/lint/framework/checks/src/test/java/com/google/android/lint/PermissionAnnotationDetectorTest.kt b/tools/lint/framework/checks/src/test/java/com/google/android/lint/PermissionAnnotationDetectorTest.kt new file mode 100644 index 000000000000..bce848a2e3a7 --- /dev/null +++ b/tools/lint/framework/checks/src/test/java/com/google/android/lint/PermissionAnnotationDetectorTest.kt @@ -0,0 +1,134 @@ +/* + * Copyright (C) 2023 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.google.android.lint.aidl + +import com.android.tools.lint.checks.infrastructure.LintDetectorTest +import com.android.tools.lint.checks.infrastructure.TestFile +import com.android.tools.lint.checks.infrastructure.TestLintTask +import com.android.tools.lint.detector.api.Detector +import com.android.tools.lint.detector.api.Issue + +@Suppress("UnstableApiUsage") +class PermissionAnnotationDetectorTest : LintDetectorTest() { + override fun getDetector(): Detector = PermissionAnnotationDetector() + + override fun getIssues(): List<Issue> = listOf( + PermissionAnnotationDetector.ISSUE_MISSING_PERMISSION_ANNOTATION, + ) + + override fun lint(): TestLintTask = super.lint().allowMissingSdk(true) + + /** No issue scenario */ + + fun testDoesNotDetectIssuesInCorrectScenario() { + lint().files( + java( + """ + public class Foo extends IFoo.Stub { + @Override + @android.annotation.EnforcePermission("android.Manifest.permission.READ_CONTACTS") + public void testMethod() { } + } + """ + ).indented(), + *stubs + ) + .run() + .expectClean() + } + + fun testMissingAnnotation() { + lint().files( + java( + """ + public class Bar extends IBar.Stub { + public void testMethod() { } + } + """ + ).indented(), + *stubs + ) + .run() + .expect( + """ + src/Bar.java:2: Error: The method testMethod is not permission-annotated. [MissingPermissionAnnotation] + public void testMethod() { } + ~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + 1 errors, 0 warnings + """ + ) + } + + fun testNoIssueWhenExtendingWithAnotherSubclass() { + lint().files( + java( + """ + public class Foo extends IFoo.Stub { + @Override + @android.annotation.EnforcePermission(android.Manifest.permission.READ_PHONE_STATE) + public void testMethod() { } + // not an AIDL method, just another method + public void someRandomMethod() { } + } + """).indented(), + java( + """ + public class Baz extends Bar { + @Override + public void someRandomMethod() { } + } + """).indented(), + *stubs + ) + .run() + .expectClean() + } + + /* Stubs */ + + // A service with permission annotation on the method. + private val interfaceIFoo: TestFile = java( + """ + public interface IFoo extends android.os.IInterface { + public static abstract class Stub extends android.os.Binder implements IFoo { + } + @Override + @android.annotation.EnforcePermission(android.Manifest.permission.READ_PHONE_STATE) + public void testMethod(); + @Override + @android.annotation.RequiresNoPermission + public void testMethodNoPermission(); + @Override + @android.annotation.PermissionManuallyEnforced + public void testMethodManual(); + } + """ + ).indented() + + // A service with no permission annotation. + private val interfaceIBar: TestFile = java( + """ + public interface IBar extends android.os.IInterface { + public static abstract class Stub extends android.os.Binder implements IBar { + } + public void testMethod(); + } + """ + ).indented() + + private val stubs = arrayOf(interfaceIFoo, interfaceIBar) +} |