Update lint checks to their state on internal master
GlobalLintChecker was added as a prebuilt already on aosp, but we
could make it a source build if we had the source on aosp.
Bug: 264451752
Test: Presubmits
Change-Id: Iea5e5d2adef8c261490e14269cf6737c2a369e6d
diff --git a/tools/lint/README.md b/tools/lint/README.md
index b534b62..b235ad6 100644
--- a/tools/lint/README.md
+++ b/tools/lint/README.md
@@ -1,15 +1,44 @@
-# Android Framework Lint Checker
+# Android Lint Checks for AOSP
-Custom lint checks written here are going to be executed for modules that opt in to those (e.g. any
+Custom Android Lint checks are written here to be executed against java modules
+in AOSP. These checks are broken down into two subdirectories:
+
+1. [Global Checks](#android-global-lint-checker)
+2. [Framework Checks](#android-framework-lint-checker)
+
+# [Android Global Lint Checker](/global)
+Checks written here are executed for the entire tree. The `AndroidGlobalLintChecker`
+build target produces a jar file that is included in the overall build output
+(`AndroidGlobalLintChecker.jar`). This file is then downloaded as a prebuilt under the
+`prebuilts/cmdline-tools` subproject, and included by soong with all invocations of lint.
+
+## How to add new global lint checks
+1. Write your detector with its issues and put it into
+ `global/checks/src/main/java/com/google/android/lint`.
+2. Add your detector's issues into `AndroidGlobalIssueRegistry`'s `issues`
+ field.
+3. Write unit tests for your detector in one file and put it into
+ `global/checks/test/java/com/google/android/lint`.
+4. Have your change reviewed and merged. Once your change is merged,
+ obtain a build number from a successful build that includes your change.
+5. Run `prebuilts/cmdline-tools/update-android-global-lint-checker.sh
+ <build_number>`. The script will create a commit that you can upload for
+ approval to the `prebuilts/cmdline-tools` subproject.
+6. Done! Your lint check should be applied in lint report builds across the
+ entire tree!
+
+# [Android Framework Lint Checker](/framework)
+
+Checks written here are going to be executed for modules that opt in to those (e.g. any
`services.XXX` module) and results will be automatically reported on CLs on gerrit.
-## How to add new lint checks
+## How to add new framework lint checks
1. Write your detector with its issues and put it into
- `checks/src/main/java/com/google/android/lint`.
+ `framework/checks/src/main/java/com/google/android/lint`.
2. Add your detector's issues into `AndroidFrameworkIssueRegistry`'s `issues` field.
3. Write unit tests for your detector in one file and put it into
- `checks/test/java/com/google/android/lint`.
+ `framework/checks/test/java/com/google/android/lint`.
4. Done! Your lint checks should be applied in lint report builds for modules that include
`AndroidFrameworkLintChecker`.
@@ -44,7 +73,11 @@
environment variable with the id of the lint. For example:
`ANDROID_LINT_CHECK=UnusedTokenOfOriginalCallingIdentity m out/[...]/lint-report.html`
-## Create or update a baseline
+# How to apply automatic fixes suggested by lint
+
+See [lint_fix](fix/README.md)
+
+# Create or update a baseline
Baseline files can be used to silence known errors (and warnings) that are deemed to be safe. When
there is a lint-baseline.xml file in the root folder of the java library, soong will
@@ -75,9 +108,10 @@
[lint.go](http://cs/aosp-master/build/soong/java/lint.go;l=451;rcl=2e778d5bc4a8d1d77b4f4a3029a4a254ad57db75)
adding `cmd.Flag("--nowarn")` and running lint again.
-## Documentation
+# Documentation
- [Android Lint Docs](https://googlesamples.github.io/android-custom-lint-rules/)
+- [Lint Check Unit Testing](https://googlesamples.github.io/android-custom-lint-rules/api-guide/unit-testing.md.html)
- [Android Lint source files](https://source.corp.google.com/studio-main/tools/base/lint/libs/lint-api/src/main/java/com/android/tools/lint/)
- [PSI source files](https://github.com/JetBrains/intellij-community/tree/master/java/java-psi-api/src/com/intellij/psi)
- [UAST source files](https://upsource.jetbrains.com/idea-ce/structure/idea-ce-7b9b8cc138bbd90aec26433f82cd2c6838694003/uast/uast-common/src/org/jetbrains/uast)
diff --git a/tools/lint/checks/src/main/java/com/google/android/lint/EnforcePermissionDetector.kt b/tools/lint/checks/src/main/java/com/google/android/lint/EnforcePermissionDetector.kt
deleted file mode 100644
index 8f553ab..0000000
--- a/tools/lint/checks/src/main/java/com/google/android/lint/EnforcePermissionDetector.kt
+++ /dev/null
@@ -1,177 +0,0 @@
-/*
- * Copyright (C) 2022 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
-
-import com.android.tools.lint.detector.api.AnnotationInfo
-import com.android.tools.lint.detector.api.AnnotationOrigin
-import com.android.tools.lint.detector.api.AnnotationUsageInfo
-import com.android.tools.lint.detector.api.AnnotationUsageType
-import com.android.tools.lint.detector.api.ConstantEvaluator
-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.PsiAnnotation
-import com.intellij.psi.PsiClass
-import com.intellij.psi.PsiMethod
-import org.jetbrains.uast.UElement
-
-/**
- * Lint Detector that ensures that any method overriding a method annotated
- * with @EnforcePermission is also annotated with the exact same annotation.
- * The intent is to surface the effective permission checks to the service
- * implementations.
- */
-class EnforcePermissionDetector : Detector(), SourceCodeScanner {
-
- val ENFORCE_PERMISSION = "android.annotation.EnforcePermission"
-
- override fun applicableAnnotations(): List<String> {
- return listOf(ENFORCE_PERMISSION)
- }
-
- private fun areAnnotationsEquivalent(
- context: JavaContext,
- anno1: PsiAnnotation,
- anno2: PsiAnnotation
- ): Boolean {
- if (anno1.qualifiedName != anno2.qualifiedName) {
- return false
- }
- val attr1 = anno1.parameterList.attributes
- val attr2 = anno2.parameterList.attributes
- if (attr1.size != attr2.size) {
- return false
- }
- for (i in attr1.indices) {
- if (attr1[i].name != attr2[i].name) {
- return false
- }
- val value1 = attr1[i].value
- val value2 = attr2[i].value
- if (value1 == null && value2 == null) {
- continue
- }
- if (value1 == null || value2 == null) {
- return false
- }
- val v1 = ConstantEvaluator.evaluate(context, value1)
- val v2 = ConstantEvaluator.evaluate(context, value2)
- if (v1 != v2) {
- return false
- }
- }
- return true
- }
-
- override fun visitAnnotationUsage(
- context: JavaContext,
- element: UElement,
- annotationInfo: AnnotationInfo,
- usageInfo: AnnotationUsageInfo
- ) {
- if (usageInfo.type == AnnotationUsageType.EXTENDS) {
- val newClass = element.sourcePsi?.parent?.parent as PsiClass
- val extendedClass: PsiClass = usageInfo.referenced as PsiClass
- val newAnnotation = newClass.getAnnotation(ENFORCE_PERMISSION)
- val extendedAnnotation = extendedClass.getAnnotation(ENFORCE_PERMISSION)!!
-
- val location = context.getLocation(element)
- val newClassName = newClass.qualifiedName
- val extendedClassName = extendedClass.qualifiedName
- if (newAnnotation == null) {
- val msg = "The class $newClassName extends the class $extendedClassName which " +
- "is annotated with @EnforcePermission. The same annotation must be used " +
- "on $newClassName."
- context.report(ISSUE_MISSING_ENFORCE_PERMISSION, element, location, msg)
- } else if (!areAnnotationsEquivalent(context, newAnnotation, extendedAnnotation)) {
- val msg = "The class $newClassName is annotated with ${newAnnotation.text} " +
- "which differs from the parent class $extendedClassName: " +
- "${extendedAnnotation.text}. The same annotation must be used for " +
- "both classes."
- context.report(ISSUE_MISMATCHING_ENFORCE_PERMISSION, element, location, msg)
- }
- } else if (usageInfo.type == AnnotationUsageType.METHOD_OVERRIDE &&
- annotationInfo.origin == AnnotationOrigin.METHOD) {
- val overridingMethod = element.sourcePsi as PsiMethod
- val overriddenMethod = usageInfo.referenced as PsiMethod
- val overridingAnnotation = overridingMethod.getAnnotation(ENFORCE_PERMISSION)
- val overriddenAnnotation = overriddenMethod.getAnnotation(ENFORCE_PERMISSION)!!
-
- val location = context.getLocation(element)
- val overridingClass = overridingMethod.parent as PsiClass
- val overriddenClass = overriddenMethod.parent as PsiClass
- val overridingName = "${overridingClass.name}.${overridingMethod.name}"
- val overriddenName = "${overriddenClass.name}.${overriddenMethod.name}"
- if (overridingAnnotation == null) {
- val msg = "The method $overridingName overrides the method $overriddenName which " +
- "is annotated with @EnforcePermission. The same annotation must be used " +
- "on $overridingName"
- context.report(ISSUE_MISSING_ENFORCE_PERMISSION, element, location, msg)
- } else if (!areAnnotationsEquivalent(
- context, overridingAnnotation, overriddenAnnotation)) {
- val msg = "The method $overridingName is annotated with " +
- "${overridingAnnotation.text} which differs from the overridden " +
- "method $overriddenName: ${overriddenAnnotation.text}. The same " +
- "annotation must be used for both methods."
- context.report(ISSUE_MISMATCHING_ENFORCE_PERMISSION, element, location, msg)
- }
- }
- }
-
- companion object {
- val EXPLANATION = """
- The @EnforcePermission annotation is used to indicate that the underlying binder code
- has already verified the caller's permissions before calling the appropriate method. The
- verification code is usually generated by the AIDL compiler, which also takes care of
- annotating the generated Java code.
-
- In order to surface that information to platform developers, the same annotation must be
- used on the implementation class or methods.
- """
-
- val ISSUE_MISSING_ENFORCE_PERMISSION: Issue = Issue.create(
- id = "MissingEnforcePermissionAnnotation",
- briefDescription = "Missing @EnforcePermission annotation on Binder method",
- explanation = EXPLANATION,
- category = Category.SECURITY,
- priority = 6,
- severity = Severity.ERROR,
- implementation = Implementation(
- EnforcePermissionDetector::class.java,
- Scope.JAVA_FILE_SCOPE
- )
- )
-
- val ISSUE_MISMATCHING_ENFORCE_PERMISSION: Issue = Issue.create(
- id = "MismatchingEnforcePermissionAnnotation",
- briefDescription = "Incorrect @EnforcePermission annotation on Binder method",
- explanation = EXPLANATION,
- category = Category.SECURITY,
- priority = 6,
- severity = Severity.ERROR,
- implementation = Implementation(
- EnforcePermissionDetector::class.java,
- Scope.JAVA_FILE_SCOPE
- )
- )
- }
-}
diff --git a/tools/lint/checks/src/test/java/com/google/android/lint/EnforcePermissionDetectorTest.kt b/tools/lint/checks/src/test/java/com/google/android/lint/EnforcePermissionDetectorTest.kt
deleted file mode 100644
index f5f4ebe..0000000
--- a/tools/lint/checks/src/test/java/com/google/android/lint/EnforcePermissionDetectorTest.kt
+++ /dev/null
@@ -1,202 +0,0 @@
-/*
- * Copyright (C) 2022 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
-
-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 EnforcePermissionDetectorTest : LintDetectorTest() {
- override fun getDetector(): Detector = EnforcePermissionDetector()
-
- override fun getIssues(): List<Issue> = listOf(
- EnforcePermissionDetector.ISSUE_MISSING_ENFORCE_PERMISSION,
- EnforcePermissionDetector.ISSUE_MISMATCHING_ENFORCE_PERMISSION
- )
-
- override fun lint(): TestLintTask = super.lint().allowMissingSdk(true)
-
- fun testDoesNotDetectIssuesCorrectAnnotationOnClass() {
- lint().files(java(
- """
- package test.pkg;
- @android.annotation.EnforcePermission(android.Manifest.permission.READ_PHONE_STATE)
- public class TestClass1 extends IFoo.Stub {
- public void testMethod() {}
- }
- """).indented(),
- *stubs
- )
- .run()
- .expectClean()
- }
-
- fun testDoesNotDetectIssuesCorrectAnnotationOnMethod() {
- lint().files(java(
- """
- package test.pkg;
- import android.annotation.EnforcePermission;
- public class TestClass2 extends IFooMethod.Stub {
- @Override
- @EnforcePermission(android.Manifest.permission.READ_PHONE_STATE)
- public void testMethod() {}
- }
- """).indented(),
- *stubs
- )
- .run()
- .expectClean()
- }
-
- fun testDetectIssuesMismatchingAnnotationOnClass() {
- lint().files(java(
- """
- package test.pkg;
- @android.annotation.EnforcePermission(android.Manifest.permission.INTERNET)
- public class TestClass3 extends IFoo.Stub {
- public void testMethod() {}
- }
- """).indented(),
- *stubs
- )
- .run()
- .expect("""src/test/pkg/TestClass3.java:3: Error: The class test.pkg.TestClass3 is \
-annotated with @android.annotation.EnforcePermission(android.Manifest.permission.INTERNET) \
-which differs from the parent class IFoo.Stub: \
-@android.annotation.EnforcePermission(android.Manifest.permission.READ_PHONE_STATE). The \
-same annotation must be used for both classes. [MismatchingEnforcePermissionAnnotation]
-public class TestClass3 extends IFoo.Stub {
- ~~~~~~~~~
-1 errors, 0 warnings""".addLineContinuation())
- }
-
- fun testDetectIssuesMismatchingAnnotationOnMethod() {
- lint().files(java(
- """
- package test.pkg;
- public class TestClass4 extends IFooMethod.Stub {
- @android.annotation.EnforcePermission(android.Manifest.permission.INTERNET)
- public void testMethod() {}
- }
- """).indented(),
- *stubs
- )
- .run()
- .expect("""src/test/pkg/TestClass4.java:4: Error: The method TestClass4.testMethod is \
-annotated with @android.annotation.EnforcePermission(android.Manifest.permission.INTERNET) \
-which differs from the overridden method Stub.testMethod: \
-@android.annotation.EnforcePermission(android.Manifest.permission.READ_PHONE_STATE). The same \
-annotation must be used for both methods. [MismatchingEnforcePermissionAnnotation]
- public void testMethod() {}
- ~~~~~~~~~~
-1 errors, 0 warnings""".addLineContinuation())
- }
-
- fun testDetectIssuesMissingAnnotationOnClass() {
- lint().files(java(
- """
- package test.pkg;
- public class TestClass5 extends IFoo.Stub {
- public void testMethod() {}
- }
- """).indented(),
- *stubs
- )
- .run()
- .expect("""src/test/pkg/TestClass5.java:2: Error: The class test.pkg.TestClass5 extends \
-the class IFoo.Stub which is annotated with @EnforcePermission. The same annotation must be \
-used on test.pkg.TestClass5. [MissingEnforcePermissionAnnotation]
-public class TestClass5 extends IFoo.Stub {
- ~~~~~~~~~
-1 errors, 0 warnings""".addLineContinuation())
- }
-
- fun testDetectIssuesMissingAnnotationOnMethod() {
- lint().files(java(
- """
- package test.pkg;
- public class TestClass6 extends IFooMethod.Stub {
- public void testMethod() {}
- }
- """).indented(),
- *stubs
- )
- .run()
- .expect("""src/test/pkg/TestClass6.java:3: Error: The method TestClass6.testMethod \
-overrides the method Stub.testMethod which is annotated with @EnforcePermission. The same \
-annotation must be used on TestClass6.testMethod [MissingEnforcePermissionAnnotation]
- public void testMethod() {}
- ~~~~~~~~~~
-1 errors, 0 warnings""".addLineContinuation())
- }
-
- /* Stubs */
-
- private val interfaceIFooStub: TestFile = java(
- """
- @android.annotation.EnforcePermission(android.Manifest.permission.READ_PHONE_STATE)
- public interface IFoo {
- @android.annotation.EnforcePermission(android.Manifest.permission.READ_PHONE_STATE)
- public static abstract class Stub implements IFoo {
- @Override
- public void testMethod() {}
- }
- public void testMethod();
- }
- """
- ).indented()
-
- private val interfaceIFooMethodStub: TestFile = java(
- """
- public interface IFooMethod {
- public static abstract class Stub implements IFooMethod {
- @Override
- @android.annotation.EnforcePermission(android.Manifest.permission.READ_PHONE_STATE)
- public void testMethod() {}
- }
- @android.annotation.EnforcePermission(android.Manifest.permission.READ_PHONE_STATE)
- public void testMethod();
- }
- """
- ).indented()
-
- private val manifestPermissionStub: TestFile = java(
- """
- package android.Manifest;
- class permission {
- public static final String READ_PHONE_STATE = "android.permission.READ_PHONE_STATE";
- public static final String INTERNET = "android.permission.INTERNET";
- }
- """
- ).indented()
-
- private val enforcePermissionAnnotationStub: TestFile = java(
- """
- package android.annotation;
- public @interface EnforcePermission {}
- """
- ).indented()
-
- private val stubs = arrayOf(interfaceIFooStub, interfaceIFooMethodStub,
- manifestPermissionStub, enforcePermissionAnnotationStub)
-
- // Substitutes "backslash + new line" with an empty string to imitate line continuation
- private fun String.addLineContinuation(): String = this.trimIndent().replace("\\\n", "")
-}
diff --git a/tools/lint/common/Android.bp b/tools/lint/common/Android.bp
new file mode 100644
index 0000000..898f88b
--- /dev/null
+++ b/tools/lint/common/Android.bp
@@ -0,0 +1,29 @@
+// Copyright (C) 2022 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 {
+ // See: http://go/android-license-faq
+ // A large-scale-change added 'default_applicable_licenses' to import
+ // all of the 'license_kinds' from "frameworks_base_license"
+ // to get the below license kinds:
+ // SPDX-license-identifier-Apache-2.0
+ default_applicable_licenses: ["frameworks_base_license"],
+}
+
+java_library_host {
+ name: "AndroidCommonLint",
+ srcs: ["src/main/java/**/*.kt"],
+ libs: ["lint_api"],
+ kotlincflags: ["-Xjvm-default=all"],
+}
diff --git a/tools/lint/common/src/main/java/com/google/android/lint/Constants.kt b/tools/lint/common/src/main/java/com/google/android/lint/Constants.kt
new file mode 100644
index 0000000..0ef165f
--- /dev/null
+++ b/tools/lint/common/src/main/java/com/google/android/lint/Constants.kt
@@ -0,0 +1,40 @@
+/*
+ * Copyright (C) 2022 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
+
+import com.google.android.lint.model.Method
+
+const val CLASS_STUB = "Stub"
+const val CLASS_CONTEXT = "android.content.Context"
+const val CLASS_ACTIVITY_MANAGER_SERVICE = "com.android.server.am.ActivityManagerService"
+const val CLASS_ACTIVITY_MANAGER_INTERNAL = "android.app.ActivityManagerInternal"
+
+// Enforce permission APIs
+val ENFORCE_PERMISSION_METHODS = listOf(
+ Method(CLASS_CONTEXT, "checkPermission"),
+ Method(CLASS_CONTEXT, "checkCallingPermission"),
+ Method(CLASS_CONTEXT, "checkCallingOrSelfPermission"),
+ Method(CLASS_CONTEXT, "enforcePermission"),
+ Method(CLASS_CONTEXT, "enforceCallingPermission"),
+ Method(CLASS_CONTEXT, "enforceCallingOrSelfPermission"),
+ Method(CLASS_ACTIVITY_MANAGER_SERVICE, "checkPermission"),
+ Method(CLASS_ACTIVITY_MANAGER_INTERNAL, "enforceCallingPermission")
+)
+
+const val ANNOTATION_PERMISSION_METHOD = "android.annotation.PermissionMethod"
+const val ANNOTATION_PERMISSION_NAME = "android.annotation.PermissionName"
+const val ANNOTATION_PERMISSION_RESULT = "android.content.pm.PackageManager.PermissionResult"
diff --git a/tools/lint/common/src/main/java/com/google/android/lint/PermissionMethodUtils.kt b/tools/lint/common/src/main/java/com/google/android/lint/PermissionMethodUtils.kt
new file mode 100644
index 0000000..9a7f8fa
--- /dev/null
+++ b/tools/lint/common/src/main/java/com/google/android/lint/PermissionMethodUtils.kt
@@ -0,0 +1,52 @@
+/*
+ * Copyright (C) 2022 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
+
+import com.android.tools.lint.detector.api.getUMethod
+import org.jetbrains.uast.UAnnotation
+import org.jetbrains.uast.UCallExpression
+import org.jetbrains.uast.UElement
+import org.jetbrains.uast.UMethod
+import org.jetbrains.uast.UParameter
+import org.jetbrains.uast.UQualifiedReferenceExpression
+
+fun isPermissionMethodCall(callExpression: UCallExpression): Boolean {
+ val method = callExpression.resolve()?.getUMethod() ?: return false
+ return hasPermissionMethodAnnotation(method)
+}
+
+fun hasPermissionMethodAnnotation(method: UMethod): Boolean =
+ getPermissionMethodAnnotation(method) != null
+
+fun getPermissionMethodAnnotation(method: UMethod?): UAnnotation? = method?.uAnnotations
+ ?.firstOrNull { it.qualifiedName == ANNOTATION_PERMISSION_METHOD }
+
+fun hasPermissionNameAnnotation(parameter: UParameter) = parameter.annotations.any {
+ it.hasQualifiedName(ANNOTATION_PERMISSION_NAME)
+}
+
+/**
+ * Attempts to return a CallExpression from a QualifiedReferenceExpression (or returns it directly if passed directly)
+ * @param callOrReferenceCall expected to be UCallExpression or UQualifiedReferenceExpression
+ * @return UCallExpression, if available
+ */
+fun findCallExpression(callOrReferenceCall: UElement?): UCallExpression? =
+ when (callOrReferenceCall) {
+ is UCallExpression -> callOrReferenceCall
+ is UQualifiedReferenceExpression -> callOrReferenceCall.selector as? UCallExpression
+ else -> null
+ }
diff --git a/tools/lint/common/src/main/java/com/google/android/lint/model/Method.kt b/tools/lint/common/src/main/java/com/google/android/lint/model/Method.kt
new file mode 100644
index 0000000..3939b61
--- /dev/null
+++ b/tools/lint/common/src/main/java/com/google/android/lint/model/Method.kt
@@ -0,0 +1,26 @@
+/*
+ * Copyright (C) 2022 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.model
+
+/**
+ * Data class to represent a Method
+ */
+data class Method(val clazz: String, val name: String) {
+ override fun toString(): String {
+ return "$clazz#$name"
+ }
+}
diff --git a/tools/lint/fix/Android.bp b/tools/lint/fix/Android.bp
new file mode 100644
index 0000000..43f2122
--- /dev/null
+++ b/tools/lint/fix/Android.bp
@@ -0,0 +1,33 @@
+// Copyright (C) 2022 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 {
+ // See: http://go/android-license-faq
+ // A large-scale-change added 'default_applicable_licenses' to import
+ // all of the 'license_kinds' from "frameworks_base_license"
+ // to get the below license kinds:
+ // SPDX-license-identifier-Apache-2.0
+ default_applicable_licenses: ["frameworks_base_license"],
+}
+
+python_binary_host {
+ name: "lint_fix",
+ main: "soong_lint_fix.py",
+ srcs: ["soong_lint_fix.py"],
+}
+
+python_library_host {
+ name: "soong_lint_fix",
+ srcs: ["soong_lint_fix.py"],
+}
diff --git a/tools/lint/fix/README.md b/tools/lint/fix/README.md
new file mode 100644
index 0000000..a5ac2be
--- /dev/null
+++ b/tools/lint/fix/README.md
@@ -0,0 +1,30 @@
+# Refactoring the platform with lint
+Inspiration: go/refactor-the-platform-with-lint\
+**Special Thanks: brufino@, azharaa@, for the prior work that made this all possible**
+
+## What is this?
+
+It's a python script that runs the framework linter,
+and then (optionally) copies modified files back into the source tree.\
+Why python, you ask? Because python is cool ¯\_(ツ)_/¯.
+
+Incidentally, this exposes a much simpler way to run individual lint checks
+against individual modules, so it's useful beyond applying fixes.
+
+## Why?
+
+Lint is not allowed to modify source files directly via lint's `--apply-suggestions` flag.
+As a compromise, soong zips up the (potentially) modified sources and leaves them in an intermediate
+directory. This script runs the lint, unpacks those files, and copies them back into the tree.
+
+## How do I run it?
+**WARNING: You probably want to commit/stash any changes to your working tree before doing this...**
+
+```
+source build/envsetup.sh
+lunch cf_x86_64_phone-userdebug # or any lunch target
+m lint_fix
+lint_fix -h
+```
+
+The script's help output explains things that are omitted here.
diff --git a/tools/lint/fix/soong_lint_fix.py b/tools/lint/fix/soong_lint_fix.py
new file mode 100644
index 0000000..cd4d778d
--- /dev/null
+++ b/tools/lint/fix/soong_lint_fix.py
@@ -0,0 +1,173 @@
+# Copyright (C) 2022 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.
+
+import argparse
+import json
+import os
+import shutil
+import subprocess
+import sys
+import zipfile
+
+ANDROID_BUILD_TOP = os.environ.get("ANDROID_BUILD_TOP")
+ANDROID_PRODUCT_OUT = os.environ.get("ANDROID_PRODUCT_OUT")
+PRODUCT_OUT = ANDROID_PRODUCT_OUT.removeprefix(f"{ANDROID_BUILD_TOP}/")
+
+SOONG_UI = "build/soong/soong_ui.bash"
+PATH_PREFIX = "out/soong/.intermediates"
+PATH_SUFFIX = "android_common/lint"
+FIX_ZIP = "suggested-fixes.zip"
+
+class SoongLintFix:
+ """
+ This class creates a command line tool that will
+ apply lint fixes to the platform via the necessary
+ combination of soong and shell commands.
+
+ It breaks up these operations into a few "private" methods
+ that are intentionally exposed so experimental code can tweak behavior.
+
+ The entry point, `run`, will apply lint fixes using the
+ intermediate `suggested-fixes` directory that soong creates during its
+ invocation of lint.
+
+ Basic usage:
+ ```
+ from soong_lint_fix import SoongLintFix
+
+ SoongLintFix().run()
+ ```
+ """
+ def __init__(self):
+ self._parser = _setup_parser()
+ self._args = None
+ self._kwargs = None
+ self._path = None
+ self._target = None
+
+
+ def run(self, additional_setup=None, custom_fix=None):
+ """
+ Run the script
+ """
+ self._setup()
+ self._find_module()
+ self._lint()
+
+ if not self._args.no_fix:
+ self._fix()
+
+ if self._args.print:
+ self._print()
+
+ def _setup(self):
+ self._args = self._parser.parse_args()
+ env = os.environ.copy()
+ if self._args.check:
+ env["ANDROID_LINT_CHECK"] = self._args.check
+ if self._args.lint_module:
+ env["ANDROID_LINT_CHECK_EXTRA_MODULES"] = self._args.lint_module
+
+ self._kwargs = {
+ "env": env,
+ "executable": "/bin/bash",
+ "shell": True,
+ }
+
+ os.chdir(ANDROID_BUILD_TOP)
+
+
+ def _find_module(self):
+ print("Refreshing soong modules...")
+ try:
+ os.mkdir(ANDROID_PRODUCT_OUT)
+ except OSError:
+ pass
+ subprocess.call(f"{SOONG_UI} --make-mode {PRODUCT_OUT}/module-info.json", **self._kwargs)
+ print("done.")
+
+ with open(f"{ANDROID_PRODUCT_OUT}/module-info.json") as f:
+ module_info = json.load(f)
+
+ if self._args.module not in module_info:
+ sys.exit(f"Module {self._args.module} not found!")
+
+ module_path = module_info[self._args.module]["path"][0]
+ print(f"Found module {module_path}/{self._args.module}.")
+
+ self._path = f"{PATH_PREFIX}/{module_path}/{self._args.module}/{PATH_SUFFIX}"
+ self._target = f"{self._path}/lint-report.txt"
+
+
+ def _lint(self):
+ print("Cleaning up any old lint results...")
+ try:
+ os.remove(f"{self._target}")
+ os.remove(f"{self._path}/{FIX_ZIP}")
+ except FileNotFoundError:
+ pass
+ print("done.")
+
+ print(f"Generating {self._target}")
+ subprocess.call(f"{SOONG_UI} --make-mode {self._target}", **self._kwargs)
+ print("done.")
+
+
+ def _fix(self):
+ print("Copying suggested fixes to the tree...")
+ with zipfile.ZipFile(f"{self._path}/{FIX_ZIP}") as zip:
+ for name in zip.namelist():
+ if name.startswith("out") or not name.endswith(".java"):
+ continue
+ with zip.open(name) as src, open(f"{ANDROID_BUILD_TOP}/{name}", "wb") as dst:
+ shutil.copyfileobj(src, dst)
+ print("done.")
+
+
+ def _print(self):
+ print("### lint-report.txt ###", end="\n\n")
+ with open(self._target, "r") as f:
+ print(f.read())
+
+
+def _setup_parser():
+ parser = argparse.ArgumentParser(description="""
+ This is a python script that applies lint fixes to the platform:
+ 1. Set up the environment, etc.
+ 2. Run lint on the specified target.
+ 3. Copy the modified files, from soong's intermediate directory, back into the tree.
+
+ **Gotcha**: You must have run `source build/envsetup.sh` and `lunch` first.
+ """, formatter_class=argparse.RawTextHelpFormatter)
+
+ parser.add_argument('module',
+ help='The soong build module to run '
+ '(e.g. framework-minus-apex or services.core.unboosted)')
+
+ parser.add_argument('--check',
+ help='Which lint to run. Passed to the ANDROID_LINT_CHECK environment variable.')
+
+ parser.add_argument('--lint-module',
+ help='Specific lint module to run. Passed to the ANDROID_LINT_CHECK_EXTRA_MODULES environment variable.')
+
+ parser.add_argument('--no-fix', action='store_true',
+ help='Just build and run the lint, do NOT apply the fixes.')
+
+ parser.add_argument('--print', action='store_true',
+ help='Print the contents of the generated lint-report.txt at the end.')
+
+ return parser
+
+if __name__ == "__main__":
+ SoongLintFix().run()
\ No newline at end of file
diff --git a/tools/lint/Android.bp b/tools/lint/framework/Android.bp
similarity index 68%
rename from tools/lint/Android.bp
rename to tools/lint/framework/Android.bp
index 2601041..30a6daa 100644
--- a/tools/lint/Android.bp
+++ b/tools/lint/framework/Android.bp
@@ -1,4 +1,4 @@
-// Copyright (C) 2021 The Android Open Source Project
+// Copyright (C) 2022 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.
@@ -29,17 +29,14 @@
"auto_service_annotations",
"lint_api",
],
+ static_libs: [
+ "AndroidCommonLint",
+ ],
kotlincflags: ["-Xjvm-default=all"],
}
java_test_host {
name: "AndroidFrameworkLintCheckerTest",
- // TODO(b/239881504): Since this test was written, Android
- // Lint was updated, and now includes classes that were
- // compiled for java 15. The soong build doesn't support
- // java 15 yet, so we can't compile against "lint". Disable
- // the test until java 15 is supported.
- enabled: false,
srcs: ["checks/src/test/java/**/*.kt"],
static_libs: [
"AndroidFrameworkLintChecker",
@@ -49,5 +46,19 @@
],
test_options: {
unit_test: true,
+ tradefed_options: [
+ {
+ // lint bundles in some classes that were built with older versions
+ // of libraries, and no longer load. Since tradefed tries to load
+ // all classes in the jar to look for tests, it crashes loading them.
+ // Exclude these classes from tradefed's search.
+ name: "exclude-paths",
+ value: "org/apache",
+ },
+ {
+ name: "exclude-paths",
+ value: "META-INF",
+ },
+ ],
},
}
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
new file mode 100644
index 0000000..b4f6a1c
--- /dev/null
+++ b/tools/lint/framework/checks/src/main/java/com/google/android/lint/AndroidFrameworkIssueRegistry.kt
@@ -0,0 +1,54 @@
+/*
+ * Copyright (C) 2021 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
+
+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.auto.service.AutoService
+
+@AutoService(IssueRegistry::class)
+@Suppress("UnstableApiUsage")
+class AndroidFrameworkIssueRegistry : IssueRegistry() {
+ override val issues = listOf(
+ CallingIdentityTokenDetector.ISSUE_UNUSED_TOKEN,
+ CallingIdentityTokenDetector.ISSUE_NON_FINAL_TOKEN,
+ CallingIdentityTokenDetector.ISSUE_NESTED_CLEAR_IDENTITY_CALLS,
+ CallingIdentityTokenDetector.ISSUE_RESTORE_IDENTITY_CALL_NOT_IN_FINALLY_BLOCK,
+ CallingIdentityTokenDetector.ISSUE_USE_OF_CALLER_AWARE_METHODS_WITH_CLEARED_IDENTITY,
+ CallingIdentityTokenDetector.ISSUE_CLEAR_IDENTITY_CALL_NOT_FOLLOWED_BY_TRY_FINALLY,
+ CallingIdentityTokenDetector.ISSUE_RESULT_OF_CLEAR_IDENTITY_CALL_NOT_STORED_IN_VARIABLE,
+ CallingSettingsNonUserGetterMethodsDetector.ISSUE_NON_USER_GETTER_CALLED,
+ SaferParcelChecker.ISSUE_UNSAFE_API_USAGE,
+ PackageVisibilityDetector.ISSUE_PACKAGE_NAME_NO_PACKAGE_VISIBILITY_FILTERS,
+ PermissionMethodDetector.ISSUE_PERMISSION_METHOD_USAGE,
+ PermissionMethodDetector.ISSUE_CAN_BE_PERMISSION_METHOD,
+ )
+
+ override val api: Int
+ get() = CURRENT_API
+
+ override val minApi: Int
+ get() = 8
+
+ override val vendor: Vendor = Vendor(
+ vendorName = "Android",
+ feedbackUrl = "http://b/issues/new?component=315013",
+ contact = "brufino@google.com"
+ )
+}
diff --git a/tools/lint/checks/src/main/java/com/google/android/lint/CallingIdentityTokenDetector.kt b/tools/lint/framework/checks/src/main/java/com/google/android/lint/CallingIdentityTokenDetector.kt
similarity index 64%
rename from tools/lint/checks/src/main/java/com/google/android/lint/CallingIdentityTokenDetector.kt
rename to tools/lint/framework/checks/src/main/java/com/google/android/lint/CallingIdentityTokenDetector.kt
index 930378b..0c375c3 100644
--- a/tools/lint/checks/src/main/java/com/google/android/lint/CallingIdentityTokenDetector.kt
+++ b/tools/lint/framework/checks/src/main/java/com/google/android/lint/CallingIdentityTokenDetector.kt
@@ -33,6 +33,7 @@
import org.jetbrains.uast.UCallExpression
import org.jetbrains.uast.UDeclarationsExpression
import org.jetbrains.uast.UElement
+import org.jetbrains.uast.UIfExpression
import org.jetbrains.uast.ULocalVariable
import org.jetbrains.uast.USimpleNameReferenceExpression
import org.jetbrains.uast.UTryExpression
@@ -52,10 +53,10 @@
private val tokensMap = mutableMapOf<String, Token>()
override fun getApplicableUastTypes(): List<Class<out UElement?>> =
- listOf(ULocalVariable::class.java, UCallExpression::class.java)
+ listOf(ULocalVariable::class.java, UCallExpression::class.java)
override fun createUastHandler(context: JavaContext): UElementHandler =
- TokenUastHandler(context)
+ TokenUastHandler(context)
/** File analysis starts with a clear map */
override fun beforeCheckFile(context: Context) {
@@ -70,9 +71,9 @@
override fun afterCheckFile(context: Context) {
for (token in tokensMap.values) {
context.report(
- ISSUE_UNUSED_TOKEN,
- token.location,
- getIncidentMessageUnusedToken(token.variableName)
+ ISSUE_UNUSED_TOKEN,
+ token.location,
+ getIncidentMessageUnusedToken(token.variableName)
)
}
tokensMap.clear()
@@ -96,9 +97,9 @@
val variableName = node.getName()
if (!node.isFinal) {
context.report(
- ISSUE_NON_FINAL_TOKEN,
- location,
- getIncidentMessageNonFinalToken(variableName)
+ ISSUE_NON_FINAL_TOKEN,
+ location,
+ getIncidentMessageNonFinalToken(variableName)
)
}
// If there exists an unused variable with the same name in the map, we can imply that
@@ -106,9 +107,9 @@
val oldToken = tokensMap[variableName]
if (oldToken != null) {
context.report(
- ISSUE_UNUSED_TOKEN,
- oldToken.location,
- getIncidentMessageUnusedToken(oldToken.variableName)
+ ISSUE_UNUSED_TOKEN,
+ oldToken.location,
+ getIncidentMessageUnusedToken(oldToken.variableName)
)
}
// If there exists a token in the same scope as the current new token, it means that
@@ -117,56 +118,84 @@
val firstCallToken = findFirstTokenInScope(node)
if (firstCallToken != null) {
context.report(
- ISSUE_NESTED_CLEAR_IDENTITY_CALLS,
- createNestedLocation(firstCallToken, location),
- getIncidentMessageNestedClearIdentityCallsPrimary(
- firstCallToken.variableName,
- variableName
- )
+ ISSUE_NESTED_CLEAR_IDENTITY_CALLS,
+ createNestedLocation(firstCallToken, location),
+ getIncidentMessageNestedClearIdentityCallsPrimary(
+ firstCallToken.variableName,
+ variableName
+ )
)
}
// If the next statement in the tree is not a try-finally statement, we need to report
// the "clearCallingIdentity() is not followed by try-finally" issue
val finallyClause = (getNextStatementOfLocalVariable(node) as? UTryExpression)
- ?.finallyClause
+ ?.finallyClause
if (finallyClause == null) {
context.report(
- ISSUE_CLEAR_IDENTITY_CALL_NOT_FOLLOWED_BY_TRY_FINALLY,
- location,
- getIncidentMessageClearIdentityCallNotFollowedByTryFinally(variableName)
+ ISSUE_CLEAR_IDENTITY_CALL_NOT_FOLLOWED_BY_TRY_FINALLY,
+ location,
+ getIncidentMessageClearIdentityCallNotFollowedByTryFinally(variableName)
)
}
tokensMap[variableName] = Token(
- variableName,
- node.sourcePsi?.getUseScope(),
- location,
- finallyClause
+ variableName,
+ node.sourcePsi?.getUseScope(),
+ location,
+ finallyClause
)
}
+ override fun visitCallExpression(node: UCallExpression) {
+ when {
+ isMethodCall(node, Method.BINDER_CLEAR_CALLING_IDENTITY) -> {
+ checkClearCallingIdentityCall(node)
+ }
+ isMethodCall(node, Method.BINDER_RESTORE_CALLING_IDENTITY) -> {
+ checkRestoreCallingIdentityCall(node)
+ }
+ isCallerAwareMethod(node) -> checkCallerAwareMethod(node)
+ }
+ }
+
+ private fun checkClearCallingIdentityCall(node: UCallExpression) {
+ var firstNonQualifiedParent = getFirstNonQualifiedParent(node)
+ // if the call expression is inside a ternary, and the ternary is assigned
+ // to a variable, then we are still technically assigning
+ // any result of clearCallingIdentity to a variable
+ if (firstNonQualifiedParent is UIfExpression && firstNonQualifiedParent.isTernary) {
+ firstNonQualifiedParent = firstNonQualifiedParent.uastParent
+ }
+ if (firstNonQualifiedParent !is ULocalVariable) {
+ context.report(
+ ISSUE_RESULT_OF_CLEAR_IDENTITY_CALL_NOT_STORED_IN_VARIABLE,
+ context.getLocation(node),
+ getIncidentMessageResultOfClearIdentityCallNotStoredInVariable(
+ node.getQualifiedParentOrThis().asRenderString()
+ )
+ )
+ }
+ }
+
+ private fun checkCallerAwareMethod(node: UCallExpression) {
+ val token = findFirstTokenInScope(node)
+ if (token != null) {
+ context.report(
+ ISSUE_USE_OF_CALLER_AWARE_METHODS_WITH_CLEARED_IDENTITY,
+ context.getLocation(node),
+ getIncidentMessageUseOfCallerAwareMethodsWithClearedIdentity(
+ token.variableName,
+ node.asRenderString()
+ )
+ )
+ }
+ }
+
/**
- * For every method():
- * - Checks use of caller-aware methods issue
- * For every call of Binder.restoreCallingIdentity(token):
* - Checks for restoreCallingIdentity() not in the finally block issue
* - Removes token from tokensMap if token is within the scope of the method
*/
- override fun visitCallExpression(node: UCallExpression) {
- val token = findFirstTokenInScope(node)
- if (isCallerAwareMethod(node) && token != null) {
- context.report(
- ISSUE_USE_OF_CALLER_AWARE_METHODS_WITH_CLEARED_IDENTITY,
- context.getLocation(node),
- getIncidentMessageUseOfCallerAwareMethodsWithClearedIdentity(
- token.variableName,
- node.asRenderString()
- )
- )
- return
- }
- if (!isMethodCall(node, Method.BINDER_RESTORE_CALLING_IDENTITY)) return
- val first = node.valueArguments[0].skipParenthesizedExprDown()
- val arg = first as? USimpleNameReferenceExpression ?: return
+ private fun checkRestoreCallingIdentityCall(node: UCallExpression) {
+ val arg = node.valueArguments[0] as? USimpleNameReferenceExpression ?: return
val variableName = arg.identifier
val originalScope = tokensMap[variableName]?.scope ?: return
val psi = arg.sourcePsi ?: return
@@ -174,26 +203,31 @@
// token declaration. If not within the scope, no action is needed because the token is
// irrelevant i.e. not in the same scope or was not declared with clearCallingIdentity()
if (!PsiSearchScopeUtil.isInScope(originalScope, psi)) return
- // - We do not report "restore identity call not in finally" issue when there is no
+ // We do not report "restore identity call not in finally" issue when there is no
// finally block because that case is already handled by "clear identity call not
// followed by try-finally" issue
- // - UCallExpression can be a child of UQualifiedReferenceExpression, i.e.
- // receiver.selector, so to get the call's immediate parent we need to get the topmost
- // parent qualified reference expression and access its parent
if (tokensMap[variableName]?.finallyBlock != null &&
- skipParenthesizedExprUp(node.getQualifiedParentOrThis().uastParent) !=
- tokensMap[variableName]?.finallyBlock) {
+ getFirstNonQualifiedParent(node) !=
+ tokensMap[variableName]?.finallyBlock
+ ) {
context.report(
- ISSUE_RESTORE_IDENTITY_CALL_NOT_IN_FINALLY_BLOCK,
- context.getLocation(node),
- getIncidentMessageRestoreIdentityCallNotInFinallyBlock(variableName)
+ ISSUE_RESTORE_IDENTITY_CALL_NOT_IN_FINALLY_BLOCK,
+ context.getLocation(node),
+ getIncidentMessageRestoreIdentityCallNotInFinallyBlock(variableName)
)
}
tokensMap.remove(variableName)
}
+ private fun getFirstNonQualifiedParent(expression: UCallExpression): UElement? {
+ // UCallExpression can be a child of UQualifiedReferenceExpression, i.e.
+ // receiver.selector, so to get the call's immediate parent we need to get the topmost
+ // parent qualified reference expression and access its parent
+ return skipParenthesizedExprUp(expression.getQualifiedParentOrThis().uastParent)
+ }
+
private fun isCallerAwareMethod(expression: UCallExpression): Boolean =
- callerAwareMethods.any { method -> isMethodCall(expression, method) }
+ callerAwareMethods.any { method -> isMethodCall(expression, method) }
private fun isMethodCall(
expression: UCallExpression,
@@ -201,12 +235,12 @@
): Boolean {
val psiMethod = expression.resolve() ?: return false
return psiMethod.getName() == method.methodName &&
- context.evaluator.methodMatches(
- psiMethod,
- method.className,
- /* allowInherit */ true,
- *method.args
- )
+ context.evaluator.methodMatches(
+ psiMethod,
+ method.className,
+ /* allowInherit */ true,
+ *method.args
+ )
}
/**
@@ -255,7 +289,7 @@
return declarations[indexInDeclarations + 1]
}
val enclosingBlock = node
- .getParentOfType<UBlockExpression>(strict = true) ?: return null
+ .getParentOfType<UBlockExpression>(strict = true) ?: return null
val expressions = enclosingBlock.expressions
val indexInBlock = expressions.indexOf(declarationsExpression as UElement)
return if (indexInBlock == -1) null else expressions.getOrNull(indexInBlock + 1)
@@ -301,12 +335,12 @@
secondCallTokenLocation: Location
): Location {
return cloneLocation(secondCallTokenLocation)
- .withSecondary(
- cloneLocation(firstCallToken.location),
- getIncidentMessageNestedClearIdentityCallsSecondary(
- firstCallToken.variableName
- )
+ .withSecondary(
+ cloneLocation(firstCallToken.location),
+ getIncidentMessageNestedClearIdentityCallsSecondary(
+ firstCallToken.variableName
)
+ )
}
private fun cloneLocation(location: Location): Location {
@@ -347,20 +381,20 @@
const val CLASS_USER_HANDLE = "android.os.UserHandle"
private val callerAwareMethods = listOf(
- Method.BINDER_GET_CALLING_PID,
- Method.BINDER_GET_CALLING_UID,
- Method.BINDER_GET_CALLING_UID_OR_THROW,
- Method.BINDER_GET_CALLING_USER_HANDLE,
- Method.USER_HANDLE_GET_CALLING_APP_ID,
- Method.USER_HANDLE_GET_CALLING_USER_ID
+ Method.BINDER_GET_CALLING_PID,
+ Method.BINDER_GET_CALLING_UID,
+ Method.BINDER_GET_CALLING_UID_OR_THROW,
+ Method.BINDER_GET_CALLING_USER_HANDLE,
+ Method.USER_HANDLE_GET_CALLING_APP_ID,
+ Method.USER_HANDLE_GET_CALLING_USER_ID
)
/** Issue: unused token from Binder.clearCallingIdentity() */
@JvmField
val ISSUE_UNUSED_TOKEN: Issue = Issue.create(
- id = "UnusedTokenOfOriginalCallingIdentity",
- briefDescription = "Unused token of Binder.clearCallingIdentity()",
- explanation = """
+ id = "UnusedTokenOfOriginalCallingIdentity",
+ briefDescription = "Unused token of Binder.clearCallingIdentity()",
+ explanation = """
You cleared the original calling identity with \
`Binder.clearCallingIdentity()`, but have not used the returned token to \
restore the identity.
@@ -370,26 +404,26 @@
`token` is the result of `Binder.clearCallingIdentity()`
""",
- category = Category.SECURITY,
- priority = 6,
- severity = Severity.WARNING,
- implementation = Implementation(
- CallingIdentityTokenDetector::class.java,
- Scope.JAVA_FILE_SCOPE
- )
+ category = Category.SECURITY,
+ priority = 6,
+ severity = Severity.WARNING,
+ implementation = Implementation(
+ CallingIdentityTokenDetector::class.java,
+ Scope.JAVA_FILE_SCOPE
+ )
)
private fun getIncidentMessageUnusedToken(variableName: String) = "`$variableName` has " +
- "not been used to restore the calling identity. Introduce a `try`-`finally` " +
- "after the declaration and call `Binder.restoreCallingIdentity($variableName)` " +
- "in `finally` or remove `$variableName`."
+ "not been used to restore the calling identity. Introduce a `try`-`finally` " +
+ "after the declaration and call `Binder.restoreCallingIdentity($variableName)` " +
+ "in `finally` or remove `$variableName`."
/** Issue: non-final token from Binder.clearCallingIdentity() */
@JvmField
val ISSUE_NON_FINAL_TOKEN: Issue = Issue.create(
- id = "NonFinalTokenOfOriginalCallingIdentity",
- briefDescription = "Non-final token of Binder.clearCallingIdentity()",
- explanation = """
+ id = "NonFinalTokenOfOriginalCallingIdentity",
+ briefDescription = "Non-final token of Binder.clearCallingIdentity()",
+ explanation = """
You cleared the original calling identity with \
`Binder.clearCallingIdentity()`, but have not made the returned token `final`.
@@ -397,47 +431,47 @@
which can cause problems when restoring the identity with \
`Binder.restoreCallingIdentity(token)`.
""",
- category = Category.SECURITY,
- priority = 6,
- severity = Severity.WARNING,
- implementation = Implementation(
- CallingIdentityTokenDetector::class.java,
- Scope.JAVA_FILE_SCOPE
- )
+ category = Category.SECURITY,
+ priority = 6,
+ severity = Severity.WARNING,
+ implementation = Implementation(
+ CallingIdentityTokenDetector::class.java,
+ Scope.JAVA_FILE_SCOPE
+ )
)
private fun getIncidentMessageNonFinalToken(variableName: String) = "`$variableName` is " +
- "a non-final token from `Binder.clearCallingIdentity()`. Add `final` keyword to " +
- "`$variableName`."
+ "a non-final token from `Binder.clearCallingIdentity()`. Add `final` keyword to " +
+ "`$variableName`."
/** Issue: nested calls of Binder.clearCallingIdentity() */
@JvmField
val ISSUE_NESTED_CLEAR_IDENTITY_CALLS: Issue = Issue.create(
- id = "NestedClearCallingIdentityCalls",
- briefDescription = "Nested calls of Binder.clearCallingIdentity()",
- explanation = """
+ id = "NestedClearCallingIdentityCalls",
+ briefDescription = "Nested calls of Binder.clearCallingIdentity()",
+ explanation = """
You cleared the original calling identity with \
`Binder.clearCallingIdentity()` twice without restoring identity with the \
result of the first call.
Make sure to restore the identity after each clear identity call.
""",
- category = Category.SECURITY,
- priority = 6,
- severity = Severity.WARNING,
- implementation = Implementation(
- CallingIdentityTokenDetector::class.java,
- Scope.JAVA_FILE_SCOPE
- )
+ category = Category.SECURITY,
+ priority = 6,
+ severity = Severity.WARNING,
+ implementation = Implementation(
+ CallingIdentityTokenDetector::class.java,
+ Scope.JAVA_FILE_SCOPE
+ )
)
private fun getIncidentMessageNestedClearIdentityCallsPrimary(
firstCallVariableName: String,
secondCallVariableName: String
): String = "The calling identity has already been cleared and returned into " +
- "`$firstCallVariableName`. Move `$secondCallVariableName` declaration after " +
- "restoring the calling identity with " +
- "`Binder.restoreCallingIdentity($firstCallVariableName)`."
+ "`$firstCallVariableName`. Move `$secondCallVariableName` declaration after " +
+ "restoring the calling identity with " +
+ "`Binder.restoreCallingIdentity($firstCallVariableName)`."
private fun getIncidentMessageNestedClearIdentityCallsSecondary(
firstCallVariableName: String
@@ -446,10 +480,10 @@
/** Issue: Binder.clearCallingIdentity() is not followed by `try-finally` statement */
@JvmField
val ISSUE_CLEAR_IDENTITY_CALL_NOT_FOLLOWED_BY_TRY_FINALLY: Issue = Issue.create(
- id = "ClearIdentityCallNotFollowedByTryFinally",
- briefDescription = "Binder.clearCallingIdentity() is not followed by try-finally " +
- "statement",
- explanation = """
+ id = "ClearIdentityCallNotFollowedByTryFinally",
+ briefDescription = "Binder.clearCallingIdentity() is not followed by try-finally " +
+ "statement",
+ explanation = """
You cleared the original calling identity with \
`Binder.clearCallingIdentity()`, but the next statement is not a `try` \
statement.
@@ -472,30 +506,30 @@
code with your identity that was originally intended to run with the calling \
application's identity.
""",
- category = Category.SECURITY,
- priority = 6,
- severity = Severity.WARNING,
- implementation = Implementation(
- CallingIdentityTokenDetector::class.java,
- Scope.JAVA_FILE_SCOPE
- )
+ category = Category.SECURITY,
+ priority = 6,
+ severity = Severity.WARNING,
+ implementation = Implementation(
+ CallingIdentityTokenDetector::class.java,
+ Scope.JAVA_FILE_SCOPE
+ )
)
private fun getIncidentMessageClearIdentityCallNotFollowedByTryFinally(
variableName: String
): String = "You cleared the calling identity and returned the result into " +
- "`$variableName`, but the next statement is not a `try`-`finally` statement. " +
- "Define a `try`-`finally` block after `$variableName` declaration to ensure a " +
- "safe restore of the calling identity by calling " +
- "`Binder.restoreCallingIdentity($variableName)` and making it an immediate child " +
- "of the `finally` block."
+ "`$variableName`, but the next statement is not a `try`-`finally` statement. " +
+ "Define a `try`-`finally` block after `$variableName` declaration to ensure a " +
+ "safe restore of the calling identity by calling " +
+ "`Binder.restoreCallingIdentity($variableName)` and making it an immediate child " +
+ "of the `finally` block."
/** Issue: Binder.restoreCallingIdentity() is not in finally block */
@JvmField
val ISSUE_RESTORE_IDENTITY_CALL_NOT_IN_FINALLY_BLOCK: Issue = Issue.create(
- id = "RestoreIdentityCallNotInFinallyBlock",
- briefDescription = "Binder.restoreCallingIdentity() is not in finally block",
- explanation = """
+ id = "RestoreIdentityCallNotInFinallyBlock",
+ briefDescription = "Binder.restoreCallingIdentity() is not in finally block",
+ explanation = """
You are restoring the original calling identity with \
`Binder.restoreCallingIdentity()`, but the call is not an immediate child of \
the `finally` block of the `try` statement.
@@ -516,28 +550,28 @@
`finally` block, you may run code with your identity that was originally \
intended to run with the calling application's identity.
""",
- category = Category.SECURITY,
- priority = 6,
- severity = Severity.WARNING,
- implementation = Implementation(
- CallingIdentityTokenDetector::class.java,
- Scope.JAVA_FILE_SCOPE
- )
+ category = Category.SECURITY,
+ priority = 6,
+ severity = Severity.WARNING,
+ implementation = Implementation(
+ CallingIdentityTokenDetector::class.java,
+ Scope.JAVA_FILE_SCOPE
+ )
)
private fun getIncidentMessageRestoreIdentityCallNotInFinallyBlock(
variableName: String
): String = "`Binder.restoreCallingIdentity($variableName)` is not an immediate child of " +
- "the `finally` block of the try statement after `$variableName` declaration. " +
- "Surround the call with `finally` block and call it unconditionally."
+ "the `finally` block of the try statement after `$variableName` declaration. " +
+ "Surround the call with `finally` block and call it unconditionally."
/** Issue: Use of caller-aware methods after Binder.clearCallingIdentity() */
@JvmField
val ISSUE_USE_OF_CALLER_AWARE_METHODS_WITH_CLEARED_IDENTITY: Issue = Issue.create(
- id = "UseOfCallerAwareMethodsWithClearedIdentity",
- briefDescription = "Use of caller-aware methods after " +
- "Binder.clearCallingIdentity()",
- explanation = """
+ id = "UseOfCallerAwareMethodsWithClearedIdentity",
+ briefDescription = "Use of caller-aware methods after " +
+ "Binder.clearCallingIdentity()",
+ explanation = """
You cleared the original calling identity with \
`Binder.clearCallingIdentity()`, but used one of the methods below before \
restoring the identity. These methods will use your own identity instead of \
@@ -556,22 +590,59 @@
UserHandle.getCallingUserId()
```
""",
- category = Category.SECURITY,
- priority = 6,
- severity = Severity.WARNING,
- implementation = Implementation(
- CallingIdentityTokenDetector::class.java,
- Scope.JAVA_FILE_SCOPE
- )
+ category = Category.SECURITY,
+ priority = 6,
+ severity = Severity.WARNING,
+ implementation = Implementation(
+ CallingIdentityTokenDetector::class.java,
+ Scope.JAVA_FILE_SCOPE
+ )
)
private fun getIncidentMessageUseOfCallerAwareMethodsWithClearedIdentity(
variableName: String,
methodName: String
): String = "You cleared the original identity with `Binder.clearCallingIdentity()` " +
- "and returned into `$variableName`, so `$methodName` will be using your own " +
- "identity instead of the caller's. Either explicitly query your own identity or " +
- "move it after restoring the identity with " +
- "`Binder.restoreCallingIdentity($variableName)`."
+ "and returned into `$variableName`, so `$methodName` will be using your own " +
+ "identity instead of the caller's. Either explicitly query your own identity or " +
+ "move it after restoring the identity with " +
+ "`Binder.restoreCallingIdentity($variableName)`."
+
+ /** Issue: Result of Binder.clearCallingIdentity() is not stored in a variable */
+ @JvmField
+ val ISSUE_RESULT_OF_CLEAR_IDENTITY_CALL_NOT_STORED_IN_VARIABLE: Issue = Issue.create(
+ id = "ResultOfClearIdentityCallNotStoredInVariable",
+ briefDescription = "Result of Binder.clearCallingIdentity() is not stored in a " +
+ "variable",
+ explanation = """
+ You cleared the original calling identity with \
+ `Binder.clearCallingIdentity()`, but did not store the result of the method \
+ call in a variable. You need to store the result in a variable and restore it later.
+
+ Use the following pattern for running operations with your own identity:
+
+ ```
+ final long token = Binder.clearCallingIdentity();
+ try {
+ // Code using your own identity
+ } finally {
+ Binder.restoreCallingIdentity(token);
+ }
+ ```
+ """,
+ category = Category.SECURITY,
+ priority = 6,
+ severity = Severity.WARNING,
+ implementation = Implementation(
+ CallingIdentityTokenDetector::class.java,
+ Scope.JAVA_FILE_SCOPE
+ )
+ )
+
+ private fun getIncidentMessageResultOfClearIdentityCallNotStoredInVariable(
+ methodName: String
+ ): String = "You cleared the original identity with `$methodName` but did not store the " +
+ "result in a variable. You need to store the result in a variable and restore it " +
+ "later."
}
}
diff --git a/tools/lint/checks/src/main/java/com/google/android/lint/CallingSettingsNonUserGetterMethodsDetector.kt b/tools/lint/framework/checks/src/main/java/com/google/android/lint/CallingSettingsNonUserGetterMethodsDetector.kt
similarity index 100%
rename from tools/lint/checks/src/main/java/com/google/android/lint/CallingSettingsNonUserGetterMethodsDetector.kt
rename to tools/lint/framework/checks/src/main/java/com/google/android/lint/CallingSettingsNonUserGetterMethodsDetector.kt
diff --git a/tools/lint/framework/checks/src/main/java/com/google/android/lint/PackageVisibilityDetector.kt b/tools/lint/framework/checks/src/main/java/com/google/android/lint/PackageVisibilityDetector.kt
new file mode 100644
index 0000000..48540b1d
--- /dev/null
+++ b/tools/lint/framework/checks/src/main/java/com/google/android/lint/PackageVisibilityDetector.kt
@@ -0,0 +1,515 @@
+/*
+ * Copyright (C) 2022 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
+
+import com.android.tools.lint.client.api.UastParser
+import com.android.tools.lint.detector.api.Category
+import com.android.tools.lint.detector.api.Context
+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.Scope
+import com.android.tools.lint.detector.api.Severity
+import com.android.tools.lint.detector.api.SourceCodeScanner
+import com.android.tools.lint.detector.api.interprocedural.CallGraph
+import com.android.tools.lint.detector.api.interprocedural.CallGraphResult
+import com.android.tools.lint.detector.api.interprocedural.searchForPaths
+import com.intellij.psi.PsiAnonymousClass
+import com.intellij.psi.PsiMethod
+import java.util.LinkedList
+import org.jetbrains.uast.UCallExpression
+import org.jetbrains.uast.UElement
+import org.jetbrains.uast.UMethod
+import org.jetbrains.uast.UParameter
+import org.jetbrains.uast.USimpleNameReferenceExpression
+import org.jetbrains.uast.visitor.AbstractUastVisitor
+
+/**
+ * A lint checker to detect potential package visibility issues for system's APIs. APIs working
+ * in the system_server and taking the package name as a parameter may have chance to reveal
+ * package existence status on the device, and break the
+ * <a href="https://developer.android.com/about/versions/11/privacy/package-visibility">
+ * Package Visibility</a> that we introduced in Android 11.
+ * <p>
+ * Take an example of the API `boolean setFoo(String packageName)`, a malicious app may have chance
+ * to detect package existence state on the device from the result of the API, if there is no
+ * package visibility filtering rule or uid identify checks applying to the parameter of the
+ * package name.
+ */
+class PackageVisibilityDetector : Detector(), SourceCodeScanner {
+
+ // Enables call graph analysis
+ override fun isCallGraphRequired(): Boolean = true
+
+ override fun analyzeCallGraph(
+ context: Context,
+ callGraph: CallGraphResult
+ ) {
+ val systemServerApiNodes = callGraph.callGraph.nodes.filter(::isSystemServerApi)
+ val sinkMethodNodes = callGraph.callGraph.nodes.filter {
+ // TODO(b/228285232): Remove enforce permission sink methods
+ isNodeInList(it, ENFORCE_PERMISSION_METHODS) || isNodeInList(it, APPOPS_METHODS)
+ }
+ val parser = context.client.getUastParser(context.project)
+ analyzeApisContainPackageNameParameters(
+ context, parser, systemServerApiNodes, sinkMethodNodes)
+ }
+
+ /**
+ * Looking for API contains package name parameters, report the lint issue if the API does not
+ * invoke any sink methods.
+ */
+ private fun analyzeApisContainPackageNameParameters(
+ context: Context,
+ parser: UastParser,
+ systemServerApiNodes: List<CallGraph.Node>,
+ sinkMethodNodes: List<CallGraph.Node>
+ ) {
+ for (apiNode in systemServerApiNodes) {
+ val apiMethod = apiNode.getUMethod() ?: continue
+ val pkgNameParamIndexes = apiMethod.uastParameters.mapIndexedNotNull { index, param ->
+ if (Parameter(param) in PACKAGE_NAME_PATTERNS && apiNode.isArgumentInUse(index)) {
+ index
+ } else {
+ null
+ }
+ }.takeIf(List<Int>::isNotEmpty) ?: continue
+
+ for (pkgNameParamIndex in pkgNameParamIndexes) {
+ // Trace the call path of the method's argument, pass the lint checks if a sink
+ // method is found
+ if (traceArgumentCallPath(
+ apiNode, pkgNameParamIndex, PACKAGE_NAME_SINK_METHOD_LIST)) {
+ continue
+ }
+ // Pass the check if one of the sink methods is invoked
+ if (hasValidPath(
+ searchForPaths(
+ sources = listOf(apiNode),
+ isSink = { it in sinkMethodNodes },
+ getNeighbors = { node -> node.edges.map { it.node!! } }
+ )
+ )
+ ) continue
+
+ // Report issue
+ val reportElement = apiMethod.uastParameters[pkgNameParamIndex] as UElement
+ val location = parser.createLocation(reportElement)
+ context.report(
+ ISSUE_PACKAGE_NAME_NO_PACKAGE_VISIBILITY_FILTERS,
+ location,
+ getMsgPackageNameNoPackageVisibilityFilters(apiMethod, pkgNameParamIndex)
+ )
+ }
+ }
+ }
+
+ /**
+ * Returns {@code true} if the method associated with the given node is a system server's
+ * public API that extends from Stub class.
+ */
+ private fun isSystemServerApi(
+ node: CallGraph.Node
+ ): Boolean {
+ val method = node.getUMethod() ?: return false
+ if (!method.hasModifierProperty("public") ||
+ method.uastBody == null ||
+ method.containingClass is PsiAnonymousClass) {
+ return false
+ }
+ val className = method.containingClass?.qualifiedName ?: return false
+ if (!className.startsWith(SYSTEM_PACKAGE_PREFIX)) {
+ return false
+ }
+ return (method.containingClass ?: return false).supers
+ .filter { it.name == CLASS_STUB }
+ .filter { it.qualifiedName !in BYPASS_STUBS }
+ .any { it.findMethodBySignature(method, /* checkBases */ true) != null }
+ }
+
+ /**
+ * Returns {@code true} if the list contains the node of the call graph.
+ */
+ private fun isNodeInList(
+ node: CallGraph.Node,
+ filters: List<Method>
+ ): Boolean {
+ val method = node.getUMethod() ?: return false
+ return Method(method) in filters
+ }
+
+ /**
+ * Trace the call paths of the argument of the method in the start entry. Return {@code true}
+ * if one of methods in the sink call list is invoked.
+ * Take an example of the call path:
+ * foo(packageName) -> a(packageName) -> b(packageName) -> filterAppAccess()
+ * It returns {@code true} if the filterAppAccess() is in the sink call list.
+ */
+ private fun traceArgumentCallPath(
+ apiNode: CallGraph.Node,
+ pkgNameParamIndex: Int,
+ sinkList: List<Method>
+ ): Boolean {
+ val startEntry = TraceEntry(apiNode, pkgNameParamIndex)
+ val traceQueue = LinkedList<TraceEntry>().apply { add(startEntry) }
+ val allVisits = mutableSetOf<TraceEntry>().apply { add(startEntry) }
+ while (!traceQueue.isEmpty()) {
+ val entry = traceQueue.poll()
+ val entryNode = entry.node
+ val entryMethod = entryNode.getUMethod() ?: continue
+ val entryArgumentName = entryMethod.uastParameters[entry.argumentIndex].name
+ for (outEdge in entryNode.edges) {
+ val outNode = outEdge.node ?: continue
+ val outMethod = outNode.getUMethod() ?: continue
+ val outArgumentIndex =
+ outEdge.call?.findArgumentIndex(
+ entryArgumentName, outMethod.uastParameters.size)
+ val sinkMethod = findInSinkList(outMethod, sinkList)
+ if (sinkMethod == null) {
+ if (outArgumentIndex == null) {
+ // Path is not relevant to the sink method and argument
+ continue
+ }
+ // Path is relevant to the argument, add a new trace entry if never visit before
+ val newEntry = TraceEntry(outNode, outArgumentIndex)
+ if (newEntry !in allVisits) {
+ traceQueue.add(newEntry)
+ allVisits.add(newEntry)
+ }
+ continue
+ }
+ if (sinkMethod.matchArgument && outArgumentIndex == null) {
+ // The sink call is required to match the argument, but not found
+ continue
+ }
+ if (sinkMethod.checkCaller &&
+ entryMethod.isInClearCallingIdentityScope(outEdge.call!!)) {
+ // The sink call is in the scope of Binder.clearCallingIdentify
+ continue
+ }
+ // A sink method is matched
+ return true
+ }
+ }
+ return false
+ }
+
+ /**
+ * Returns the UMethod associated with the given node of call graph.
+ */
+ private fun CallGraph.Node.getUMethod(): UMethod? = this.target.element as? UMethod
+
+ /**
+ * Returns the system module name (e.g. com.android.server.pm) of the method of the
+ * call graph node.
+ */
+ private fun CallGraph.Node.getModuleName(): String? {
+ val method = getUMethod() ?: return null
+ val className = method.containingClass?.qualifiedName ?: return null
+ if (!className.startsWith(SYSTEM_PACKAGE_PREFIX)) {
+ return null
+ }
+ val dotPos = className.indexOf(".", SYSTEM_PACKAGE_PREFIX.length)
+ if (dotPos == -1) {
+ return SYSTEM_PACKAGE_PREFIX
+ }
+ return className.substring(0, dotPos)
+ }
+
+ /**
+ * Return {@code true} if the argument in the method's body is in-use.
+ */
+ private fun CallGraph.Node.isArgumentInUse(argIndex: Int): Boolean {
+ val method = getUMethod() ?: return false
+ val argumentName = method.uastParameters[argIndex].name
+ var foundArg = false
+ val methodVisitor = object : AbstractUastVisitor() {
+ override fun visitSimpleNameReferenceExpression(
+ node: USimpleNameReferenceExpression
+ ): Boolean {
+ if (node.identifier == argumentName) {
+ foundArg = true
+ }
+ return true
+ }
+ }
+ method.uastBody?.accept(methodVisitor)
+ return foundArg
+ }
+
+ /**
+ * Given an argument name, returns the index of argument in the call expression.
+ */
+ private fun UCallExpression.findArgumentIndex(
+ argumentName: String,
+ parameterSize: Int
+ ): Int? {
+ if (valueArgumentCount == 0 || parameterSize == 0) {
+ return null
+ }
+ var match = false
+ val argVisitor = object : AbstractUastVisitor() {
+ override fun visitSimpleNameReferenceExpression(
+ node: USimpleNameReferenceExpression
+ ): Boolean {
+ if (node.identifier == argumentName) {
+ match = true
+ }
+ return true
+ }
+ override fun visitCallExpression(node: UCallExpression): Boolean {
+ return true
+ }
+ }
+ valueArguments.take(parameterSize).forEachIndexed { index, argument ->
+ argument.accept(argVisitor)
+ if (match) {
+ return index
+ }
+ }
+ return null
+ }
+
+ /**
+ * Given a UMethod, returns a method from the sink method list.
+ */
+ private fun findInSinkList(
+ uMethod: UMethod,
+ sinkCallList: List<Method>
+ ): Method? {
+ return sinkCallList.find {
+ it == Method(uMethod) ||
+ it == Method(uMethod.containingClass?.qualifiedName ?: "", "*")
+ }
+ }
+
+ /**
+ * Returns {@code true} if the call expression is in the scope of the
+ * Binder.clearCallingIdentify.
+ */
+ private fun UMethod.isInClearCallingIdentityScope(call: UCallExpression): Boolean {
+ var isInScope = false
+ val methodVisitor = object : AbstractUastVisitor() {
+ private var clearCallingIdentity = 0
+ override fun visitCallExpression(node: UCallExpression): Boolean {
+ if (call == node && clearCallingIdentity != 0) {
+ isInScope = true
+ return true
+ }
+ val visitMethod = Method(node.resolve() ?: return false)
+ if (visitMethod == METHOD_CLEAR_CALLING_IDENTITY) {
+ clearCallingIdentity++
+ } else if (visitMethod == METHOD_RESTORE_CALLING_IDENTITY) {
+ clearCallingIdentity--
+ }
+ return false
+ }
+ }
+ accept(methodVisitor)
+ return isInScope
+ }
+
+ /**
+ * Checks the module name of the start node and the last node that invokes the sink method
+ * (e.g. checkPermission) in a path, returns {@code true} if one of the paths has the same
+ * module name for both nodes.
+ */
+ private fun hasValidPath(paths: Collection<List<CallGraph.Node>>): Boolean {
+ for (pathNodes in paths) {
+ if (pathNodes.size < VALID_CALL_PATH_NODES_SIZE) {
+ continue
+ }
+ val startModule = pathNodes[0].getModuleName() ?: continue
+ val lastCallModule = pathNodes[pathNodes.size - 2].getModuleName() ?: continue
+ if (startModule == lastCallModule) {
+ return true
+ }
+ }
+ return false
+ }
+
+ /**
+ * A data class to represent the method.
+ */
+ private data class Method(
+ val clazz: String,
+ val name: String
+ ) {
+ // Used by traceArgumentCallPath to indicate that the method is required to match the
+ // argument name
+ var matchArgument = true
+
+ // Used by traceArgumentCallPath to indicate that the method is required to check whether
+ // the Binder.clearCallingIdentity is invoked.
+ var checkCaller = false
+
+ constructor(
+ clazz: String,
+ name: String,
+ matchArgument: Boolean = true,
+ checkCaller: Boolean = false
+ ) : this(clazz, name) {
+ this.matchArgument = matchArgument
+ this.checkCaller = checkCaller
+ }
+
+ constructor(
+ method: PsiMethod
+ ) : this(method.containingClass?.qualifiedName ?: "", method.name)
+
+ constructor(
+ method: com.google.android.lint.model.Method
+ ) : this(method.clazz, method.name)
+ }
+
+ /**
+ * A data class to represent the parameter of the method. The parameter name is converted to
+ * lower case letters for comparison.
+ */
+ private data class Parameter private constructor(
+ val typeName: String,
+ val parameterName: String
+ ) {
+ constructor(uParameter: UParameter) : this(
+ uParameter.type.canonicalText,
+ uParameter.name.lowercase()
+ )
+
+ companion object {
+ fun create(typeName: String, parameterName: String) =
+ Parameter(typeName, parameterName.lowercase())
+ }
+ }
+
+ /**
+ * A data class wraps a method node of the call graph and an index that indicates an
+ * argument of the method to record call trace information.
+ */
+ private data class TraceEntry(
+ val node: CallGraph.Node,
+ val argumentIndex: Int
+ )
+
+ companion object {
+ private const val SYSTEM_PACKAGE_PREFIX = "com.android.server."
+ // A valid call path list needs to contain a start node and a sink node
+ private const val VALID_CALL_PATH_NODES_SIZE = 2
+
+ private const val CLASS_STRING = "java.lang.String"
+ private const val CLASS_PACKAGE_MANAGER = "android.content.pm.PackageManager"
+ private const val CLASS_IPACKAGE_MANAGER = "android.content.pm.IPackageManager"
+ private const val CLASS_APPOPS_MANAGER = "android.app.AppOpsManager"
+ private const val CLASS_BINDER = "android.os.Binder"
+ private const val CLASS_PACKAGE_MANAGER_INTERNAL =
+ "android.content.pm.PackageManagerInternal"
+
+ // Patterns of package name parameter
+ private val PACKAGE_NAME_PATTERNS = setOf(
+ Parameter.create(CLASS_STRING, "packageName"),
+ Parameter.create(CLASS_STRING, "callingPackage"),
+ Parameter.create(CLASS_STRING, "callingPackageName"),
+ Parameter.create(CLASS_STRING, "pkgName"),
+ Parameter.create(CLASS_STRING, "callingPkg"),
+ Parameter.create(CLASS_STRING, "pkg")
+ )
+
+ // Package manager APIs
+ private val PACKAGE_NAME_SINK_METHOD_LIST = listOf(
+ Method(CLASS_PACKAGE_MANAGER_INTERNAL, "filterAppAccess", matchArgument = false),
+ Method(CLASS_PACKAGE_MANAGER_INTERNAL, "canQueryPackage"),
+ Method(CLASS_PACKAGE_MANAGER_INTERNAL, "isSameApp"),
+ Method(CLASS_PACKAGE_MANAGER, "*", checkCaller = true),
+ Method(CLASS_IPACKAGE_MANAGER, "*", checkCaller = true),
+ Method(CLASS_PACKAGE_MANAGER, "getPackagesForUid", matchArgument = false),
+ Method(CLASS_IPACKAGE_MANAGER, "getPackagesForUid", matchArgument = false)
+ )
+
+ // AppOps APIs which include uid and package visibility filters checks
+ private val APPOPS_METHODS = listOf(
+ Method(CLASS_APPOPS_MANAGER, "noteOp"),
+ Method(CLASS_APPOPS_MANAGER, "noteOpNoThrow"),
+ Method(CLASS_APPOPS_MANAGER, "noteOperation"),
+ Method(CLASS_APPOPS_MANAGER, "noteProxyOp"),
+ Method(CLASS_APPOPS_MANAGER, "noteProxyOpNoThrow"),
+ Method(CLASS_APPOPS_MANAGER, "startOp"),
+ Method(CLASS_APPOPS_MANAGER, "startOpNoThrow"),
+ Method(CLASS_APPOPS_MANAGER, "FinishOp"),
+ Method(CLASS_APPOPS_MANAGER, "finishProxyOp"),
+ Method(CLASS_APPOPS_MANAGER, "checkPackage")
+ )
+
+ // Enforce permission APIs
+ private val ENFORCE_PERMISSION_METHODS =
+ com.google.android.lint.ENFORCE_PERMISSION_METHODS
+ .map(PackageVisibilityDetector::Method)
+
+ private val BYPASS_STUBS = listOf(
+ "android.content.pm.IPackageDataObserver.Stub",
+ "android.content.pm.IPackageDeleteObserver.Stub",
+ "android.content.pm.IPackageDeleteObserver2.Stub",
+ "android.content.pm.IPackageInstallObserver2.Stub",
+ "com.android.internal.app.IAppOpsCallback.Stub",
+
+ // TODO(b/228285637): Do not bypass PackageManagerService API
+ "android.content.pm.IPackageManager.Stub",
+ "android.content.pm.IPackageManagerNative.Stub"
+ )
+
+ private val METHOD_CLEAR_CALLING_IDENTITY =
+ Method(CLASS_BINDER, "clearCallingIdentity")
+ private val METHOD_RESTORE_CALLING_IDENTITY =
+ Method(CLASS_BINDER, "restoreCallingIdentity")
+
+ private fun getMsgPackageNameNoPackageVisibilityFilters(
+ method: UMethod,
+ argumentIndex: Int
+ ): String = "Api: ${method.name} contains a package name parameter: " +
+ "${method.uastParameters[argumentIndex].name} does not apply " +
+ "package visibility filtering rules."
+
+ private val EXPLANATION = """
+ APIs working in the system_server and taking the package name as a parameter may have
+ chance to reveal package existence status on the device, and break the package
+ visibility that we introduced in Android 11.
+ (https://developer.android.com/about/versions/11/privacy/package-visibility)
+
+ Take an example of the API `boolean setFoo(String packageName)`, a malicious app may
+ have chance to get package existence state on the device from the result of the API,
+ if there is no package visibility filtering rule or uid identify checks applying to
+ the parameter of the package name.
+
+ To resolve it, you could apply package visibility filtering rules to the package name
+ via PackageManagerInternal.filterAppAccess API, before starting to use the package name.
+ If the parameter is a calling package name, use the PackageManager API such as
+ PackageManager.getPackagesForUid to verify the calling identify.
+ """
+
+ val ISSUE_PACKAGE_NAME_NO_PACKAGE_VISIBILITY_FILTERS = Issue.create(
+ id = "ApiMightLeakAppVisibility",
+ briefDescription = "Api takes package name parameter doesn't apply " +
+ "package visibility filters",
+ explanation = EXPLANATION,
+ category = Category.SECURITY,
+ priority = 1,
+ severity = Severity.WARNING,
+ implementation = Implementation(
+ PackageVisibilityDetector::class.java,
+ Scope.JAVA_FILE_SCOPE
+ )
+ )
+ }
+}
diff --git a/tools/lint/framework/checks/src/main/java/com/google/android/lint/PermissionMethodDetector.kt b/tools/lint/framework/checks/src/main/java/com/google/android/lint/PermissionMethodDetector.kt
new file mode 100644
index 0000000..e12ec3d
--- /dev/null
+++ b/tools/lint/framework/checks/src/main/java/com/google/android/lint/PermissionMethodDetector.kt
@@ -0,0 +1,199 @@
+/*
+ * Copyright (C) 2022 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
+
+import com.android.tools.lint.client.api.UElementHandler
+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.android.tools.lint.detector.api.getUMethod
+import com.intellij.psi.PsiType
+import org.jetbrains.uast.UAnnotation
+import org.jetbrains.uast.UBlockExpression
+import org.jetbrains.uast.UCallExpression
+import org.jetbrains.uast.UElement
+import org.jetbrains.uast.UExpression
+import org.jetbrains.uast.UIfExpression
+import org.jetbrains.uast.UMethod
+import org.jetbrains.uast.UQualifiedReferenceExpression
+import org.jetbrains.uast.UReturnExpression
+import org.jetbrains.uast.getContainingUMethod
+
+/**
+ * Stops incorrect usage of {@link PermissionMethod}
+ * TODO: add tests once re-enabled (b/240445172, b/247542171)
+ */
+class PermissionMethodDetector : Detector(), SourceCodeScanner {
+
+ override fun getApplicableUastTypes(): List<Class<out UElement>> =
+ listOf(UAnnotation::class.java, UMethod::class.java)
+
+ override fun createUastHandler(context: JavaContext): UElementHandler =
+ PermissionMethodHandler(context)
+
+ private inner class PermissionMethodHandler(val context: JavaContext) : UElementHandler() {
+ override fun visitMethod(node: UMethod) {
+ if (hasPermissionMethodAnnotation(node)) return
+ if (onlyCallsPermissionMethod(node)) {
+ val location = context.getLocation(node.javaPsi.modifierList)
+ val fix = fix()
+ .annotate(ANNOTATION_PERMISSION_METHOD)
+ .range(location)
+ .autoFix()
+ .build()
+
+ context.report(
+ ISSUE_CAN_BE_PERMISSION_METHOD,
+ location,
+ "Annotate method with @PermissionMethod",
+ fix
+ )
+ }
+ }
+
+ override fun visitAnnotation(node: UAnnotation) {
+ if (node.qualifiedName != ANNOTATION_PERMISSION_METHOD) return
+ val method = node.getContainingUMethod() ?: return
+
+ if (!isPermissionMethodReturnType(method)) {
+ context.report(
+ ISSUE_PERMISSION_METHOD_USAGE,
+ context.getLocation(node),
+ """
+ Methods annotated with `@PermissionMethod` should return `void`, \
+ `boolean`, or `@PackageManager.PermissionResult int`."
+ """.trimIndent()
+ )
+ }
+
+ if (method.returnType == PsiType.INT &&
+ method.annotations.none { it.hasQualifiedName(ANNOTATION_PERMISSION_RESULT) }
+ ) {
+ context.report(
+ ISSUE_PERMISSION_METHOD_USAGE,
+ context.getLocation(node),
+ """
+ Methods annotated with `@PermissionMethod` that return `int` should \
+ also be annotated with `@PackageManager.PermissionResult.`"
+ """.trimIndent()
+ )
+ }
+ }
+ }
+
+ companion object {
+
+ private val EXPLANATION_PERMISSION_METHOD_USAGE = """
+ `@PermissionMethod` should annotate methods that ONLY perform permission lookups. \
+ Said methods should return `boolean`, `@PackageManager.PermissionResult int`, or return \
+ `void` and potentially throw `SecurityException`.
+ """.trimIndent()
+
+ @JvmField
+ val ISSUE_PERMISSION_METHOD_USAGE = Issue.create(
+ id = "PermissionMethodUsage",
+ briefDescription = "@PermissionMethod used incorrectly",
+ explanation = EXPLANATION_PERMISSION_METHOD_USAGE,
+ category = Category.CORRECTNESS,
+ priority = 5,
+ severity = Severity.ERROR,
+ implementation = Implementation(
+ PermissionMethodDetector::class.java,
+ Scope.JAVA_FILE_SCOPE
+ ),
+ enabledByDefault = true
+ )
+
+ private val EXPLANATION_CAN_BE_PERMISSION_METHOD = """
+ Methods that only call other methods annotated with @PermissionMethod (and do NOTHING else) can themselves \
+ be annotated with @PermissionMethod. For example:
+ ```
+ void wrapperHelper() {
+ // Context.enforceCallingPermission is annotated with @PermissionMethod
+ context.enforceCallingPermission(SOME_PERMISSION)
+ }
+ ```
+ """.trimIndent()
+
+ @JvmField
+ val ISSUE_CAN_BE_PERMISSION_METHOD = Issue.create(
+ id = "CanBePermissionMethod",
+ briefDescription = "Method can be annotated with @PermissionMethod",
+ explanation = EXPLANATION_CAN_BE_PERMISSION_METHOD,
+ category = Category.SECURITY,
+ priority = 5,
+ severity = Severity.WARNING,
+ implementation = Implementation(
+ PermissionMethodDetector::class.java,
+ Scope.JAVA_FILE_SCOPE
+ ),
+ enabledByDefault = false
+ )
+
+ private fun isPermissionMethodReturnType(method: UMethod): Boolean =
+ listOf(PsiType.VOID, PsiType.INT, PsiType.BOOLEAN).contains(method.returnType)
+
+ /**
+ * Identifies methods that...
+ * DO call other methods annotated with @PermissionMethod
+ * DO NOT do anything else
+ */
+ private fun onlyCallsPermissionMethod(method: UMethod): Boolean {
+ val body = method.uastBody as? UBlockExpression ?: return false
+ if (body.expressions.isEmpty()) return false
+ for (expression in body.expressions) {
+ when (expression) {
+ is UQualifiedReferenceExpression -> {
+ if (!isPermissionMethodCall(expression.selector)) return false
+ }
+ is UReturnExpression -> {
+ if (!isPermissionMethodCall(expression.returnExpression)) return false
+ }
+ is UCallExpression -> {
+ if (!isPermissionMethodCall(expression)) return false
+ }
+ is UIfExpression -> {
+ if (expression.thenExpression !is UReturnExpression) return false
+ if (!isPermissionMethodCall(expression.condition)) return false
+ }
+ else -> return false
+ }
+ }
+ return true
+ }
+
+ private fun isPermissionMethodCall(expression: UExpression?): Boolean {
+ return when (expression) {
+ is UQualifiedReferenceExpression ->
+ return isPermissionMethodCall(expression.selector)
+ is UCallExpression -> {
+ val calledMethod = expression.resolve()?.getUMethod() ?: return false
+ return hasPermissionMethodAnnotation(calledMethod)
+ }
+ else -> false
+ }
+ }
+
+ private fun hasPermissionMethodAnnotation(method: UMethod): Boolean = method.annotations
+ .any { it.hasQualifiedName(ANNOTATION_PERMISSION_METHOD) }
+ }
+}
diff --git a/tools/lint/framework/checks/src/main/java/com/google/android/lint/parcel/CallMigrators.kt b/tools/lint/framework/checks/src/main/java/com/google/android/lint/parcel/CallMigrators.kt
new file mode 100644
index 0000000..06c098d
--- /dev/null
+++ b/tools/lint/framework/checks/src/main/java/com/google/android/lint/parcel/CallMigrators.kt
@@ -0,0 +1,229 @@
+/*
+ * Copyright (C) 2022 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.parcel
+
+import com.android.tools.lint.detector.api.JavaContext
+import com.android.tools.lint.detector.api.LintFix
+import com.android.tools.lint.detector.api.Location
+import com.intellij.psi.PsiArrayType
+import com.intellij.psi.PsiCallExpression
+import com.intellij.psi.PsiClassType
+import com.intellij.psi.PsiIntersectionType
+import com.intellij.psi.PsiMethod
+import com.intellij.psi.PsiType
+import com.intellij.psi.PsiTypeParameter
+import com.intellij.psi.PsiWildcardType
+import org.jetbrains.uast.UCallExpression
+import org.jetbrains.uast.UElement
+import org.jetbrains.uast.UExpression
+import org.jetbrains.uast.UVariable
+
+/**
+ * Subclass this class and override {@link #getBoundingClass} to report an unsafe Parcel API issue
+ * with a fix that migrates towards the new safer API by appending an argument in the form of
+ * {@code com.package.ItemType.class} coming from the result of the overridden method.
+ */
+abstract class CallMigrator(
+ val method: Method,
+ private val rejects: Set<String> = emptySet(),
+) {
+ open fun report(context: JavaContext, call: UCallExpression, method: PsiMethod) {
+ val location = context.getLocation(call)
+ val itemType = filter(getBoundingClass(context, call, method))
+ val fix = (itemType as? PsiClassType)?.let { type ->
+ getParcelFix(location, this.method.name, getArgumentSuffix(type))
+ }
+ val message = "Unsafe `${this.method.className}.${this.method.name}()` API usage"
+ context.report(SaferParcelChecker.ISSUE_UNSAFE_API_USAGE, call, location, message, fix)
+ }
+
+ protected open fun getArgumentSuffix(type: PsiClassType) =
+ ", ${type.rawType().canonicalText}.class"
+
+ protected open fun getBoundingClass(
+ context: JavaContext,
+ call: UCallExpression,
+ method: PsiMethod,
+ ): PsiType? = null
+
+ protected fun getItemType(type: PsiType, container: String): PsiClassType? {
+ val supers = getParentTypes(type).mapNotNull { it as? PsiClassType }
+ val containerType = supers.firstOrNull { it.rawType().canonicalText == container }
+ ?: return null
+ val itemType = containerType.parameters.getOrNull(0) ?: return null
+ // TODO: Expand to other types, see PsiTypeVisitor
+ return when (itemType) {
+ is PsiClassType -> itemType
+ is PsiWildcardType -> itemType.bound as PsiClassType
+ else -> null
+ }
+ }
+
+ /**
+ * Tries to obtain the type expected by the "receiving" end given a certain {@link UElement}.
+ *
+ * This could be an assignment, an argument passed to a method call, to a constructor call, a
+ * type cast, etc. If no receiving end is found, the type of the UExpression itself is returned.
+ */
+ protected fun getReceivingType(expression: UElement): PsiType? {
+ val parent = expression.uastParent
+ var type = when (parent) {
+ is UCallExpression -> {
+ val i = parent.valueArguments.indexOf(expression)
+ val psiCall = parent.sourcePsi as? PsiCallExpression ?: return null
+ val typeSubstitutor = psiCall.resolveMethodGenerics().substitutor
+ val method = psiCall.resolveMethod()!!
+ method.getSignature(typeSubstitutor).parameterTypes[i]
+ }
+ is UVariable -> parent.type
+ is UExpression -> parent.getExpressionType()
+ else -> null
+ }
+ if (type == null && expression is UExpression) {
+ type = expression.getExpressionType()
+ }
+ return type
+ }
+
+ protected fun filter(type: PsiType?): PsiType? {
+ // It's important that PsiIntersectionType case is above the one that check the type in
+ // rejects, because for intersect types, the canonicalText is one of the terms.
+ if (type is PsiIntersectionType) {
+ return type.conjuncts.mapNotNull(this::filter).firstOrNull()
+ }
+ if (type == null || type.canonicalText in rejects) {
+ return null
+ }
+ if (type is PsiClassType && type.resolve() is PsiTypeParameter) {
+ return null
+ }
+ return type
+ }
+
+ private fun getParentTypes(type: PsiType): Set<PsiType> =
+ type.superTypes.flatMap(::getParentTypes).toSet() + type
+
+ protected fun getParcelFix(location: Location, method: String, arguments: String) =
+ LintFix
+ .create()
+ .name("Migrate to safer Parcel.$method() API")
+ .replace()
+ .range(location)
+ .pattern("$method\\s*\\(((?:.|\\n)*)\\)")
+ .with("\\k<1>$arguments")
+ .autoFix()
+ .build()
+}
+
+/**
+ * This class derives the type to be appended by inferring the generic type of the {@code container}
+ * type (eg. "java.util.List") of the {@code argument}-th argument.
+ */
+class ContainerArgumentMigrator(
+ method: Method,
+ private val argument: Int,
+ private val container: String,
+ rejects: Set<String> = emptySet(),
+) : CallMigrator(method, rejects) {
+ override fun getBoundingClass(
+ context: JavaContext, call: UCallExpression, method: PsiMethod
+ ): PsiType? {
+ val firstParamType = call.valueArguments[argument].getExpressionType() ?: return null
+ return getItemType(firstParamType, container)!!
+ }
+
+ /**
+ * We need to insert a casting construct in the class parameter. For example:
+ * (Class<Foo<Bar>>) (Class<?>) Foo.class.
+ * This is needed for when the arguments of the conflict (eg. when there is List<Foo<Bar>> and
+ * class type is Class<Foo?).
+ */
+ override fun getArgumentSuffix(type: PsiClassType): String {
+ if (type.parameters.isNotEmpty()) {
+ val rawType = type.rawType()
+ return ", (Class<${type.canonicalText}>) (Class<?>) ${rawType.canonicalText}.class"
+ }
+ return super.getArgumentSuffix(type)
+ }
+}
+
+/**
+ * This class derives the type to be appended by inferring the generic type of the {@code container}
+ * type (eg. "java.util.List") of the return type of the method.
+ */
+class ContainerReturnMigrator(
+ method: Method,
+ private val container: String,
+ rejects: Set<String> = emptySet(),
+) : CallMigrator(method, rejects) {
+ override fun getBoundingClass(
+ context: JavaContext, call: UCallExpression, method: PsiMethod
+ ): PsiType? {
+ val type = getReceivingType(call.uastParent!!) ?: return null
+ return getItemType(type, container)
+ }
+}
+
+/**
+ * This class derives the type to be appended by inferring the expected type for the method result.
+ */
+class ReturnMigrator(
+ method: Method,
+ rejects: Set<String> = emptySet(),
+) : CallMigrator(method, rejects) {
+ override fun getBoundingClass(
+ context: JavaContext, call: UCallExpression, method: PsiMethod
+ ): PsiType? {
+ return getReceivingType(call.uastParent!!)
+ }
+}
+
+/**
+ * This class appends the class loader and the class object by deriving the type from the method
+ * result.
+ */
+class ReturnMigratorWithClassLoader(
+ method: Method,
+ rejects: Set<String> = emptySet(),
+) : CallMigrator(method, rejects) {
+ override fun getBoundingClass(
+ context: JavaContext, call: UCallExpression, method: PsiMethod
+ ): PsiType? {
+ return getReceivingType(call.uastParent!!)
+ }
+
+ override fun getArgumentSuffix(type: PsiClassType): String =
+ "${type.rawType().canonicalText}.class.getClassLoader(), " +
+ "${type.rawType().canonicalText}.class"
+
+}
+
+/**
+ * This class derives the type to be appended by inferring the expected array type
+ * for the method result.
+ */
+class ArrayReturnMigrator(
+ method: Method,
+ rejects: Set<String> = emptySet(),
+) : CallMigrator(method, rejects) {
+ override fun getBoundingClass(
+ context: JavaContext, call: UCallExpression, method: PsiMethod
+ ): PsiType? {
+ val type = getReceivingType(call.uastParent!!)
+ return (type as? PsiArrayType)?.componentType
+ }
+}
diff --git a/tools/lint/framework/checks/src/main/java/com/google/android/lint/parcel/Method.kt b/tools/lint/framework/checks/src/main/java/com/google/android/lint/parcel/Method.kt
new file mode 100644
index 0000000..0826e8e
--- /dev/null
+++ b/tools/lint/framework/checks/src/main/java/com/google/android/lint/parcel/Method.kt
@@ -0,0 +1,42 @@
+/*
+ * Copyright (C) 2022 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.parcel
+
+data class Method(
+ val params: List<String>,
+ val clazz: String,
+ val name: String,
+ val parameters: List<String>
+) {
+ constructor(
+ clazz: String,
+ name: String,
+ parameters: List<String>
+ ) : this(
+ listOf(), clazz, name, parameters
+ )
+
+ val signature: String
+ get() {
+ val prefix = if (params.isEmpty()) "" else "${params.joinToString(", ", "<", ">")} "
+ return "$prefix$clazz.$name(${parameters.joinToString()})"
+ }
+
+ val className: String by lazy {
+ clazz.split(".").last()
+ }
+}
diff --git a/tools/lint/framework/checks/src/main/java/com/google/android/lint/parcel/SaferParcelChecker.kt b/tools/lint/framework/checks/src/main/java/com/google/android/lint/parcel/SaferParcelChecker.kt
new file mode 100644
index 0000000..f928263
--- /dev/null
+++ b/tools/lint/framework/checks/src/main/java/com/google/android/lint/parcel/SaferParcelChecker.kt
@@ -0,0 +1,126 @@
+/*
+ * Copyright (C) 2022 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.parcel
+
+import com.android.tools.lint.detector.api.*
+import com.intellij.psi.PsiMethod
+import com.intellij.psi.PsiSubstitutor
+import com.intellij.psi.PsiType
+import com.intellij.psi.PsiTypeParameter
+import org.jetbrains.uast.UCallExpression
+import java.util.*
+
+@Suppress("UnstableApiUsage")
+class SaferParcelChecker : Detector(), SourceCodeScanner {
+ override fun getApplicableMethodNames(): List<String> =
+ MIGRATORS
+ .map(CallMigrator::method)
+ .map(Method::name)
+
+ override fun visitMethodCall(context: JavaContext, node: UCallExpression, method: PsiMethod) {
+ if (!isAtLeastT(context)) return
+ val signature = getSignature(method)
+ val migrator = MIGRATORS.firstOrNull { it.method.signature == signature } ?: return
+ migrator.report(context, node, method)
+ }
+
+ private fun getSignature(method: PsiMethod): String {
+ val name = UastLintUtils.getQualifiedName(method)
+ val signature = method.getSignature(PsiSubstitutor.EMPTY)
+ val parameters =
+ signature.parameterTypes.joinToString(transform = PsiType::getCanonicalText)
+ val types = signature.typeParameters.map(PsiTypeParameter::getName)
+ val prefix = if (types.isEmpty()) "" else types.joinToString(", ", "<", ">") + " "
+ return "$prefix$name($parameters)"
+ }
+
+ private fun isAtLeastT(context: Context): Boolean {
+ val project = if (context.isGlobalAnalysis()) context.mainProject else context.project
+ return project.isAndroidProject && project.minSdkVersion.featureLevel >= 33
+ }
+
+ companion object {
+ @JvmField
+ val ISSUE_UNSAFE_API_USAGE: Issue = Issue.create(
+ id = "UnsafeParcelApi",
+ briefDescription = "Use of unsafe deserialization API",
+ explanation = """
+ You are using a deprecated deserialization API that doesn't accept the expected class as\
+ a parameter. This means that unexpected classes could be instantiated and\
+ unexpected code executed.
+
+ Please migrate to the safer alternative that takes an extra Class<T> parameter.
+ """,
+ category = Category.SECURITY,
+ priority = 8,
+ severity = Severity.WARNING,
+
+ implementation = Implementation(
+ SaferParcelChecker::class.java,
+ Scope.JAVA_FILE_SCOPE
+ )
+ )
+
+ // Parcel
+ private val PARCEL_METHOD_READ_SERIALIZABLE = Method("android.os.Parcel", "readSerializable", listOf())
+ private val PARCEL_METHOD_READ_ARRAY_LIST = Method("android.os.Parcel", "readArrayList", listOf("java.lang.ClassLoader"))
+ private val PARCEL_METHOD_READ_LIST = Method("android.os.Parcel", "readList", listOf("java.util.List", "java.lang.ClassLoader"))
+ private val PARCEL_METHOD_READ_PARCELABLE = Method(listOf("T"), "android.os.Parcel", "readParcelable", listOf("java.lang.ClassLoader"))
+ private val PARCEL_METHOD_READ_PARCELABLE_LIST = Method(listOf("T"), "android.os.Parcel", "readParcelableList", listOf("java.util.List<T>", "java.lang.ClassLoader"))
+ private val PARCEL_METHOD_READ_SPARSE_ARRAY = Method(listOf("T"), "android.os.Parcel", "readSparseArray", listOf("java.lang.ClassLoader"))
+ private val PARCEL_METHOD_READ_ARRAY = Method("android.os.Parcel", "readArray", listOf("java.lang.ClassLoader"))
+ private val PARCEL_METHOD_READ_PARCELABLE_ARRAY = Method("android.os.Parcel", "readParcelableArray", listOf("java.lang.ClassLoader"))
+
+ // Bundle
+ private val BUNDLE_METHOD_GET_SERIALIZABLE = Method("android.os.Bundle", "getSerializable", listOf("java.lang.String"))
+ private val BUNDLE_METHOD_GET_PARCELABLE = Method(listOf("T"), "android.os.Bundle", "getParcelable", listOf("java.lang.String"))
+ private val BUNDLE_METHOD_GET_PARCELABLE_ARRAY_LIST = Method(listOf("T"), "android.os.Bundle", "getParcelableArrayList", listOf("java.lang.String"))
+ private val BUNDLE_METHOD_GET_PARCELABLE_ARRAY = Method("android.os.Bundle", "getParcelableArray", listOf("java.lang.String"))
+ private val BUNDLE_METHOD_GET_SPARSE_PARCELABLE_ARRAY = Method(listOf("T"), "android.os.Bundle", "getSparseParcelableArray", listOf("java.lang.String"))
+
+ // Intent
+ private val INTENT_METHOD_GET_SERIALIZABLE_EXTRA = Method("android.content.Intent", "getSerializableExtra", listOf("java.lang.String"))
+ private val INTENT_METHOD_GET_PARCELABLE_EXTRA = Method(listOf("T"), "android.content.Intent", "getParcelableExtra", listOf("java.lang.String"))
+ private val INTENT_METHOD_GET_PARCELABLE_ARRAY_EXTRA = Method("android.content.Intent", "getParcelableArrayExtra", listOf("java.lang.String"))
+ private val INTENT_METHOD_GET_PARCELABLE_ARRAY_LIST_EXTRA = Method(listOf("T"), "android.content.Intent", "getParcelableArrayListExtra", listOf("java.lang.String"))
+
+ // TODO: Write migrators for methods below
+ private val PARCEL_METHOD_READ_PARCELABLE_CREATOR = Method("android.os.Parcel", "readParcelableCreator", listOf("java.lang.ClassLoader"))
+
+ private val MIGRATORS = listOf(
+ ReturnMigrator(PARCEL_METHOD_READ_PARCELABLE, setOf("android.os.Parcelable")),
+ ContainerArgumentMigrator(PARCEL_METHOD_READ_LIST, 0, "java.util.List"),
+ ContainerReturnMigrator(PARCEL_METHOD_READ_ARRAY_LIST, "java.util.Collection"),
+ ContainerReturnMigrator(PARCEL_METHOD_READ_SPARSE_ARRAY, "android.util.SparseArray"),
+ ContainerArgumentMigrator(PARCEL_METHOD_READ_PARCELABLE_LIST, 0, "java.util.List"),
+ ReturnMigratorWithClassLoader(PARCEL_METHOD_READ_SERIALIZABLE),
+ ArrayReturnMigrator(PARCEL_METHOD_READ_ARRAY, setOf("java.lang.Object")),
+ ArrayReturnMigrator(PARCEL_METHOD_READ_PARCELABLE_ARRAY, setOf("android.os.Parcelable")),
+
+ ReturnMigrator(BUNDLE_METHOD_GET_PARCELABLE, setOf("android.os.Parcelable")),
+ ContainerReturnMigrator(BUNDLE_METHOD_GET_PARCELABLE_ARRAY_LIST, "java.util.Collection", setOf("android.os.Parcelable")),
+ ArrayReturnMigrator(BUNDLE_METHOD_GET_PARCELABLE_ARRAY, setOf("android.os.Parcelable")),
+ ContainerReturnMigrator(BUNDLE_METHOD_GET_SPARSE_PARCELABLE_ARRAY, "android.util.SparseArray", setOf("android.os.Parcelable")),
+ ReturnMigrator(BUNDLE_METHOD_GET_SERIALIZABLE, setOf("java.io.Serializable")),
+
+ ReturnMigrator(INTENT_METHOD_GET_PARCELABLE_EXTRA, setOf("android.os.Parcelable")),
+ ContainerReturnMigrator(INTENT_METHOD_GET_PARCELABLE_ARRAY_LIST_EXTRA, "java.util.Collection", setOf("android.os.Parcelable")),
+ ArrayReturnMigrator(INTENT_METHOD_GET_PARCELABLE_ARRAY_EXTRA, setOf("android.os.Parcelable")),
+ ReturnMigrator(INTENT_METHOD_GET_SERIALIZABLE_EXTRA, setOf("java.io.Serializable")),
+ )
+ }
+}
diff --git a/tools/lint/checks/src/test/java/com/google/android/lint/CallingIdentityTokenDetectorTest.kt b/tools/lint/framework/checks/src/test/java/com/google/android/lint/CallingIdentityTokenDetectorTest.kt
similarity index 88%
rename from tools/lint/checks/src/test/java/com/google/android/lint/CallingIdentityTokenDetectorTest.kt
rename to tools/lint/framework/checks/src/test/java/com/google/android/lint/CallingIdentityTokenDetectorTest.kt
index e1a5c61..d90f3e3 100644
--- a/tools/lint/checks/src/test/java/com/google/android/lint/CallingIdentityTokenDetectorTest.kt
+++ b/tools/lint/framework/checks/src/test/java/com/google/android/lint/CallingIdentityTokenDetectorTest.kt
@@ -27,12 +27,13 @@
override fun getDetector(): Detector = CallingIdentityTokenDetector()
override fun getIssues(): List<Issue> = listOf(
- CallingIdentityTokenDetector.ISSUE_UNUSED_TOKEN,
- CallingIdentityTokenDetector.ISSUE_NON_FINAL_TOKEN,
- CallingIdentityTokenDetector.ISSUE_NESTED_CLEAR_IDENTITY_CALLS,
- CallingIdentityTokenDetector.ISSUE_RESTORE_IDENTITY_CALL_NOT_IN_FINALLY_BLOCK,
- CallingIdentityTokenDetector.ISSUE_USE_OF_CALLER_AWARE_METHODS_WITH_CLEARED_IDENTITY,
- CallingIdentityTokenDetector.ISSUE_CLEAR_IDENTITY_CALL_NOT_FOLLOWED_BY_TRY_FINALLY
+ CallingIdentityTokenDetector.ISSUE_UNUSED_TOKEN,
+ CallingIdentityTokenDetector.ISSUE_NON_FINAL_TOKEN,
+ CallingIdentityTokenDetector.ISSUE_NESTED_CLEAR_IDENTITY_CALLS,
+ CallingIdentityTokenDetector.ISSUE_RESTORE_IDENTITY_CALL_NOT_IN_FINALLY_BLOCK,
+ CallingIdentityTokenDetector.ISSUE_USE_OF_CALLER_AWARE_METHODS_WITH_CLEARED_IDENTITY,
+ CallingIdentityTokenDetector.ISSUE_CLEAR_IDENTITY_CALL_NOT_FOLLOWED_BY_TRY_FINALLY,
+ CallingIdentityTokenDetector.ISSUE_RESULT_OF_CLEAR_IDENTITY_CALL_NOT_STORED_IN_VARIABLE
)
override fun lint(): TestLintTask = super.lint().allowMissingSdk(true)
@@ -41,8 +42,8 @@
fun testDoesNotDetectIssuesInCorrectScenario() {
lint().files(
- java(
- """
+ java(
+ """
package test.pkg;
import android.os.Binder;
public class TestClass1 extends Binder {
@@ -62,22 +63,29 @@
} finally {
restoreCallingIdentity(token3);
}
+ final Long token4 = true ? Binder.clearCallingIdentity() : null;
+ try {
+ } finally {
+ if (token4 != null) {
+ restoreCallingIdentity(token4);
+ }
+ }
}
}
"""
- ).indented(),
- *stubs
+ ).indented(),
+ *stubs
)
- .run()
- .expectClean()
+ .run()
+ .expectClean()
}
/** Unused token issue tests */
fun testDetectsUnusedTokens() {
lint().files(
- java(
- """
+ java(
+ """
package test.pkg;
import android.os.Binder;
public class TestClass1 extends Binder {
@@ -101,12 +109,12 @@
}
}
"""
- ).indented(),
- *stubs
+ ).indented(),
+ *stubs
)
- .run()
- .expect(
- """
+ .run()
+ .expect(
+ """
src/test/pkg/TestClass1.java:5: Warning: token1 has not been used to \
restore the calling identity. Introduce a try-finally after the \
declaration and call Binder.restoreCallingIdentity(token1) in finally or \
@@ -127,13 +135,13 @@
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
0 errors, 3 warnings
""".addLineContinuation()
- )
+ )
}
fun testDetectsUnusedTokensInScopes() {
lint().files(
- java(
- """
+ java(
+ """
package test.pkg;
import android.os.Binder;
public class TestClass1 {
@@ -152,12 +160,12 @@
}
}
"""
- ).indented(),
- *stubs
+ ).indented(),
+ *stubs
)
- .run()
- .expect(
- """
+ .run()
+ .expect(
+ """
src/test/pkg/TestClass1.java:5: Warning: token has not been used to \
restore the calling identity. Introduce a try-finally after the \
declaration and call Binder.restoreCallingIdentity(token) in finally or \
@@ -166,13 +174,13 @@
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
0 errors, 1 warnings
""".addLineContinuation()
- )
+ )
}
fun testDoesNotDetectUsedTokensInScopes() {
lint().files(
- java(
- """
+ java(
+ """
package test.pkg;
import android.os.Binder;
public class TestClass1 {
@@ -192,17 +200,17 @@
}
}
"""
- ).indented(),
- *stubs
+ ).indented(),
+ *stubs
)
- .run()
- .expectClean()
+ .run()
+ .expectClean()
}
fun testDetectsUnusedTokensWithSimilarNamesInScopes() {
lint().files(
- java(
- """
+ java(
+ """
package test.pkg;
import android.os.Binder;
public class TestClass1 {
@@ -220,12 +228,12 @@
}
}
"""
- ).indented(),
- *stubs
+ ).indented(),
+ *stubs
)
- .run()
- .expect(
- """
+ .run()
+ .expect(
+ """
src/test/pkg/TestClass1.java:5: Warning: token has not been used to \
restore the calling identity. Introduce a try-finally after the \
declaration and call Binder.restoreCallingIdentity(token) in finally or \
@@ -240,15 +248,15 @@
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
0 errors, 2 warnings
""".addLineContinuation()
- )
+ )
}
/** Non-final token issue tests */
fun testDetectsNonFinalTokens() {
lint().files(
- java(
- """
+ java(
+ """
package test.pkg;
import android.os.Binder;
public class TestClass1 extends Binder {
@@ -271,12 +279,12 @@
}
}
"""
- ).indented(),
- *stubs
+ ).indented(),
+ *stubs
)
- .run()
- .expect(
- """
+ .run()
+ .expect(
+ """
src/test/pkg/TestClass1.java:5: Warning: token1 is a non-final token from \
Binder.clearCallingIdentity(). Add final keyword to token1. \
[NonFinalTokenOfOriginalCallingIdentity]
@@ -294,7 +302,7 @@
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
0 errors, 3 warnings
""".addLineContinuation()
- )
+ )
}
/** Nested clearCallingIdentity() calls issue tests */
@@ -302,8 +310,8 @@
fun testDetectsNestedClearCallingIdentityCalls() {
// Pattern: clear - clear - clear - restore - restore - restore
lint().files(
- java(
- """
+ java(
+ """
package test.pkg;
import android.os.Binder;
public class TestClass1 extends Binder {
@@ -326,12 +334,12 @@
}
}
"""
- ).indented(),
- *stubs
+ ).indented(),
+ *stubs
)
- .run()
- .expect(
- """
+ .run()
+ .expect(
+ """
src/test/pkg/TestClass1.java:7: Warning: The calling identity has already \
been cleared and returned into token1. Move token2 declaration after \
restoring the calling identity with Binder.restoreCallingIdentity(token1). \
@@ -348,15 +356,15 @@
src/test/pkg/TestClass1.java:5: Location of the token1 declaration.
0 errors, 2 warnings
""".addLineContinuation()
- )
+ )
}
/** clearCallingIdentity() not followed by try-finally issue tests */
fun testDetectsClearIdentityCallNotFollowedByTryFinally() {
lint().files(
- java(
- """
+ java(
+ """
package test.pkg;
import android.os.Binder;
public class TestClass1 extends Binder{
@@ -397,12 +405,12 @@
}
}
"""
- ).indented(),
- *stubs
+ ).indented(),
+ *stubs
)
- .run()
- .expect(
- """
+ .run()
+ .expect(
+ """
src/test/pkg/TestClass1.java:5: Warning: You cleared the calling identity \
and returned the result into token, but the next statement is not a \
try-finally statement. Define a try-finally block after token declaration \
@@ -445,15 +453,15 @@
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
0 errors, 5 warnings
""".addLineContinuation()
- )
+ )
}
/** restoreCallingIdentity() call not in finally block issue tests */
fun testDetectsRestoreCallingIdentityCallNotInFinally() {
lint().files(
- java(
- """
+ java(
+ """
package test.pkg;
import android.os.Binder;
public class TestClass1 extends Binder {
@@ -482,12 +490,12 @@
}
}
"""
- ).indented(),
- *stubs
+ ).indented(),
+ *stubs
)
- .run()
- .expect(
- """
+ .run()
+ .expect(
+ """
src/test/pkg/TestClass1.java:10: Warning: \
Binder.restoreCallingIdentity(token) is not an immediate child of the \
finally block of the try statement after token declaration. Surround the c\
@@ -511,13 +519,13 @@
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
0 errors, 3 warnings
""".addLineContinuation()
- )
+ )
}
fun testDetectsRestoreCallingIdentityCallNotInFinallyInScopes() {
lint().files(
- java(
- """
+ java(
+ """
package test.pkg;
import android.os.Binder;
public class TestClass1 extends Binder {
@@ -560,12 +568,12 @@
}
}
"""
- ).indented(),
- *stubs
+ ).indented(),
+ *stubs
)
- .run()
- .expect(
- """
+ .run()
+ .expect(
+ """
src/test/pkg/TestClass1.java:11: Warning: \
Binder.restoreCallingIdentity(token1) is not an immediate child of the \
finally block of the try statement after token1 declaration. Surround the \
@@ -596,15 +604,15 @@
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
0 errors, 4 warnings
""".addLineContinuation()
- )
+ )
}
/** Use of caller-aware methods after clearCallingIdentity() issue tests */
fun testDetectsUseOfCallerAwareMethodsWithClearedIdentityIssuesInScopes() {
lint().files(
- java(
- """
+ java(
+ """
package test.pkg;
import android.os.Binder;
import android.os.UserHandle;
@@ -632,12 +640,12 @@
}
}
"""
- ).indented(),
- *stubs
+ ).indented(),
+ *stubs
)
- .run()
- .expect(
- """
+ .run()
+ .expect(
+ """
src/test/pkg/TestClass1.java:8: Warning: You cleared the original identity \
with Binder.clearCallingIdentity() and returned into token, so \
getCallingPid() will be using your own identity instead of the \
@@ -736,13 +744,58 @@
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
0 errors, 12 warnings
""".addLineContinuation()
- )
+ )
+ }
+
+ /** Result of Binder.clearCallingIdentity() is not stored in a variable issue tests */
+
+ fun testDetectsResultOfClearIdentityCallNotStoredInVariable() {
+ lint().files(
+ java(
+ """
+ package test.pkg;
+ import android.os.Binder;
+ public class TestClass1 extends Binder {
+ private void testMethod() {
+ Binder.clearCallingIdentity();
+ android.os.Binder.clearCallingIdentity();
+ clearCallingIdentity();
+ }
+ }
+ """
+ ).indented(),
+ *stubs
+ )
+ .run()
+ .expect(
+ """
+ src/test/pkg/TestClass1.java:5: Warning: You cleared the original identity \
+ with Binder.clearCallingIdentity() but did not store the result in a \
+ variable. You need to store the result in a variable and restore it later. \
+ [ResultOfClearIdentityCallNotStoredInVariable]
+ Binder.clearCallingIdentity();
+ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+ src/test/pkg/TestClass1.java:6: Warning: You cleared the original identity \
+ with android.os.Binder.clearCallingIdentity() but did not store the result \
+ in a variable. You need to store the result in a variable and restore it \
+ later. [ResultOfClearIdentityCallNotStoredInVariable]
+ android.os.Binder.clearCallingIdentity();
+ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+ src/test/pkg/TestClass1.java:7: Warning: You cleared the original identity \
+ with clearCallingIdentity() but did not store the result in a variable. \
+ You need to store the result in a variable and restore it later. \
+ [ResultOfClearIdentityCallNotStoredInVariable]
+ clearCallingIdentity();
+ ~~~~~~~~~~~~~~~~~~~~~~
+ 0 errors, 3 warnings
+ """.addLineContinuation()
+ )
}
/** Stubs for classes used for testing */
private val binderStub: TestFile = java(
- """
+ """
package android.os;
public class Binder {
public static final native long clearCallingIdentity() {
@@ -767,7 +820,7 @@
).indented()
private val userHandleStub: TestFile = java(
- """
+ """
package android.os;
import android.annotation.AppIdInt;
import android.annotation.UserIdInt;
@@ -792,7 +845,7 @@
).indented()
private val userIdIntStub: TestFile = java(
- """
+ """
package android.annotation;
public @interface UserIdInt {
}
@@ -800,7 +853,7 @@
).indented()
private val appIdIntStub: TestFile = java(
- """
+ """
package android.annotation;
public @interface AppIdInt {
}
diff --git a/tools/lint/checks/src/test/java/com/google/android/lint/CallingSettingsNonUserGetterMethodsIssueDetectorTest.kt b/tools/lint/framework/checks/src/test/java/com/google/android/lint/CallingSettingsNonUserGetterMethodsIssueDetectorTest.kt
similarity index 100%
rename from tools/lint/checks/src/test/java/com/google/android/lint/CallingSettingsNonUserGetterMethodsIssueDetectorTest.kt
rename to tools/lint/framework/checks/src/test/java/com/google/android/lint/CallingSettingsNonUserGetterMethodsIssueDetectorTest.kt
diff --git a/tools/lint/framework/checks/src/test/java/com/google/android/lint/PackageVisibilityDetectorTest.kt b/tools/lint/framework/checks/src/test/java/com/google/android/lint/PackageVisibilityDetectorTest.kt
new file mode 100644
index 0000000..a70644a
--- /dev/null
+++ b/tools/lint/framework/checks/src/test/java/com/google/android/lint/PackageVisibilityDetectorTest.kt
@@ -0,0 +1,271 @@
+/*
+ * Copyright (C) 2022 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
+
+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 PackageVisibilityDetectorTest : LintDetectorTest() {
+ override fun getDetector(): Detector = PackageVisibilityDetector()
+
+ override fun getIssues(): MutableList<Issue> = mutableListOf(
+ PackageVisibilityDetector.ISSUE_PACKAGE_NAME_NO_PACKAGE_VISIBILITY_FILTERS
+ )
+
+ override fun lint(): TestLintTask = super.lint().allowMissingSdk(true)
+
+ fun testDetectIssuesParameterDoesNotApplyPackageVisibilityFilters() {
+ lint().files(java(
+ """
+ package com.android.server.lint.test;
+ import android.internal.test.IFoo;
+
+ public class TestClass extends IFoo.Stub {
+ @Override
+ public boolean hasPackage(String packageName) {
+ return packageName != null;
+ }
+ }
+ """).indented(), *stubs
+ ).run().expect(
+ """
+ src/com/android/server/lint/test/TestClass.java:6: Warning: \
+ Api: hasPackage contains a package name parameter: packageName does not apply \
+ package visibility filtering rules. \
+ [ApiMightLeakAppVisibility]
+ public boolean hasPackage(String packageName) {
+ ~~~~~~~~~~~~~~~~~~
+ 0 errors, 1 warnings
+ """.addLineContinuation()
+ )
+ }
+
+ fun testDoesNotDetectIssuesApiInvokesAppOps() {
+ lint().files(java(
+ """
+ package com.android.server.lint.test;
+ import android.app.AppOpsManager;
+ import android.os.Binder;
+ import android.internal.test.IFoo;
+
+ public class TestClass extends IFoo.Stub {
+ private AppOpsManager mAppOpsManager;
+
+ @Override
+ public boolean hasPackage(String packageName) {
+ checkPackage(packageName);
+ return packageName != null;
+ }
+
+ private void checkPackage(String packageName) {
+ mAppOpsManager.checkPackage(Binder.getCallingUid(), packageName);
+ }
+ }
+ """
+ ).indented(), *stubs).run().expectClean()
+ }
+
+ fun testDoesNotDetectIssuesApiInvokesEnforcePermission() {
+ lint().files(java(
+ """
+ package com.android.server.lint.test;
+ import android.content.Context;
+ import android.internal.test.IFoo;
+
+ public class TestClass extends IFoo.Stub {
+ private Context mContext;
+
+ @Override
+ public boolean hasPackage(String packageName) {
+ enforcePermission();
+ return packageName != null;
+ }
+
+ private void enforcePermission() {
+ mContext.checkCallingPermission(
+ android.Manifest.permission.ACCESS_INPUT_FLINGER);
+ }
+ }
+ """
+ ).indented(), *stubs).run().expectClean()
+ }
+
+ fun testDoesNotDetectIssuesApiInvokesPackageManager() {
+ lint().files(java(
+ """
+ package com.android.server.lint.test;
+ import android.content.pm.PackageInfo;
+ import android.content.pm.PackageManager;
+ import android.internal.test.IFoo;
+
+ public class TestClass extends IFoo.Stub {
+ private PackageManager mPackageManager;
+
+ @Override
+ public boolean hasPackage(String packageName) {
+ return getPackageInfo(packageName) != null;
+ }
+
+ private PackageInfo getPackageInfo(String packageName) {
+ try {
+ return mPackageManager.getPackageInfo(packageName, 0);
+ } catch (PackageManager.NameNotFoundException e) {
+ return null;
+ }
+ }
+ }
+ """
+ ).indented(), *stubs).run().expectClean()
+ }
+
+ fun testDetectIssuesApiInvokesPackageManagerAndClearCallingIdentify() {
+ lint().files(java(
+ """
+ package com.android.server.lint.test;
+ import android.content.pm.PackageInfo;
+ import android.content.pm.PackageManager;
+ import android.internal.test.IFoo;import android.os.Binder;
+
+ public class TestClass extends IFoo.Stub {
+ private PackageManager mPackageManager;
+
+ @Override
+ public boolean hasPackage(String packageName) {
+ return getPackageInfo(packageName) != null;
+ }
+
+ private PackageInfo getPackageInfo(String packageName) {
+ long token = Binder.clearCallingIdentity();
+ try {
+ try {
+ return mPackageManager.getPackageInfo(packageName, 0);
+ } catch (PackageManager.NameNotFoundException e) {
+ return null;
+ }
+ } finally{
+ Binder.restoreCallingIdentity(token);
+ }
+ }
+ }
+ """).indented(), *stubs
+ ).run().expect(
+ """
+ src/com/android/server/lint/test/TestClass.java:10: Warning: \
+ Api: hasPackage contains a package name parameter: packageName does not apply \
+ package visibility filtering rules. \
+ [ApiMightLeakAppVisibility]
+ public boolean hasPackage(String packageName) {
+ ~~~~~~~~~~~~~~~~~~
+ 0 errors, 1 warnings
+ """.addLineContinuation()
+ )
+ }
+
+ fun testDoesNotDetectIssuesApiNotSystemPackagePrefix() {
+ lint().files(java(
+ """
+ package com.test.not.system.prefix;
+ import android.internal.test.IFoo;
+
+ public class TestClass extends IFoo.Stub {
+ @Override
+ public boolean hasPackage(String packageName) {
+ return packageName != null;
+ }
+ }
+ """
+ ).indented(), *stubs).run().expectClean()
+ }
+
+ private val contextStub: TestFile = java(
+ """
+ package android.content;
+
+ public abstract class Context {
+ public abstract int checkCallingPermission(String permission);
+ }
+ """
+ ).indented()
+
+ private val appOpsManagerStub: TestFile = java(
+ """
+ package android.app;
+
+ public class AppOpsManager {
+ public void checkPackage(int uid, String packageName) {
+ }
+ }
+ """
+ ).indented()
+
+ private val packageManagerStub: TestFile = java(
+ """
+ package android.content.pm;
+ import android.content.pm.PackageInfo;
+
+ public abstract class PackageManager {
+ public static class NameNotFoundException extends AndroidException {
+ }
+
+ public abstract PackageInfo getPackageInfo(String packageName, int flags)
+ throws NameNotFoundException;
+ }
+ """
+ ).indented()
+
+ private val packageInfoStub: TestFile = java(
+ """
+ package android.content.pm;
+ public class PackageInfo {}
+ """
+ ).indented()
+
+ private val binderStub: TestFile = java(
+ """
+ package android.os;
+
+ public class Binder {
+ public static final native long clearCallingIdentity();
+ public static final native void restoreCallingIdentity(long token);
+ public static final native int getCallingUid();
+ }
+ """
+ ).indented()
+
+ private val interfaceIFooStub: TestFile = java(
+ """
+ package android.internal.test;
+ import android.os.Binder;
+
+ public interface IFoo {
+ boolean hasPackage(String packageName);
+ public abstract static class Stub extends Binder implements IFoo {
+ }
+ }
+ """
+ ).indented()
+
+ private val stubs = arrayOf(contextStub, appOpsManagerStub, packageManagerStub,
+ packageInfoStub, binderStub, interfaceIFooStub)
+
+ // Substitutes "backslash + new line" with an empty string to imitate line continuation
+ private fun String.addLineContinuation(): String = this.trimIndent().replace("\\\n", "")
+}
diff --git a/tools/lint/framework/checks/src/test/java/com/google/android/lint/parcel/SaferParcelCheckerTest.kt b/tools/lint/framework/checks/src/test/java/com/google/android/lint/parcel/SaferParcelCheckerTest.kt
new file mode 100644
index 0000000..e686695
--- /dev/null
+++ b/tools/lint/framework/checks/src/test/java/com/google/android/lint/parcel/SaferParcelCheckerTest.kt
@@ -0,0 +1,823 @@
+/*
+ * Copyright (C) 2022 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.parcel
+
+import com.android.tools.lint.checks.infrastructure.LintDetectorTest
+import com.android.tools.lint.checks.infrastructure.TestLintTask
+import com.android.tools.lint.checks.infrastructure.TestMode
+import com.android.tools.lint.detector.api.Detector
+import com.android.tools.lint.detector.api.Issue
+
+@Suppress("UnstableApiUsage")
+class SaferParcelCheckerTest : LintDetectorTest() {
+ override fun getDetector(): Detector = SaferParcelChecker()
+
+ override fun getIssues(): List<Issue> = listOf(
+ SaferParcelChecker.ISSUE_UNSAFE_API_USAGE
+ )
+
+ override fun lint(): TestLintTask =
+ super.lint()
+ .allowMissingSdk(true)
+ // We don't do partial analysis in the platform
+ .skipTestModes(TestMode.PARTIAL)
+
+ /** Parcel Tests */
+
+ fun testParcelDetectUnsafeReadSerializable() {
+ lint()
+ .files(
+ java(
+ """
+ package test.pkg;
+ import android.os.Parcel;
+ import java.io.Serializable;
+
+ public class TestClass {
+ private TestClass(Parcel p) {
+ Serializable ans = p.readSerializable();
+ }
+ }
+ """
+ ).indented(),
+ *includes
+ )
+ .expectIdenticalTestModeOutput(false)
+ .run()
+ .expect(
+ """
+ src/test/pkg/TestClass.java:7: Warning: Unsafe Parcel.readSerializable() \
+ API usage [UnsafeParcelApi]
+ Serializable ans = p.readSerializable();
+ ~~~~~~~~~~~~~~~~~~~~
+ 0 errors, 1 warnings
+ """.addLineContinuation()
+ )
+ }
+
+ fun testParcelDoesNotDetectSafeReadSerializable() {
+ lint()
+ .files(
+ java(
+ """
+ package test.pkg;
+ import android.os.Parcel;
+ import java.io.Serializable;
+
+ public class TestClass {
+ private TestClass(Parcel p) {
+ String ans = p.readSerializable(null, String.class);
+ }
+ }
+ """
+ ).indented(),
+ *includes
+ )
+ .run()
+ .expect("No warnings.")
+ }
+
+ fun testParcelDetectUnsafeReadArrayList() {
+ lint()
+ .files(
+ java(
+ """
+ package test.pkg;
+ import android.os.Parcel;
+
+ public class TestClass {
+ private TestClass(Parcel p) {
+ ArrayList ans = p.readArrayList(null);
+ }
+ }
+ """
+ ).indented(),
+ *includes
+ )
+ .run()
+ .expect(
+ """
+ src/test/pkg/TestClass.java:6: Warning: Unsafe Parcel.readArrayList() API \
+ usage [UnsafeParcelApi]
+ ArrayList ans = p.readArrayList(null);
+ ~~~~~~~~~~~~~~~~~~~~~
+ 0 errors, 1 warnings
+ """.addLineContinuation()
+ )
+ }
+
+ fun testParcelDoesNotDetectSafeReadArrayList() {
+ lint()
+ .files(
+ java(
+ """
+ package test.pkg;
+ import android.content.Intent;
+ import android.os.Parcel;
+
+ public class TestClass {
+ private TestClass(Parcel p) {
+ ArrayList<Intent> ans = p.readArrayList(null, Intent.class);
+ }
+ }
+ """
+ ).indented(),
+ *includes
+ )
+ .run()
+ .expect("No warnings.")
+ }
+
+ fun testParcelDetectUnsafeReadList() {
+ lint()
+ .files(
+ java(
+ """
+ package test.pkg;
+ import android.content.Intent;
+ import android.os.Parcel;
+ import java.util.List;
+
+ public class TestClass {
+ private TestClass(Parcel p) {
+ List<Intent> list = new ArrayList<Intent>();
+ p.readList(list, null);
+ }
+ }
+ """
+ ).indented(),
+ *includes
+ )
+ .run()
+ .expect(
+ """
+ src/test/pkg/TestClass.java:9: Warning: Unsafe Parcel.readList() API usage \
+ [UnsafeParcelApi]
+ p.readList(list, null);
+ ~~~~~~~~~~~~~~~~~~~~~~
+ 0 errors, 1 warnings
+ """.addLineContinuation()
+ )
+ }
+
+ fun testDParceloesNotDetectSafeReadList() {
+ lint()
+ .files(
+ java(
+ """
+ package test.pkg;
+ import android.content.Intent;
+ import android.os.Parcel;
+ import java.util.List;
+
+ public class TestClass {
+ private TestClass(Parcel p) {
+ List<Intent> list = new ArrayList<Intent>();
+ p.readList(list, null, Intent.class);
+ }
+ }
+ """
+ ).indented(),
+ *includes
+ )
+ .run()
+ .expect("No warnings.")
+ }
+
+ fun testParcelDetectUnsafeReadParcelable() {
+ lint()
+ .files(
+ java(
+ """
+ package test.pkg;
+ import android.content.Intent;
+ import android.os.Parcel;
+
+ public class TestClass {
+ private TestClass(Parcel p) {
+ Intent ans = p.readParcelable(null);
+ }
+ }
+ """
+ ).indented(),
+ *includes
+ )
+ .run()
+ .expect(
+ """
+ src/test/pkg/TestClass.java:7: Warning: Unsafe Parcel.readParcelable() API \
+ usage [UnsafeParcelApi]
+ Intent ans = p.readParcelable(null);
+ ~~~~~~~~~~~~~~~~~~~~~~
+ 0 errors, 1 warnings
+ """.addLineContinuation()
+ )
+ }
+
+ fun testParcelDoesNotDetectSafeReadParcelable() {
+ lint()
+ .files(
+ java(
+ """
+ package test.pkg;
+ import android.content.Intent;
+ import android.os.Parcel;
+
+ public class TestClass {
+ private TestClass(Parcel p) {
+ Intent ans = p.readParcelable(null, Intent.class);
+ }
+ }
+ """
+ ).indented(),
+ *includes
+ )
+ .run()
+ .expect("No warnings.")
+ }
+
+ fun testParcelDetectUnsafeReadParcelableList() {
+ lint()
+ .files(
+ java(
+ """
+ package test.pkg;
+ import android.content.Intent;
+ import android.os.Parcel;
+ import java.util.List;
+
+ public class TestClass {
+ private TestClass(Parcel p) {
+ List<Intent> list = new ArrayList<Intent>();
+ List<Intent> ans = p.readParcelableList(list, null);
+ }
+ }
+ """
+ ).indented(),
+ *includes
+ )
+ .run()
+ .expect(
+ """
+ src/test/pkg/TestClass.java:9: Warning: Unsafe Parcel.readParcelableList() \
+ API usage [UnsafeParcelApi]
+ List<Intent> ans = p.readParcelableList(list, null);
+ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+ 0 errors, 1 warnings
+ """.addLineContinuation()
+ )
+ }
+
+ fun testParcelDoesNotDetectSafeReadParcelableList() {
+ lint()
+ .files(
+ java(
+ """
+ package test.pkg;
+ import android.content.Intent;
+ import android.os.Parcel;
+ import java.util.List;
+
+ public class TestClass {
+ private TestClass(Parcel p) {
+ List<Intent> list = new ArrayList<Intent>();
+ List<Intent> ans =
+ p.readParcelableList(list, null, Intent.class);
+ }
+ }
+ """
+ ).indented(),
+ *includes
+ )
+ .run()
+ .expect("No warnings.")
+ }
+
+ fun testParcelDetectUnsafeReadSparseArray() {
+ lint()
+ .files(
+ java(
+ """
+ package test.pkg;
+ import android.content.Intent;
+ import android.os.Parcel;
+ import android.util.SparseArray;
+
+ public class TestClass {
+ private TestClass(Parcel p) {
+ SparseArray<Intent> ans = p.readSparseArray(null);
+ }
+ }
+ """
+ ).indented(),
+ *includes
+ )
+ .run()
+ .expect(
+ """
+ src/test/pkg/TestClass.java:8: Warning: Unsafe Parcel.readSparseArray() API\
+ usage [UnsafeParcelApi]
+ SparseArray<Intent> ans = p.readSparseArray(null);
+ ~~~~~~~~~~~~~~~~~~~~~~~
+ 0 errors, 1 warnings
+ """.addLineContinuation()
+ )
+ }
+
+ fun testParcelDoesNotDetectSafeReadSparseArray() {
+ lint()
+ .files(
+ java(
+ """
+ package test.pkg;
+ import android.content.Intent;
+ import android.os.Parcel;
+ import android.util.SparseArray;
+
+ public class TestClass {
+ private TestClass(Parcel p) {
+ SparseArray<Intent> ans =
+ p.readSparseArray(null, Intent.class);
+ }
+ }
+ """
+ ).indented(),
+ *includes
+ )
+ .run()
+ .expect("No warnings.")
+ }
+
+ fun testParcelDetectUnsafeReadSArray() {
+ lint()
+ .files(
+ java(
+ """
+ package test.pkg;
+ import android.content.Intent;
+ import android.os.Parcel;
+
+ public class TestClass {
+ private TestClass(Parcel p) {
+ Intent[] ans = p.readArray(null);
+ }
+ }
+ """
+ ).indented(),
+ *includes
+ )
+ .run()
+ .expect(
+ """
+ src/test/pkg/TestClass.java:7: Warning: Unsafe Parcel.readArray() API\
+ usage [UnsafeParcelApi]
+ Intent[] ans = p.readArray(null);
+ ~~~~~~~~~~~~~~~~~
+ 0 errors, 1 warnings
+ """.addLineContinuation()
+ )
+ }
+
+ fun testParcelDoesNotDetectSafeReadArray() {
+ lint()
+ .files(
+ java(
+ """
+ package test.pkg;
+ import android.content.Intent;
+ import android.os.Parcel;
+
+ public class TestClass {
+ private TestClass(Parcel p) {
+ Intent[] ans = p.readArray(null, Intent.class);
+ }
+ }
+ """
+ ).indented(),
+ *includes
+ )
+ .run()
+ .expect("No warnings.")
+ }
+
+ fun testParcelDetectUnsafeReadParcelableSArray() {
+ lint()
+ .files(
+ java(
+ """
+ package test.pkg;
+ import android.content.Intent;
+ import android.os.Parcel;
+
+ public class TestClass {
+ private TestClass(Parcel p) {
+ Intent[] ans = p.readParcelableArray(null);
+ }
+ }
+ """
+ ).indented(),
+ *includes
+ )
+ .run()
+ .expect(
+ """
+ src/test/pkg/TestClass.java:7: Warning: Unsafe Parcel.readParcelableArray() API\
+ usage [UnsafeParcelApi]
+ Intent[] ans = p.readParcelableArray(null);
+ ~~~~~~~~~~~~~~~~~~~~~~~~~~~
+ 0 errors, 1 warnings
+ """.addLineContinuation()
+ )
+ }
+
+ fun testParcelDoesNotDetectSafeReadParcelableArray() {
+ lint()
+ .files(
+ java(
+ """
+ package test.pkg;
+ import android.content.Intent;
+ import android.os.Parcel;
+
+ public class TestClass {
+ private TestClass(Parcel p) {
+ Intent[] ans = p.readParcelableArray(null, Intent.class);
+ }
+ }
+ """
+ ).indented(),
+ *includes
+ )
+ .run()
+ .expect("No warnings.")
+ }
+
+ /** Bundle Tests */
+
+ fun testBundleDetectUnsafeGetParcelable() {
+ lint()
+ .files(
+ java(
+ """
+ package test.pkg;
+ import android.content.Intent;
+ import android.os.Bundle;
+
+ public class TestClass {
+ private TestClass(Bundle b) {
+ Intent ans = b.getParcelable("key");
+ }
+ }
+ """
+ ).indented(),
+ *includes
+ )
+ .run()
+ .expect(
+ """
+ src/test/pkg/TestClass.java:7: Warning: Unsafe Bundle.getParcelable() API usage [UnsafeParcelApi]
+ Intent ans = b.getParcelable("key");
+ ~~~~~~~~~~~~~~~~~~~~~~
+ 0 errors, 1 warnings
+ """.addLineContinuation()
+ )
+ }
+
+ fun testBundleDoesNotDetectSafeGetParcelable() {
+ lint()
+ .files(
+ java(
+ """
+ package test.pkg;
+ import android.content.Intent;
+ import android.os.Bundle;
+
+ public class TestClass {
+ private TestClass(Bundle b) {
+ Intent ans = b.getParcelable("key", Intent.class);
+ }
+ }
+ """
+ ).indented(),
+ *includes
+ )
+ .run()
+ .expect("No warnings.")
+ }
+
+ fun testBundleDetectUnsafeGetParcelableArrayList() {
+ lint()
+ .files(
+ java(
+ """
+ package test.pkg;
+ import android.content.Intent;
+ import android.os.Bundle;
+
+ public class TestClass {
+ private TestClass(Bundle b) {
+ ArrayList<Intent> ans = b.getParcelableArrayList("key");
+ }
+ }
+ """
+ ).indented(),
+ *includes
+ )
+ .run()
+ .expect(
+ """
+ src/test/pkg/TestClass.java:7: Warning: Unsafe Bundle.getParcelableArrayList() API usage [UnsafeParcelApi]
+ ArrayList<Intent> ans = b.getParcelableArrayList("key");
+ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+ 0 errors, 1 warnings
+ """.addLineContinuation()
+ )
+ }
+
+ fun testBundleDoesNotDetectSafeGetParcelableArrayList() {
+ lint()
+ .files(
+ java(
+ """
+ package test.pkg;
+ import android.content.Intent;
+ import android.os.Bundle;
+
+ public class TestClass {
+ private TestClass(Bundle b) {
+ ArrayList<Intent> ans = b.getParcelableArrayList("key", Intent.class);
+ }
+ }
+ """
+ ).indented(),
+ *includes
+ )
+ .run()
+ .expect("No warnings.")
+ }
+
+ fun testBundleDetectUnsafeGetParcelableArray() {
+ lint()
+ .files(
+ java(
+ """
+ package test.pkg;
+ import android.content.Intent;
+ import android.os.Bundle;
+
+ public class TestClass {
+ private TestClass(Bundle b) {
+ Intent[] ans = b.getParcelableArray("key");
+ }
+ }
+ """
+ ).indented(),
+ *includes
+ )
+ .run()
+ .expect(
+ """
+ src/test/pkg/TestClass.java:7: Warning: Unsafe Bundle.getParcelableArray() API usage [UnsafeParcelApi]
+ Intent[] ans = b.getParcelableArray("key");
+ ~~~~~~~~~~~~~~~~~~~~~~~~~~~
+ 0 errors, 1 warnings
+ """.addLineContinuation()
+ )
+ }
+
+ fun testBundleDoesNotDetectSafeGetParcelableArray() {
+ lint()
+ .files(
+ java(
+ """
+ package test.pkg;
+ import android.content.Intent;
+ import android.os.Bundle;
+
+ public class TestClass {
+ private TestClass(Bundle b) {
+ Intent[] ans = b.getParcelableArray("key", Intent.class);
+ }
+ }
+ """
+ ).indented(),
+ *includes
+ )
+ .run()
+ .expect("No warnings.")
+ }
+
+ fun testBundleDetectUnsafeGetSparseParcelableArray() {
+ lint()
+ .files(
+ java(
+ """
+ package test.pkg;
+ import android.content.Intent;
+ import android.os.Bundle;
+
+ public class TestClass {
+ private TestClass(Bundle b) {
+ SparseArray<Intent> ans = b.getSparseParcelableArray("key");
+ }
+ }
+ """
+ ).indented(),
+ *includes
+ )
+ .run()
+ .expect(
+ """
+ src/test/pkg/TestClass.java:7: Warning: Unsafe Bundle.getSparseParcelableArray() API usage [UnsafeParcelApi]
+ SparseArray<Intent> ans = b.getSparseParcelableArray("key");
+ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+ 0 errors, 1 warnings
+ """.addLineContinuation()
+ )
+ }
+
+ fun testBundleDoesNotDetectSafeGetSparseParcelableArray() {
+ lint()
+ .files(
+ java(
+ """
+ package test.pkg;
+ import android.content.Intent;
+ import android.os.Bundle;
+
+ public class TestClass {
+ private TestClass(Bundle b) {
+ SparseArray<Intent> ans = b.getSparseParcelableArray("key", Intent.class);
+ }
+ }
+ """
+ ).indented(),
+ *includes
+ )
+ .run()
+ .expect("No warnings.")
+ }
+
+ /** Intent Tests */
+
+ fun testIntentDetectUnsafeGetParcelableExtra() {
+ lint()
+ .files(
+ java(
+ """
+ package test.pkg;
+ import android.content.Intent;
+
+ public class TestClass {
+ private TestClass(Intent i) {
+ Intent ans = i.getParcelableExtra("name");
+ }
+ }
+ """
+ ).indented(),
+ *includes
+ )
+ .run()
+ .expect(
+ """
+ src/test/pkg/TestClass.java:6: Warning: Unsafe Intent.getParcelableExtra() API usage [UnsafeParcelApi]
+ Intent ans = i.getParcelableExtra("name");
+ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+ 0 errors, 1 warnings
+ """.addLineContinuation()
+ )
+ }
+
+ fun testIntentDoesNotDetectSafeGetParcelableExtra() {
+ lint()
+ .files(
+ java(
+ """
+ package test.pkg;
+ import android.content.Intent;
+
+ public class TestClass {
+ private TestClass(Intent i) {
+ Intent ans = i.getParcelableExtra("name", Intent.class);
+ }
+ }
+ """
+ ).indented(),
+ *includes
+ )
+ .run()
+ .expect("No warnings.")
+ }
+
+
+ /** Stubs for classes used for testing */
+
+
+ private val includes =
+ arrayOf(
+ manifest().minSdk("33"),
+ java(
+ """
+ package android.os;
+ import java.util.ArrayList;
+ import java.util.List;
+ import java.util.Map;
+ import java.util.HashMap;
+
+ public final class Parcel {
+ // Deprecated
+ public Object[] readArray(ClassLoader loader) { return null; }
+ public ArrayList readArrayList(ClassLoader loader) { return null; }
+ public HashMap readHashMap(ClassLoader loader) { return null; }
+ public void readList(List outVal, ClassLoader loader) {}
+ public void readMap(Map outVal, ClassLoader loader) {}
+ public <T extends Parcelable> T readParcelable(ClassLoader loader) { return null; }
+ public Parcelable[] readParcelableArray(ClassLoader loader) { return null; }
+ public Parcelable.Creator<?> readParcelableCreator(ClassLoader loader) { return null; }
+ public <T extends Parcelable> List<T> readParcelableList(List<T> list, ClassLoader cl) { return null; }
+ public Serializable readSerializable() { return null; }
+ public <T> SparseArray<T> readSparseArray(ClassLoader loader) { return null; }
+
+ // Replacements
+ public <T> T[] readArray(ClassLoader loader, Class<T> clazz) { return null; }
+ public <T> ArrayList<T> readArrayList(ClassLoader loader, Class<? extends T> clazz) { return null; }
+ public <K, V> HashMap<K,V> readHashMap(ClassLoader loader, Class<? extends K> clazzKey, Class<? extends V> clazzValue) { return null; }
+ public <T> void readList(List<? super T> outVal, ClassLoader loader, Class<T> clazz) {}
+ public <K, V> void readMap(Map<? super K, ? super V> outVal, ClassLoader loader, Class<K> clazzKey, Class<V> clazzValue) {}
+ public <T> T readParcelable(ClassLoader loader, Class<T> clazz) { return null; }
+ public <T> T[] readParcelableArray(ClassLoader loader, Class<T> clazz) { return null; }
+ public <T> Parcelable.Creator<T> readParcelableCreator(ClassLoader loader, Class<T> clazz) { return null; }
+ public <T> List<T> readParcelableList(List<T> list, ClassLoader cl, Class<T> clazz) { return null; }
+ public <T> T readSerializable(ClassLoader loader, Class<T> clazz) { return null; }
+ public <T> SparseArray<T> readSparseArray(ClassLoader loader, Class<? extends T> clazz) { return null; }
+ }
+ """
+ ).indented(),
+ java(
+ """
+ package android.os;
+ import java.util.ArrayList;
+ import java.util.List;
+ import java.util.Map;
+ import java.util.HashMap;
+
+ public final class Bundle {
+ // Deprecated
+ public <T extends Parcelable> T getParcelable(String key) { return null; }
+ public <T extends Parcelable> ArrayList<T> getParcelableArrayList(String key) { return null; }
+ public Parcelable[] getParcelableArray(String key) { return null; }
+ public <T extends Parcelable> SparseArray<T> getSparseParcelableArray(String key) { return null; }
+
+ // Replacements
+ public <T> T getParcelable(String key, Class<T> clazz) { return null; }
+ public <T> ArrayList<T> getParcelableArrayList(String key, Class<? extends T> clazz) { return null; }
+ public <T> T[] getParcelableArray(String key, Class<T> clazz) { return null; }
+ public <T> SparseArray<T> getSparseParcelableArray(String key, Class<? extends T> clazz) { return null; }
+
+ }
+ """
+ ).indented(),
+ java(
+ """
+ package android.os;
+ public interface Parcelable {}
+ """
+ ).indented(),
+ java(
+ """
+ package android.content;
+ public class Intent implements Parcelable, Cloneable {
+ // Deprecated
+ public <T extends Parcelable> T getParcelableExtra(String name) { return null; }
+
+ // Replacements
+ public <T> T getParcelableExtra(String name, Class<T> clazz) { return null; }
+
+ }
+ """
+ ).indented(),
+ java(
+ """
+ package android.util;
+ public class SparseArray<E> implements Cloneable {}
+ """
+ ).indented(),
+ )
+
+ // Substitutes "backslash + new line" with an empty string to imitate line continuation
+ private fun String.addLineContinuation(): String = this.trimIndent().replace("\\\n", "")
+}
diff --git a/tools/lint/Android.bp b/tools/lint/global/Android.bp
similarity index 61%
copy from tools/lint/Android.bp
copy to tools/lint/global/Android.bp
index 2601041..bedb7bd 100644
--- a/tools/lint/Android.bp
+++ b/tools/lint/global/Android.bp
@@ -1,4 +1,4 @@
-// Copyright (C) 2021 The Android Open Source Project
+// Copyright (C) 2022 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.
@@ -22,32 +22,44 @@
}
java_library_host {
- name: "AndroidFrameworkLintChecker",
+ name: "AndroidGlobalLintChecker",
srcs: ["checks/src/main/java/**/*.kt"],
plugins: ["auto_service_plugin"],
libs: [
"auto_service_annotations",
"lint_api",
],
+ static_libs: ["AndroidCommonLint"],
kotlincflags: ["-Xjvm-default=all"],
+ dist: {
+ targets: ["droid"],
+ },
}
java_test_host {
- name: "AndroidFrameworkLintCheckerTest",
- // TODO(b/239881504): Since this test was written, Android
- // Lint was updated, and now includes classes that were
- // compiled for java 15. The soong build doesn't support
- // java 15 yet, so we can't compile against "lint". Disable
- // the test until java 15 is supported.
- enabled: false,
+ name: "AndroidGlobalLintCheckerTest",
srcs: ["checks/src/test/java/**/*.kt"],
static_libs: [
- "AndroidFrameworkLintChecker",
+ "AndroidGlobalLintChecker",
"junit",
"lint",
"lint_tests",
],
test_options: {
unit_test: true,
+ tradefed_options: [
+ {
+ // lint bundles in some classes that were built with older versions
+ // of libraries, and no longer load. Since tradefed tries to load
+ // all classes in the jar to look for tests, it crashes loading them.
+ // Exclude these classes from tradefed's search.
+ name: "exclude-paths",
+ value: "org/apache",
+ },
+ {
+ name: "exclude-paths",
+ value: "META-INF",
+ },
+ ],
},
}
diff --git a/tools/lint/checks/src/main/java/com/google/android/lint/AndroidFrameworkIssueRegistry.kt b/tools/lint/global/checks/src/main/java/com/google/android/lint/AndroidGlobalIssueRegistry.kt
similarity index 63%
rename from tools/lint/checks/src/main/java/com/google/android/lint/AndroidFrameworkIssueRegistry.kt
rename to tools/lint/global/checks/src/main/java/com/google/android/lint/AndroidGlobalIssueRegistry.kt
index a6fd9bb..a20266a 100644
--- a/tools/lint/checks/src/main/java/com/google/android/lint/AndroidFrameworkIssueRegistry.kt
+++ b/tools/lint/global/checks/src/main/java/com/google/android/lint/AndroidGlobalIssueRegistry.kt
@@ -1,5 +1,5 @@
/*
- * Copyright (C) 2021 The Android Open Source Project
+ * Copyright (C) 2022 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.
@@ -19,21 +19,19 @@
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.aidl.EnforcePermissionDetector
+import com.google.android.lint.aidl.EnforcePermissionHelperDetector
+import com.google.android.lint.aidl.SimpleManualPermissionEnforcementDetector
import com.google.auto.service.AutoService
@AutoService(IssueRegistry::class)
@Suppress("UnstableApiUsage")
-class AndroidFrameworkIssueRegistry : IssueRegistry() {
+class AndroidGlobalIssueRegistry : IssueRegistry() {
override val issues = listOf(
- CallingIdentityTokenDetector.ISSUE_UNUSED_TOKEN,
- CallingIdentityTokenDetector.ISSUE_NON_FINAL_TOKEN,
- CallingIdentityTokenDetector.ISSUE_NESTED_CLEAR_IDENTITY_CALLS,
- CallingIdentityTokenDetector.ISSUE_RESTORE_IDENTITY_CALL_NOT_IN_FINALLY_BLOCK,
- CallingIdentityTokenDetector.ISSUE_USE_OF_CALLER_AWARE_METHODS_WITH_CLEARED_IDENTITY,
- CallingIdentityTokenDetector.ISSUE_CLEAR_IDENTITY_CALL_NOT_FOLLOWED_BY_TRY_FINALLY,
- CallingSettingsNonUserGetterMethodsDetector.ISSUE_NON_USER_GETTER_CALLED,
EnforcePermissionDetector.ISSUE_MISSING_ENFORCE_PERMISSION,
- EnforcePermissionDetector.ISSUE_MISMATCHING_ENFORCE_PERMISSION
+ EnforcePermissionDetector.ISSUE_MISMATCHING_ENFORCE_PERMISSION,
+ EnforcePermissionHelperDetector.ISSUE_ENFORCE_PERMISSION_HELPER,
+ SimpleManualPermissionEnforcementDetector.ISSUE_SIMPLE_MANUAL_PERMISSION_ENFORCEMENT,
)
override val api: Int
@@ -45,6 +43,6 @@
override val vendor: Vendor = Vendor(
vendorName = "Android",
feedbackUrl = "http://b/issues/new?component=315013",
- contact = "brufino@google.com"
+ contact = "repsonsible-apis@google.com"
)
-}
+}
\ No newline at end of file
diff --git a/tools/lint/global/checks/src/main/java/com/google/android/lint/aidl/AidlImplementationDetector.kt b/tools/lint/global/checks/src/main/java/com/google/android/lint/aidl/AidlImplementationDetector.kt
new file mode 100644
index 0000000..ab6d871
--- /dev/null
+++ b/tools/lint/global/checks/src/main/java/com/google/android/lint/aidl/AidlImplementationDetector.kt
@@ -0,0 +1,52 @@
+/*
+ * Copyright (C) 2022 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.client.api.UElementHandler
+import com.android.tools.lint.detector.api.Detector
+import com.android.tools.lint.detector.api.JavaContext
+import com.android.tools.lint.detector.api.SourceCodeScanner
+import org.jetbrains.uast.UBlockExpression
+import org.jetbrains.uast.UElement
+import org.jetbrains.uast.UMethod
+
+/**
+ * Abstract class for detectors that look for methods implementing
+ * generated AIDL interface stubs
+ */
+abstract class AidlImplementationDetector : Detector(), SourceCodeScanner {
+ override fun getApplicableUastTypes(): List<Class<out UElement?>> =
+ listOf(UMethod::class.java)
+
+ override fun createUastHandler(context: JavaContext): UElementHandler = AidlStubHandler(context)
+
+ private inner class AidlStubHandler(val context: JavaContext) : UElementHandler() {
+ override fun visitMethod(node: UMethod) {
+ val interfaceName = getContainingAidlInterface(context, node)
+ .takeUnless(EXCLUDED_CPP_INTERFACES::contains) ?: return
+ val body = (node.uastBody as? UBlockExpression) ?: return
+ visitAidlMethod(context, node, interfaceName, body)
+ }
+ }
+
+ abstract fun visitAidlMethod(
+ context: JavaContext,
+ node: UMethod,
+ interfaceName: String,
+ body: UBlockExpression,
+ )
+}
diff --git a/tools/lint/global/checks/src/main/java/com/google/android/lint/aidl/Constants.kt b/tools/lint/global/checks/src/main/java/com/google/android/lint/aidl/Constants.kt
new file mode 100644
index 0000000..dcfbe95
--- /dev/null
+++ b/tools/lint/global/checks/src/main/java/com/google/android/lint/aidl/Constants.kt
@@ -0,0 +1,76 @@
+/*
+ * Copyright (C) 2022 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
+
+const val ANNOTATION_ENFORCE_PERMISSION = "android.annotation.EnforcePermission"
+const val ANNOTATION_REQUIRES_NO_PERMISSION = "android.annotation.RequiresNoPermission"
+const val ANNOTATION_PERMISSION_MANUALLY_ENFORCED = "android.annotation.PermissionManuallyEnforced"
+
+val AIDL_PERMISSION_ANNOTATIONS = listOf(
+ ANNOTATION_ENFORCE_PERMISSION,
+ ANNOTATION_REQUIRES_NO_PERMISSION,
+ ANNOTATION_PERMISSION_MANUALLY_ENFORCED
+)
+
+const val BINDER_CLASS = "android.os.Binder"
+const val IINTERFACE_INTERFACE = "android.os.IInterface"
+
+const val AIDL_PERMISSION_HELPER_SUFFIX = "_enforcePermission"
+
+/**
+ * If a non java (e.g. c++) backend is enabled, the @EnforcePermission
+ * annotation cannot be used. At time of writing, the mechanism
+ * is not implemented for non java backends.
+ * TODO: b/242564874 (have lint know which interfaces have the c++ backend enabled)
+ * rather than hard coding this list?
+ */
+val EXCLUDED_CPP_INTERFACES = listOf(
+ "AdbTransportType",
+ "FingerprintAndPairDevice",
+ "IAdbCallback",
+ "IAdbManager",
+ "PairDevice",
+ "IStatsBootstrapAtomService",
+ "StatsBootstrapAtom",
+ "StatsBootstrapAtomValue",
+ "FixedSizeArrayExample",
+ "PlaybackTrackMetadata",
+ "RecordTrackMetadata",
+ "SinkMetadata",
+ "SourceMetadata",
+ "IUpdateEngineStable",
+ "IUpdateEngineStableCallback",
+ "AudioCapabilities",
+ "ConfidenceLevel",
+ "ModelParameter",
+ "ModelParameterRange",
+ "Phrase",
+ "PhraseRecognitionEvent",
+ "PhraseRecognitionExtra",
+ "PhraseSoundModel",
+ "Properties",
+ "RecognitionConfig",
+ "RecognitionEvent",
+ "RecognitionMode",
+ "RecognitionStatus",
+ "SoundModel",
+ "SoundModelType",
+ "Status",
+ "IThermalService",
+ "IPowerManager",
+ "ITunerResourceManager"
+)
diff --git a/tools/lint/global/checks/src/main/java/com/google/android/lint/aidl/EnforcePermissionDetector.kt b/tools/lint/global/checks/src/main/java/com/google/android/lint/aidl/EnforcePermissionDetector.kt
new file mode 100644
index 0000000..0baac2c
--- /dev/null
+++ b/tools/lint/global/checks/src/main/java/com/google/android/lint/aidl/EnforcePermissionDetector.kt
@@ -0,0 +1,226 @@
+/*
+ * Copyright (C) 2022 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.client.api.UElementHandler
+import com.android.tools.lint.detector.api.AnnotationInfo
+import com.android.tools.lint.detector.api.AnnotationOrigin
+import com.android.tools.lint.detector.api.AnnotationUsageInfo
+import com.android.tools.lint.detector.api.AnnotationUsageType
+import com.android.tools.lint.detector.api.Category
+import com.android.tools.lint.detector.api.ConstantEvaluator
+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.PsiAnnotation
+import com.intellij.psi.PsiArrayInitializerMemberValue
+import com.intellij.psi.PsiClass
+import com.intellij.psi.PsiElement
+import com.intellij.psi.PsiMethod
+import org.jetbrains.uast.UAnnotation
+import org.jetbrains.uast.UElement
+import org.jetbrains.uast.UMethod
+import org.jetbrains.uast.toUElement
+
+/**
+ * Lint Detector that ensures that any method overriding a method annotated
+ * with @EnforcePermission is also annotated with the exact same annotation.
+ * The intent is to surface the effective permission checks to the service
+ * implementations.
+ *
+ * This is done with 2 mechanisms:
+ * 1. Visit any annotation usage, to ensure that any derived class will have
+ * the correct annotation on each methods. This is for the top to bottom
+ * propagation.
+ * 2. Visit any annotation, to ensure that if a method is annotated, it has
+ * its ancestor also annotated. This is to avoid having an annotation on a
+ * Java method without the corresponding annotation on the AIDL interface.
+ */
+class EnforcePermissionDetector : Detector(), SourceCodeScanner {
+
+ override fun applicableAnnotations(): List<String> {
+ return listOf(ANNOTATION_ENFORCE_PERMISSION)
+ }
+
+ override fun getApplicableUastTypes(): List<Class<out UElement>> {
+ return listOf(UAnnotation::class.java)
+ }
+
+ private fun annotationValueGetChildren(elem: PsiElement): Array<PsiElement> {
+ if (elem is PsiArrayInitializerMemberValue)
+ return elem.getInitializers().map { it as PsiElement }.toTypedArray()
+ return elem.getChildren()
+ }
+
+ private fun areAnnotationsEquivalent(
+ context: JavaContext,
+ anno1: PsiAnnotation,
+ anno2: PsiAnnotation
+ ): Boolean {
+ if (anno1.qualifiedName != anno2.qualifiedName) {
+ return false
+ }
+ val attr1 = anno1.parameterList.attributes
+ val attr2 = anno2.parameterList.attributes
+ if (attr1.size != attr2.size) {
+ return false
+ }
+ for (i in attr1.indices) {
+ if (attr1[i].name != attr2[i].name) {
+ return false
+ }
+ val value1 = attr1[i].value ?: return false
+ val value2 = attr2[i].value ?: return false
+ // Try to compare values directly with each other.
+ val v1 = ConstantEvaluator.evaluate(context, value1)
+ val v2 = ConstantEvaluator.evaluate(context, value2)
+ if (v1 != null && v2 != null) {
+ if (v1 != v2) {
+ return false
+ }
+ } else {
+ val children1 = annotationValueGetChildren(value1)
+ val children2 = annotationValueGetChildren(value2)
+ if (children1.size != children2.size) {
+ return false
+ }
+ for (j in children1.indices) {
+ val c1 = ConstantEvaluator.evaluate(context, children1[j])
+ val c2 = ConstantEvaluator.evaluate(context, children2[j])
+ if (c1 != c2) {
+ return false
+ }
+ }
+ }
+ }
+ return true
+ }
+
+ private fun compareMethods(
+ context: JavaContext,
+ element: UElement,
+ overridingMethod: PsiMethod,
+ overriddenMethod: PsiMethod,
+ checkEquivalence: Boolean = true
+ ) {
+ // If method is not from a Stub subclass, this method shouldn't use @EP at all.
+ // This is handled by EnforcePermissionHelperDetector.
+ if (!isContainedInSubclassOfStub(context, overridingMethod.toUElement() as? UMethod)) {
+ return
+ }
+ val overridingAnnotation = overridingMethod.getAnnotation(ANNOTATION_ENFORCE_PERMISSION)
+ val overriddenAnnotation = overriddenMethod.getAnnotation(ANNOTATION_ENFORCE_PERMISSION)
+ val location = context.getLocation(element)
+ val overridingClass = overridingMethod.parent as PsiClass
+ val overriddenClass = overriddenMethod.parent as PsiClass
+ val overridingName = "${overridingClass.name}.${overridingMethod.name}"
+ val overriddenName = "${overriddenClass.name}.${overriddenMethod.name}"
+ if (overridingAnnotation == null) {
+ val msg = "The method $overridingName overrides the method $overriddenName which " +
+ "is annotated with @EnforcePermission. The same annotation must be used " +
+ "on $overridingName"
+ context.report(ISSUE_MISSING_ENFORCE_PERMISSION, element, location, msg)
+ } else if (overriddenAnnotation == null) {
+ val msg = "The method $overridingName overrides the method $overriddenName which " +
+ "is not annotated with @EnforcePermission. The same annotation must be " +
+ "used on $overriddenName. Did you forget to annotate the AIDL definition?"
+ context.report(ISSUE_MISSING_ENFORCE_PERMISSION, element, location, msg)
+ } else if (checkEquivalence && !areAnnotationsEquivalent(
+ context, overridingAnnotation, overriddenAnnotation)) {
+ val msg = "The method $overridingName is annotated with " +
+ "${overridingAnnotation.text} which differs from the overridden " +
+ "method $overriddenName: ${overriddenAnnotation.text}. The same " +
+ "annotation must be used for both methods."
+ context.report(ISSUE_MISMATCHING_ENFORCE_PERMISSION, element, location, msg)
+ }
+ }
+
+ override fun visitAnnotationUsage(
+ context: JavaContext,
+ element: UElement,
+ annotationInfo: AnnotationInfo,
+ usageInfo: AnnotationUsageInfo
+ ) {
+ if (usageInfo.type == AnnotationUsageType.METHOD_OVERRIDE &&
+ annotationInfo.origin == AnnotationOrigin.METHOD) {
+ val overridingMethod = element.sourcePsi as PsiMethod
+ val overriddenMethod = usageInfo.referenced as PsiMethod
+ compareMethods(context, element, overridingMethod, overriddenMethod)
+ }
+ }
+
+ override fun createUastHandler(context: JavaContext): UElementHandler {
+ return object : UElementHandler() {
+ override fun visitAnnotation(node: UAnnotation) {
+ if (node.qualifiedName != ANNOTATION_ENFORCE_PERMISSION) {
+ return
+ }
+ val method = node.uastParent as? UMethod ?: return
+ val overridingMethod = method as PsiMethod
+ val parents = overridingMethod.findSuperMethods()
+ for (overriddenMethod in parents) {
+ // The equivalence check can be skipped, if both methods are
+ // annotated, it will be verified by visitAnnotationUsage.
+ compareMethods(context, method, overridingMethod,
+ overriddenMethod, checkEquivalence = false)
+ }
+ }
+ }
+ }
+
+ companion object {
+ val EXPLANATION = """
+ The @EnforcePermission annotation is used to indicate that the underlying binder code
+ has already verified the caller's permissions before calling the appropriate method. The
+ verification code is usually generated by the AIDL compiler, which also takes care of
+ annotating the generated Java code.
+
+ In order to surface that information to platform developers, the same annotation must be
+ used on the implementation class or methods.
+ """
+
+ val ISSUE_MISSING_ENFORCE_PERMISSION: Issue = Issue.create(
+ id = "MissingEnforcePermissionAnnotation",
+ briefDescription = "Missing @EnforcePermission annotation on Binder method",
+ explanation = EXPLANATION,
+ category = Category.SECURITY,
+ priority = 6,
+ severity = Severity.ERROR,
+ implementation = Implementation(
+ EnforcePermissionDetector::class.java,
+ Scope.JAVA_FILE_SCOPE
+ )
+ )
+
+ val ISSUE_MISMATCHING_ENFORCE_PERMISSION: Issue = Issue.create(
+ id = "MismatchingEnforcePermissionAnnotation",
+ briefDescription = "Incorrect @EnforcePermission annotation on Binder method",
+ explanation = EXPLANATION,
+ category = Category.SECURITY,
+ priority = 6,
+ severity = Severity.ERROR,
+ implementation = Implementation(
+ EnforcePermissionDetector::class.java,
+ Scope.JAVA_FILE_SCOPE
+ )
+ )
+ }
+}
diff --git a/tools/lint/global/checks/src/main/java/com/google/android/lint/aidl/EnforcePermissionFix.kt b/tools/lint/global/checks/src/main/java/com/google/android/lint/aidl/EnforcePermissionFix.kt
new file mode 100644
index 0000000..25d208d
--- /dev/null
+++ b/tools/lint/global/checks/src/main/java/com/google/android/lint/aidl/EnforcePermissionFix.kt
@@ -0,0 +1,384 @@
+/*
+ * Copyright (C) 2022 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.JavaContext
+import com.android.tools.lint.detector.api.LintFix
+import com.android.tools.lint.detector.api.Location
+import com.android.tools.lint.detector.api.UastLintUtils.Companion.getAnnotationBooleanValue
+import com.android.tools.lint.detector.api.UastLintUtils.Companion.getAnnotationStringValues
+import com.android.tools.lint.detector.api.findSelector
+import com.android.tools.lint.detector.api.getUMethod
+import com.google.android.lint.findCallExpression
+import com.google.android.lint.getPermissionMethodAnnotation
+import com.google.android.lint.hasPermissionNameAnnotation
+import com.google.android.lint.isPermissionMethodCall
+import com.intellij.psi.PsiClassType
+import com.intellij.psi.PsiType
+import org.jetbrains.kotlin.psi.psiUtil.parameterIndex
+import org.jetbrains.uast.UBinaryExpression
+import org.jetbrains.uast.UBlockExpression
+import org.jetbrains.uast.UCallExpression
+import org.jetbrains.uast.UExpression
+import org.jetbrains.uast.UExpressionList
+import org.jetbrains.uast.UIfExpression
+import org.jetbrains.uast.UMethod
+import org.jetbrains.uast.UThrowExpression
+import org.jetbrains.uast.UastBinaryOperator
+import org.jetbrains.uast.evaluateString
+import org.jetbrains.uast.skipParenthesizedExprDown
+import org.jetbrains.uast.visitor.AbstractUastVisitor
+
+/**
+ * Helper class that facilitates the creation of lint auto fixes
+ */
+data class EnforcePermissionFix(
+ val manualCheckLocations: List<Location>,
+ val permissionNames: List<String>,
+ val errorLevel: Boolean,
+ val anyOf: Boolean,
+) {
+ fun toLintFix(context: JavaContext, node: UMethod): LintFix {
+ val methodLocation = context.getLocation(node)
+ val replaceOrRemoveFixes = manualCheckLocations.mapIndexed { index, manualCheckLocation ->
+ if (index == 0) {
+ // Replace the first manual check with a call to the helper method
+ getHelperMethodFix(node, manualCheckLocation, false)
+ } else {
+ // Remove all subsequent manual checks
+ LintFix.create()
+ .replace()
+ .reformat(true)
+ .range(manualCheckLocation)
+ .with("")
+ .autoFix()
+ .build()
+ }
+ }
+
+ // Annotate the method with @EnforcePermission(...)
+ val annotateFix = LintFix.create()
+ .annotate(annotation)
+ .range(methodLocation)
+ .autoFix()
+ .build()
+
+ return LintFix.create().composite(annotateFix, *replaceOrRemoveFixes.toTypedArray())
+ }
+
+ private val annotation: String
+ get() {
+ val quotedPermissions = permissionNames.joinToString(", ") { """"$it"""" }
+
+ val attributeName =
+ if (permissionNames.size > 1) {
+ if (anyOf) "anyOf" else "allOf"
+ } else null
+
+ val annotationParameter =
+ if (attributeName != null) "$attributeName={$quotedPermissions}"
+ else quotedPermissions
+
+ return "@$ANNOTATION_ENFORCE_PERMISSION($annotationParameter)"
+ }
+
+ companion object {
+ /**
+ * Walks the expressions in a block, looking for simple permission checks.
+ *
+ * As soon as something other than a permission check is encountered, stop looking,
+ * as some other business logic is happening that prevents an automated fix.
+ */
+ fun fromBlockExpression(
+ context: JavaContext,
+ blockExpression: UBlockExpression
+ ): EnforcePermissionFix? {
+ try {
+ val singleFixes = mutableListOf<EnforcePermissionFix>()
+ for (expression in blockExpression.expressions) {
+ val fix = fromExpression(context, expression) ?: break
+ singleFixes.add(fix)
+ }
+ return compose(singleFixes)
+ } catch (e: AnyOfAllOfException) {
+ return null
+ }
+ }
+
+ /**
+ * Conditionally constructs EnforcePermissionFix from any UExpression
+ *
+ * @return EnforcePermissionFix if the expression boils down to a permission check,
+ * else null
+ */
+ fun fromExpression(
+ context: JavaContext,
+ expression: UExpression
+ ): EnforcePermissionFix? {
+ val trimmedExpression = expression.skipParenthesizedExprDown()
+ if (trimmedExpression is UIfExpression) {
+ return fromIfExpression(context, trimmedExpression)
+ }
+ findCallExpression(trimmedExpression)?.let {
+ return fromCallExpression(context, it)
+ }
+ return null
+ }
+
+ /**
+ * Conditionally constructs EnforcePermissionFix from a UCallExpression
+ *
+ * @return EnforcePermissionFix if the called method is annotated with @PermissionMethod, else null
+ */
+ fun fromCallExpression(
+ context: JavaContext,
+ callExpression: UCallExpression
+ ): EnforcePermissionFix? {
+ val method = callExpression.resolve()?.getUMethod() ?: return null
+ val annotation = getPermissionMethodAnnotation(method) ?: return null
+ val returnsVoid = method.returnType == PsiType.VOID
+ val orSelf = getAnnotationBooleanValue(annotation, "orSelf") ?: false
+ val anyOf = getAnnotationBooleanValue(annotation, "anyOf") ?: false
+ return EnforcePermissionFix(
+ listOf(getPermissionCheckLocation(context, callExpression)),
+ getPermissionCheckValues(callExpression),
+ errorLevel = isErrorLevel(throws = returnsVoid, orSelf = orSelf),
+ anyOf,
+ )
+ }
+
+ /**
+ * Conditionally constructs EnforcePermissionFix from a UCallExpression
+ *
+ * @return EnforcePermissionFix IF AND ONLY IF:
+ * * The condition of the if statement compares the return value of a
+ * PermissionMethod to one of the PackageManager.PermissionResult values
+ * * The expression inside the if statement does nothing but throw SecurityException
+ */
+ fun fromIfExpression(
+ context: JavaContext,
+ ifExpression: UIfExpression
+ ): EnforcePermissionFix? {
+ val condition = ifExpression.condition.skipParenthesizedExprDown()
+ if (condition !is UBinaryExpression) return null
+
+ val maybeLeftCall = findCallExpression(condition.leftOperand)
+ val maybeRightCall = findCallExpression(condition.rightOperand)
+
+ val (callExpression, comparison) =
+ if (maybeLeftCall is UCallExpression) {
+ Pair(maybeLeftCall, condition.rightOperand)
+ } else if (maybeRightCall is UCallExpression) {
+ Pair(maybeRightCall, condition.leftOperand)
+ } else return null
+
+ val permissionMethodAnnotation = getPermissionMethodAnnotation(
+ callExpression.resolve()?.getUMethod()) ?: return null
+
+ val equalityCheck =
+ when (comparison.findSelector().asSourceString()
+ .filterNot(Char::isWhitespace)) {
+ "PERMISSION_GRANTED" -> UastBinaryOperator.IDENTITY_NOT_EQUALS
+ "PERMISSION_DENIED" -> UastBinaryOperator.IDENTITY_EQUALS
+ else -> return null
+ }
+
+ if (condition.operator != equalityCheck) return null
+
+ val throwExpression: UThrowExpression? =
+ ifExpression.thenExpression as? UThrowExpression
+ ?: (ifExpression.thenExpression as? UBlockExpression)
+ ?.expressions?.firstOrNull()
+ as? UThrowExpression
+
+
+ val thrownClass = (throwExpression?.thrownExpression?.getExpressionType()
+ as? PsiClassType)?.resolve() ?: return null
+ if (!context.evaluator.inheritsFrom(
+ thrownClass, "java.lang.SecurityException")){
+ return null
+ }
+
+ val orSelf = getAnnotationBooleanValue(permissionMethodAnnotation, "orSelf") ?: false
+ val anyOf = getAnnotationBooleanValue(permissionMethodAnnotation, "anyOf") ?: false
+
+ return EnforcePermissionFix(
+ listOf(context.getLocation(ifExpression)),
+ getPermissionCheckValues(callExpression),
+ errorLevel = isErrorLevel(throws = true, orSelf = orSelf),
+ anyOf = anyOf
+ )
+ }
+
+
+ fun compose(individuals: List<EnforcePermissionFix>): EnforcePermissionFix? {
+ if (individuals.isEmpty()) return null
+ val anyOfs = individuals.filter(EnforcePermissionFix::anyOf)
+ // anyOf/allOf should be consistent. If we encounter some @PermissionMethods that are anyOf
+ // and others that aren't, we don't know what to do.
+ if (anyOfs.isNotEmpty() && anyOfs.size < individuals.size) {
+ throw AnyOfAllOfException()
+ }
+ return EnforcePermissionFix(
+ individuals.flatMap(EnforcePermissionFix::manualCheckLocations),
+ individuals.flatMap(EnforcePermissionFix::permissionNames),
+ errorLevel = individuals.all(EnforcePermissionFix::errorLevel),
+ anyOf = anyOfs.isNotEmpty()
+ )
+ }
+
+ /**
+ * Given a permission check, get its proper location
+ * so that a lint fix can remove the entire expression
+ */
+ private fun getPermissionCheckLocation(
+ context: JavaContext,
+ callExpression: UCallExpression
+ ):
+ Location {
+ val javaPsi = callExpression.javaPsi!!
+ return Location.create(
+ context.file,
+ javaPsi.containingFile?.text,
+ javaPsi.textRange.startOffset,
+ // unfortunately the element doesn't include the ending semicolon
+ javaPsi.textRange.endOffset + 1
+ )
+ }
+
+ /**
+ * Given a @PermissionMethod, find arguments annotated with @PermissionName
+ * and pull out the permission value(s) being used. Also evaluates nested calls
+ * to @PermissionMethod(s) in the given method's body.
+ */
+ @Throws(AnyOfAllOfException::class)
+ private fun getPermissionCheckValues(
+ callExpression: UCallExpression
+ ): List<String> {
+ if (!isPermissionMethodCall(callExpression)) return emptyList()
+
+ val result = mutableSetOf<String>() // protect against duplicate permission values
+ val visitedCalls = mutableSetOf<UCallExpression>() // don't visit the same call twice
+ val bfsQueue = ArrayDeque(listOf(callExpression))
+
+ var anyOfAllOfState: AnyOfAllOfState = AnyOfAllOfState.INITIAL
+
+ // Bread First Search - evaluating nested @PermissionMethod(s) in the available
+ // source code for @PermissionName(s).
+ while (bfsQueue.isNotEmpty()) {
+ val currentCallExpression = bfsQueue.removeFirst()
+ visitedCalls.add(currentCallExpression)
+ val currentPermissions = findPermissions(currentCallExpression)
+ result.addAll(currentPermissions)
+
+ val currentAnnotation = getPermissionMethodAnnotation(
+ currentCallExpression.resolve()?.getUMethod())
+ val currentAnyOf = getAnnotationBooleanValue(currentAnnotation, "anyOf") ?: false
+
+ // anyOf/allOf should be consistent. If we encounter a nesting of @PermissionMethods
+ // where we start in an anyOf state and switch to allOf, or vice versa,
+ // we don't know what to do.
+ if (anyOfAllOfState == AnyOfAllOfState.INITIAL) {
+ if (currentAnyOf) anyOfAllOfState = AnyOfAllOfState.ANY_OF
+ else if (result.isNotEmpty()) anyOfAllOfState = AnyOfAllOfState.ALL_OF
+ }
+
+ if (anyOfAllOfState == AnyOfAllOfState.ALL_OF && currentAnyOf) {
+ throw AnyOfAllOfException()
+ }
+
+ if (anyOfAllOfState == AnyOfAllOfState.ANY_OF &&
+ !currentAnyOf && currentPermissions.size > 1) {
+ throw AnyOfAllOfException()
+ }
+
+ currentCallExpression.resolve()?.getUMethod()
+ ?.accept(PermissionCheckValuesVisitor(visitedCalls, bfsQueue))
+ }
+
+ return result.toList()
+ }
+
+ private enum class AnyOfAllOfState {
+ INITIAL,
+ ANY_OF,
+ ALL_OF
+ }
+
+ /**
+ * Adds visited permission method calls to the provided
+ * queue in support of the BFS traversal happening while
+ * this is used
+ */
+ private class PermissionCheckValuesVisitor(
+ val visitedCalls: Set<UCallExpression>,
+ val bfsQueue: ArrayDeque<UCallExpression>
+ ) : AbstractUastVisitor() {
+ override fun visitCallExpression(node: UCallExpression): Boolean {
+ if (isPermissionMethodCall(node) && node !in visitedCalls) {
+ bfsQueue.add(node)
+ }
+ return false
+ }
+ }
+
+ private fun findPermissions(
+ callExpression: UCallExpression,
+ ): List<String> {
+ val annotation = getPermissionMethodAnnotation(callExpression.resolve()?.getUMethod())
+
+ val hardCodedPermissions = (getAnnotationStringValues(annotation, "value")
+ ?: emptyArray())
+ .toList()
+
+ val indices = callExpression.resolve()?.getUMethod()
+ ?.uastParameters
+ ?.filter(::hasPermissionNameAnnotation)
+ ?.mapNotNull { it.sourcePsi?.parameterIndex() }
+ ?: emptyList()
+
+ val argPermissions = indices
+ .flatMap { i ->
+ when (val argument = callExpression.getArgumentForParameter(i)) {
+ null -> listOf(null)
+ is UExpressionList -> // varargs e.g. someMethod(String...)
+ argument.expressions.map(UExpression::evaluateString)
+ else -> listOf(argument.evaluateString())
+ }
+ }
+ .filterNotNull()
+
+ return hardCodedPermissions + argPermissions
+ }
+
+ /**
+ * If we detect that the PermissionMethod enforces that permission is granted,
+ * AND is of the "orSelf" variety, we are very confident that this is a behavior
+ * preserving migration to @EnforcePermission. Thus, the incident should be ERROR
+ * level.
+ */
+ private fun isErrorLevel(throws: Boolean, orSelf: Boolean): Boolean = throws && orSelf
+ }
+}
+/**
+ * anyOf/allOf @PermissionMethods must be consistent to apply @EnforcePermission -
+ * meaning if we encounter some @PermissionMethods that are anyOf, and others are allOf,
+ * we don't know which to apply.
+ */
+class AnyOfAllOfException : Exception() {
+ override val message: String = "anyOf/allOf permission methods cannot be mixed"
+}
diff --git a/tools/lint/global/checks/src/main/java/com/google/android/lint/aidl/EnforcePermissionHelperDetector.kt b/tools/lint/global/checks/src/main/java/com/google/android/lint/aidl/EnforcePermissionHelperDetector.kt
new file mode 100644
index 0000000..df13af5
--- /dev/null
+++ b/tools/lint/global/checks/src/main/java/com/google/android/lint/aidl/EnforcePermissionHelperDetector.kt
@@ -0,0 +1,149 @@
+/*
+ * Copyright (C) 2022 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.client.api.UElementHandler
+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.google.android.lint.findCallExpression
+import com.intellij.psi.PsiElement
+import org.jetbrains.uast.UBlockExpression
+import org.jetbrains.uast.UDeclarationsExpression
+import org.jetbrains.uast.UElement
+import org.jetbrains.uast.UExpression
+import org.jetbrains.uast.UMethod
+import org.jetbrains.uast.skipParenthesizedExprDown
+
+class EnforcePermissionHelperDetector : Detector(), SourceCodeScanner {
+ override fun getApplicableUastTypes(): List<Class<out UElement?>> =
+ listOf(UMethod::class.java)
+
+ override fun createUastHandler(context: JavaContext): UElementHandler = AidlStubHandler(context)
+
+ private inner class AidlStubHandler(val context: JavaContext) : UElementHandler() {
+ override fun visitMethod(node: UMethod) {
+ if (context.evaluator.isAbstract(node)) return
+ if (!node.hasAnnotation(ANNOTATION_ENFORCE_PERMISSION)) return
+
+ if (!isContainedInSubclassOfStub(context, node)) {
+ context.report(
+ ISSUE_MISUSING_ENFORCE_PERMISSION,
+ node,
+ context.getLocation(node),
+ "The class of ${node.name} does not inherit from an AIDL generated Stub class"
+ )
+ return
+ }
+
+ val targetExpression = getHelperMethodCallSourceString(node)
+ val message =
+ "Method must start with $targetExpression or super.${node.name}(), if applicable"
+
+ val firstExpression = (node.uastBody as? UBlockExpression)
+ ?.expressions?.firstOrNull()
+
+ if (firstExpression == null) {
+ context.report(
+ ISSUE_ENFORCE_PERMISSION_HELPER,
+ context.getLocation(node),
+ message,
+ )
+ return
+ }
+
+ val firstExpressionSource = firstExpression.skipParenthesizedExprDown()
+ .asSourceString()
+ .filterNot(Char::isWhitespace)
+
+ if (firstExpressionSource != targetExpression &&
+ firstExpressionSource != "super.$targetExpression") {
+ // calling super.<methodName>() is also legal
+ val directSuper = context.evaluator.getSuperMethod(node)
+ val firstCall = findCallExpression(firstExpression)?.resolve()
+ if (directSuper != null && firstCall == directSuper) return
+
+ val locationTarget = getLocationTarget(firstExpression)
+ val expressionLocation = context.getLocation(locationTarget)
+
+ context.report(
+ ISSUE_ENFORCE_PERMISSION_HELPER,
+ context.getLocation(node),
+ message,
+ getHelperMethodFix(node, expressionLocation),
+ )
+ }
+ }
+ }
+
+ companion object {
+ private const val HELPER_SUFFIX = "_enforcePermission"
+
+ private const val EXPLANATION = """
+ The @EnforcePermission annotation can only be used on methods whose class extends from
+ the Stub class generated by the AIDL compiler. When @EnforcePermission is applied, the
+ AIDL compiler generates a Stub method to do the permission check called yourMethodName$HELPER_SUFFIX.
+
+ yourMethodName$HELPER_SUFFIX must be executed before any other operation. To do that, you can
+ either call it directly, or call it indirectly via super.yourMethodName().
+ """
+
+ val ISSUE_ENFORCE_PERMISSION_HELPER: Issue = Issue.create(
+ id = "MissingEnforcePermissionHelper",
+ briefDescription = """Missing permission-enforcing method call in AIDL method
+ |annotated with @EnforcePermission""".trimMargin(),
+ explanation = EXPLANATION,
+ category = Category.SECURITY,
+ priority = 6,
+ severity = Severity.ERROR,
+ implementation = Implementation(
+ EnforcePermissionHelperDetector::class.java,
+ Scope.JAVA_FILE_SCOPE
+ )
+ )
+
+ val ISSUE_MISUSING_ENFORCE_PERMISSION: Issue = Issue.create(
+ id = "MisusingEnforcePermissionAnnotation",
+ briefDescription = "@EnforcePermission cannot be used here",
+ explanation = EXPLANATION,
+ category = Category.SECURITY,
+ priority = 6,
+ severity = Severity.ERROR,
+ implementation = Implementation(
+ EnforcePermissionDetector::class.java,
+ Scope.JAVA_FILE_SCOPE
+ )
+ )
+
+ /**
+ * handles an edge case with UDeclarationsExpression, where sourcePsi is null,
+ * resulting in an incorrect Location if used directly
+ */
+ private fun getLocationTarget(firstExpression: UExpression): PsiElement? {
+ if (firstExpression.sourcePsi != null) return firstExpression.sourcePsi
+ if (firstExpression is UDeclarationsExpression) {
+ return firstExpression.declarations.firstOrNull()?.sourcePsi
+ }
+ return null
+ }
+ }
+}
diff --git a/tools/lint/global/checks/src/main/java/com/google/android/lint/aidl/EnforcePermissionUtils.kt b/tools/lint/global/checks/src/main/java/com/google/android/lint/aidl/EnforcePermissionUtils.kt
new file mode 100644
index 0000000..d41fee3
--- /dev/null
+++ b/tools/lint/global/checks/src/main/java/com/google/android/lint/aidl/EnforcePermissionUtils.kt
@@ -0,0 +1,96 @@
+/*
+ * Copyright (C) 2022 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.JavaContext
+import com.android.tools.lint.detector.api.LintFix
+import com.android.tools.lint.detector.api.Location
+import com.intellij.psi.PsiClass
+import com.intellij.psi.PsiReferenceList
+import org.jetbrains.uast.UMethod
+
+/**
+ * Given a UMethod, determine if this method is
+ * the entrypoint to an interface generated by AIDL,
+ * returning the interface name if so, otherwise returning null
+ */
+fun getContainingAidlInterface(context: JavaContext, node: UMethod): String? {
+ if (!isContainedInSubclassOfStub(context, node)) return null
+ for (superMethod in node.findSuperMethods()) {
+ for (extendsInterface in superMethod.containingClass?.extendsList?.referenceElements
+ ?: continue) {
+ if (extendsInterface.qualifiedName == IINTERFACE_INTERFACE) {
+ return superMethod.containingClass?.name
+ }
+ }
+ }
+ return null
+}
+
+fun isContainedInSubclassOfStub(context: JavaContext, node: UMethod?): Boolean {
+ var superClass = node?.containingClass?.superClass
+ while (superClass != null) {
+ if (isStub(context, superClass)) return true
+ superClass = superClass.superClass
+ }
+ return false
+}
+
+fun isStub(context: JavaContext, psiClass: PsiClass?): Boolean {
+ if (psiClass == null) return false
+ if (psiClass.name != "Stub") return false
+ if (!context.evaluator.isStatic(psiClass)) return false
+ if (!context.evaluator.isAbstract(psiClass)) return false
+
+ if (!hasSingleAncestor(psiClass.extendsList, BINDER_CLASS)) return false
+
+ val parent = psiClass.parent as? PsiClass ?: return false
+ if (!hasSingleAncestor(parent.extendsList, IINTERFACE_INTERFACE)) return false
+
+ val parentName = parent.qualifiedName ?: return false
+ if (!hasSingleAncestor(psiClass.implementsList, parentName)) return false
+
+ return true
+}
+
+private fun hasSingleAncestor(references: PsiReferenceList?, qualifiedName: String) =
+ references != null &&
+ references.referenceElements.size == 1 &&
+ references.referenceElements[0].qualifiedName == qualifiedName
+
+fun getHelperMethodCallSourceString(node: UMethod) = "${node.name}$AIDL_PERMISSION_HELPER_SUFFIX()"
+
+fun getHelperMethodFix(
+ node: UMethod,
+ manualCheckLocation: Location,
+ prepend: Boolean = true
+): LintFix {
+ val helperMethodSource = getHelperMethodCallSourceString(node)
+ val indent = " ".repeat(manualCheckLocation.start?.column ?: 0)
+ val newText = "$helperMethodSource;${if (prepend) "\n\n$indent" else ""}"
+
+ val fix = LintFix.create()
+ .replace()
+ .range(manualCheckLocation)
+ .with(newText)
+ .reformat(true)
+ .autoFix()
+
+ if (prepend) fix.beginning()
+
+ return fix.build()
+}
diff --git a/tools/lint/global/checks/src/main/java/com/google/android/lint/aidl/SimpleManualPermissionEnforcementDetector.kt b/tools/lint/global/checks/src/main/java/com/google/android/lint/aidl/SimpleManualPermissionEnforcementDetector.kt
new file mode 100644
index 0000000..c7be36e
--- /dev/null
+++ b/tools/lint/global/checks/src/main/java/com/google/android/lint/aidl/SimpleManualPermissionEnforcementDetector.kt
@@ -0,0 +1,92 @@
+/*
+ * Copyright (C) 2022 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.Incident
+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
+
+/**
+ * Looks for methods implementing generated AIDL interface stubs
+ * that can have simple permission checks migrated to
+ * @EnforcePermission annotations
+ */
+@Suppress("UnstableApiUsage")
+class SimpleManualPermissionEnforcementDetector : AidlImplementationDetector() {
+ override fun visitAidlMethod(
+ context: JavaContext,
+ node: UMethod,
+ interfaceName: String,
+ body: UBlockExpression
+ ) {
+ val enforcePermissionFix = EnforcePermissionFix.fromBlockExpression(context, body) ?: return
+ val lintFix = enforcePermissionFix.toLintFix(context, node)
+ val message =
+ "$interfaceName permission check ${
+ if (enforcePermissionFix.errorLevel) "should" else "can"
+ } be converted to @EnforcePermission annotation"
+
+ val incident = Incident(
+ ISSUE_SIMPLE_MANUAL_PERMISSION_ENFORCEMENT,
+ enforcePermissionFix.manualCheckLocations.last(),
+ message,
+ lintFix
+ )
+
+ // TODO(b/265014041): turn on errors once all code that would cause one is fixed
+ // if (enforcePermissionFix.errorLevel) {
+ // incident.overrideSeverity(Severity.ERROR)
+ // }
+
+ context.report(incident)
+ }
+
+ companion object {
+
+ private val EXPLANATION = """
+ Whenever possible, method implementations of AIDL interfaces should use the @EnforcePermission
+ annotation to declare the permissions to be enforced. The verification code is then
+ generated by the AIDL compiler, which also takes care of annotating the generated java
+ code.
+
+ This reduces the risk of bugs around these permission checks (that often become vulnerabilities).
+ It also enables easier auditing and review.
+
+ Please migrate to an @EnforcePermission annotation. (See: go/aidl-enforce-howto)
+ """.trimIndent()
+
+ @JvmField
+ val ISSUE_SIMPLE_MANUAL_PERMISSION_ENFORCEMENT = Issue.create(
+ id = "SimpleManualPermissionEnforcement",
+ briefDescription = "Manual permission check can be @EnforcePermission annotation",
+ explanation = EXPLANATION,
+ category = Category.SECURITY,
+ priority = 5,
+ severity = Severity.WARNING,
+ implementation = Implementation(
+ SimpleManualPermissionEnforcementDetector::class.java,
+ Scope.JAVA_FILE_SCOPE
+ ),
+ )
+ }
+}
diff --git a/tools/lint/global/checks/src/test/java/com/google/android/lint/aidl/EnforcePermissionDetectorCodegenTest.kt b/tools/lint/global/checks/src/test/java/com/google/android/lint/aidl/EnforcePermissionDetectorCodegenTest.kt
new file mode 100644
index 0000000..f2930d9
--- /dev/null
+++ b/tools/lint/global/checks/src/test/java/com/google/android/lint/aidl/EnforcePermissionDetectorCodegenTest.kt
@@ -0,0 +1,557 @@
+/*
+ * Copyright (C) 2022 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 EnforcePermissionDetectorCodegenTest : LintDetectorTest() {
+ override fun getDetector(): Detector = EnforcePermissionDetector()
+
+ override fun getIssues(): List<Issue> = listOf(
+ EnforcePermissionDetector.ISSUE_MISSING_ENFORCE_PERMISSION,
+ EnforcePermissionDetector.ISSUE_MISMATCHING_ENFORCE_PERMISSION
+ )
+
+ override fun lint(): TestLintTask = super.lint().allowMissingSdk(true)
+
+ fun test_generated_IProtected() {
+ lint().files(
+ java(
+ """
+ /*
+ * This file is auto-generated. DO NOT MODIFY.
+ */
+ package android.aidl.tests.permission;
+ public interface IProtected extends android.os.IInterface
+ {
+ /** Default implementation for IProtected. */
+ public static class Default implements android.aidl.tests.permission.IProtected
+ {
+ @Override public void PermissionProtected() throws android.os.RemoteException
+ {
+ }
+ @Override public void MultiplePermissionsAll() throws android.os.RemoteException
+ {
+ }
+ @Override public void MultiplePermissionsAny() throws android.os.RemoteException
+ {
+ }
+ @Override public void NonManifestPermission() throws android.os.RemoteException
+ {
+ }
+ // Used by the integration tests to dynamically set permissions that are considered granted.
+ @Override public void SetGranted(java.util.List<java.lang.String> permissions) throws android.os.RemoteException
+ {
+ }
+ @Override
+ public android.os.IBinder asBinder() {
+ return null;
+ }
+ }
+ /** Local-side IPC implementation stub class. */
+ public static abstract class Stub extends android.os.Binder implements android.aidl.tests.permission.IProtected
+ {
+ private final android.os.PermissionEnforcer mEnforcer;
+ /** Construct the stub using the Enforcer provided. */
+ public Stub(android.os.PermissionEnforcer enforcer)
+ {
+ this.attachInterface(this, DESCRIPTOR);
+ if (enforcer == null) {
+ throw new IllegalArgumentException("enforcer cannot be null");
+ }
+ mEnforcer = enforcer;
+ }
+ @Deprecated
+ /** Default constructor. */
+ public Stub() {
+ this(android.os.PermissionEnforcer.fromContext(
+ android.app.ActivityThread.currentActivityThread().getSystemContext()));
+ }
+ /**
+ * Cast an IBinder object into an android.aidl.tests.permission.IProtected interface,
+ * generating a proxy if needed.
+ */
+ public static android.aidl.tests.permission.IProtected asInterface(android.os.IBinder obj)
+ {
+ if ((obj==null)) {
+ return null;
+ }
+ android.os.IInterface iin = obj.queryLocalInterface(DESCRIPTOR);
+ if (((iin!=null)&&(iin instanceof android.aidl.tests.permission.IProtected))) {
+ return ((android.aidl.tests.permission.IProtected)iin);
+ }
+ return new android.aidl.tests.permission.IProtected.Stub.Proxy(obj);
+ }
+ @Override public android.os.IBinder asBinder()
+ {
+ return this;
+ }
+ /** @hide */
+ public static java.lang.String getDefaultTransactionName(int transactionCode)
+ {
+ switch (transactionCode)
+ {
+ case TRANSACTION_PermissionProtected:
+ {
+ return "PermissionProtected";
+ }
+ case TRANSACTION_MultiplePermissionsAll:
+ {
+ return "MultiplePermissionsAll";
+ }
+ case TRANSACTION_MultiplePermissionsAny:
+ {
+ return "MultiplePermissionsAny";
+ }
+ case TRANSACTION_NonManifestPermission:
+ {
+ return "NonManifestPermission";
+ }
+ case TRANSACTION_SetGranted:
+ {
+ return "SetGranted";
+ }
+ default:
+ {
+ return null;
+ }
+ }
+ }
+ /** @hide */
+ public java.lang.String getTransactionName(int transactionCode)
+ {
+ return this.getDefaultTransactionName(transactionCode);
+ }
+ @Override public boolean onTransact(int code, android.os.Parcel data, android.os.Parcel reply, int flags) throws android.os.RemoteException
+ {
+ java.lang.String descriptor = DESCRIPTOR;
+ if (code >= android.os.IBinder.FIRST_CALL_TRANSACTION && code <= android.os.IBinder.LAST_CALL_TRANSACTION) {
+ data.enforceInterface(descriptor);
+ }
+ switch (code)
+ {
+ case INTERFACE_TRANSACTION:
+ {
+ reply.writeString(descriptor);
+ return true;
+ }
+ }
+ switch (code)
+ {
+ case TRANSACTION_PermissionProtected:
+ {
+ this.PermissionProtected();
+ reply.writeNoException();
+ break;
+ }
+ case TRANSACTION_MultiplePermissionsAll:
+ {
+ this.MultiplePermissionsAll();
+ reply.writeNoException();
+ break;
+ }
+ case TRANSACTION_MultiplePermissionsAny:
+ {
+ this.MultiplePermissionsAny();
+ reply.writeNoException();
+ break;
+ }
+ case TRANSACTION_NonManifestPermission:
+ {
+ this.NonManifestPermission();
+ reply.writeNoException();
+ break;
+ }
+ case TRANSACTION_SetGranted:
+ {
+ java.util.List<java.lang.String> _arg0;
+ _arg0 = data.createStringArrayList();
+ data.enforceNoDataAvail();
+ this.SetGranted(_arg0);
+ reply.writeNoException();
+ break;
+ }
+ default:
+ {
+ return super.onTransact(code, data, reply, flags);
+ }
+ }
+ return true;
+ }
+ private static class Proxy implements android.aidl.tests.permission.IProtected
+ {
+ private android.os.IBinder mRemote;
+ Proxy(android.os.IBinder remote)
+ {
+ mRemote = remote;
+ }
+ @Override public android.os.IBinder asBinder()
+ {
+ return mRemote;
+ }
+ public java.lang.String getInterfaceDescriptor()
+ {
+ return DESCRIPTOR;
+ }
+ @Override public void PermissionProtected() throws android.os.RemoteException
+ {
+ android.os.Parcel _data = android.os.Parcel.obtain(asBinder());
+ android.os.Parcel _reply = android.os.Parcel.obtain();
+ try {
+ _data.writeInterfaceToken(DESCRIPTOR);
+ boolean _status = mRemote.transact(Stub.TRANSACTION_PermissionProtected, _data, _reply, 0);
+ _reply.readException();
+ }
+ finally {
+ _reply.recycle();
+ _data.recycle();
+ }
+ }
+ @Override public void MultiplePermissionsAll() throws android.os.RemoteException
+ {
+ android.os.Parcel _data = android.os.Parcel.obtain(asBinder());
+ android.os.Parcel _reply = android.os.Parcel.obtain();
+ try {
+ _data.writeInterfaceToken(DESCRIPTOR);
+ boolean _status = mRemote.transact(Stub.TRANSACTION_MultiplePermissionsAll, _data, _reply, 0);
+ _reply.readException();
+ }
+ finally {
+ _reply.recycle();
+ _data.recycle();
+ }
+ }
+ @Override public void MultiplePermissionsAny() throws android.os.RemoteException
+ {
+ android.os.Parcel _data = android.os.Parcel.obtain(asBinder());
+ android.os.Parcel _reply = android.os.Parcel.obtain();
+ try {
+ _data.writeInterfaceToken(DESCRIPTOR);
+ boolean _status = mRemote.transact(Stub.TRANSACTION_MultiplePermissionsAny, _data, _reply, 0);
+ _reply.readException();
+ }
+ finally {
+ _reply.recycle();
+ _data.recycle();
+ }
+ }
+ @Override public void NonManifestPermission() throws android.os.RemoteException
+ {
+ android.os.Parcel _data = android.os.Parcel.obtain(asBinder());
+ android.os.Parcel _reply = android.os.Parcel.obtain();
+ try {
+ _data.writeInterfaceToken(DESCRIPTOR);
+ boolean _status = mRemote.transact(Stub.TRANSACTION_NonManifestPermission, _data, _reply, 0);
+ _reply.readException();
+ }
+ finally {
+ _reply.recycle();
+ _data.recycle();
+ }
+ }
+ // Used by the integration tests to dynamically set permissions that are considered granted.
+ @Override public void SetGranted(java.util.List<java.lang.String> permissions) throws android.os.RemoteException
+ {
+ android.os.Parcel _data = android.os.Parcel.obtain(asBinder());
+ android.os.Parcel _reply = android.os.Parcel.obtain();
+ try {
+ _data.writeInterfaceToken(DESCRIPTOR);
+ _data.writeStringList(permissions);
+ boolean _status = mRemote.transact(Stub.TRANSACTION_SetGranted, _data, _reply, 0);
+ _reply.readException();
+ }
+ finally {
+ _reply.recycle();
+ _data.recycle();
+ }
+ }
+ }
+ static final int TRANSACTION_PermissionProtected = (android.os.IBinder.FIRST_CALL_TRANSACTION + 0);
+ /** Helper method to enforce permissions for PermissionProtected */
+ protected void PermissionProtected_enforcePermission() throws SecurityException {
+ android.content.AttributionSource source = new android.content.AttributionSource(getCallingUid(), null, null);
+ mEnforcer.enforcePermission(android.Manifest.permission.READ_PHONE_STATE, source);
+ }
+ static final int TRANSACTION_MultiplePermissionsAll = (android.os.IBinder.FIRST_CALL_TRANSACTION + 1);
+ /** Helper method to enforce permissions for MultiplePermissionsAll */
+ protected void MultiplePermissionsAll_enforcePermission() throws SecurityException {
+ android.content.AttributionSource source = new android.content.AttributionSource(getCallingUid(), null, null);
+ mEnforcer.enforcePermissionAllOf(new String[]{android.Manifest.permission.INTERNET, android.Manifest.permission.VIBRATE}, source);
+ }
+ static final int TRANSACTION_MultiplePermissionsAny = (android.os.IBinder.FIRST_CALL_TRANSACTION + 2);
+ /** Helper method to enforce permissions for MultiplePermissionsAny */
+ protected void MultiplePermissionsAny_enforcePermission() throws SecurityException {
+ android.content.AttributionSource source = new android.content.AttributionSource(getCallingUid(), null, null);
+ mEnforcer.enforcePermissionAnyOf(new String[]{android.Manifest.permission.INTERNET, android.Manifest.permission.VIBRATE}, source);
+ }
+ static final int TRANSACTION_NonManifestPermission = (android.os.IBinder.FIRST_CALL_TRANSACTION + 3);
+ /** Helper method to enforce permissions for NonManifestPermission */
+ protected void NonManifestPermission_enforcePermission() throws SecurityException {
+ android.content.AttributionSource source = new android.content.AttributionSource(getCallingUid(), null, null);
+ mEnforcer.enforcePermission(android.net.NetworkStack.PERMISSION_MAINLINE_NETWORK_STACK, source);
+ }
+ static final int TRANSACTION_SetGranted = (android.os.IBinder.FIRST_CALL_TRANSACTION + 4);
+ /** @hide */
+ public int getMaxTransactionId()
+ {
+ return 4;
+ }
+ }
+
+ @android.annotation.EnforcePermission(android.Manifest.permission.READ_PHONE_STATE)
+ public void PermissionProtected() throws android.os.RemoteException;
+ @android.annotation.EnforcePermission(allOf = {android.Manifest.permission.INTERNET, android.Manifest.permission.VIBRATE})
+ public void MultiplePermissionsAll() throws android.os.RemoteException;
+ @android.annotation.EnforcePermission(anyOf = {android.Manifest.permission.INTERNET, android.Manifest.permission.VIBRATE})
+ public void MultiplePermissionsAny() throws android.os.RemoteException;
+ @android.annotation.EnforcePermission(android.net.NetworkStack.PERMISSION_MAINLINE_NETWORK_STACK)
+ public void NonManifestPermission() throws android.os.RemoteException;
+ // Used by the integration tests to dynamically set permissions that are considered granted.
+ @android.annotation.RequiresNoPermission
+ public void SetGranted(java.util.List<java.lang.String> permissions) throws android.os.RemoteException;
+ }
+ """
+ ).indented(),
+ *stubs
+ )
+ .run()
+ .expectClean()
+ }
+
+ fun test_generated_IProtectedInterface() {
+ lint().files(
+ java(
+ """
+ /*
+ * This file is auto-generated. DO NOT MODIFY.
+ */
+ package android.aidl.tests.permission;
+ public interface IProtectedInterface extends android.os.IInterface
+ {
+ /** Default implementation for IProtectedInterface. */
+ public static class Default implements android.aidl.tests.permission.IProtectedInterface
+ {
+ @Override public void Method1() throws android.os.RemoteException
+ {
+ }
+ @Override public void Method2() throws android.os.RemoteException
+ {
+ }
+ @Override
+ public android.os.IBinder asBinder() {
+ return null;
+ }
+ }
+ /** Local-side IPC implementation stub class. */
+ public static abstract class Stub extends android.os.Binder implements android.aidl.tests.permission.IProtectedInterface
+ {
+ private final android.os.PermissionEnforcer mEnforcer;
+ /** Construct the stub using the Enforcer provided. */
+ public Stub(android.os.PermissionEnforcer enforcer)
+ {
+ this.attachInterface(this, DESCRIPTOR);
+ if (enforcer == null) {
+ throw new IllegalArgumentException("enforcer cannot be null");
+ }
+ mEnforcer = enforcer;
+ }
+ @Deprecated
+ /** Default constructor. */
+ public Stub() {
+ this(android.os.PermissionEnforcer.fromContext(
+ android.app.ActivityThread.currentActivityThread().getSystemContext()));
+ }
+ /**
+ * Cast an IBinder object into an android.aidl.tests.permission.IProtectedInterface interface,
+ * generating a proxy if needed.
+ */
+ public static android.aidl.tests.permission.IProtectedInterface asInterface(android.os.IBinder obj)
+ {
+ if ((obj==null)) {
+ return null;
+ }
+ android.os.IInterface iin = obj.queryLocalInterface(DESCRIPTOR);
+ if (((iin!=null)&&(iin instanceof android.aidl.tests.permission.IProtectedInterface))) {
+ return ((android.aidl.tests.permission.IProtectedInterface)iin);
+ }
+ return new android.aidl.tests.permission.IProtectedInterface.Stub.Proxy(obj);
+ }
+ @Override public android.os.IBinder asBinder()
+ {
+ return this;
+ }
+ /** @hide */
+ public static java.lang.String getDefaultTransactionName(int transactionCode)
+ {
+ switch (transactionCode)
+ {
+ case TRANSACTION_Method1:
+ {
+ return "Method1";
+ }
+ case TRANSACTION_Method2:
+ {
+ return "Method2";
+ }
+ default:
+ {
+ return null;
+ }
+ }
+ }
+ /** @hide */
+ public java.lang.String getTransactionName(int transactionCode)
+ {
+ return this.getDefaultTransactionName(transactionCode);
+ }
+ @Override public boolean onTransact(int code, android.os.Parcel data, android.os.Parcel reply, int flags) throws android.os.RemoteException
+ {
+ java.lang.String descriptor = DESCRIPTOR;
+ if (code >= android.os.IBinder.FIRST_CALL_TRANSACTION && code <= android.os.IBinder.LAST_CALL_TRANSACTION) {
+ data.enforceInterface(descriptor);
+ }
+ switch (code)
+ {
+ case INTERFACE_TRANSACTION:
+ {
+ reply.writeString(descriptor);
+ return true;
+ }
+ }
+ switch (code)
+ {
+ case TRANSACTION_Method1:
+ {
+ this.Method1();
+ reply.writeNoException();
+ break;
+ }
+ case TRANSACTION_Method2:
+ {
+ this.Method2();
+ reply.writeNoException();
+ break;
+ }
+ default:
+ {
+ return super.onTransact(code, data, reply, flags);
+ }
+ }
+ return true;
+ }
+ private static class Proxy implements android.aidl.tests.permission.IProtectedInterface
+ {
+ private android.os.IBinder mRemote;
+ Proxy(android.os.IBinder remote)
+ {
+ mRemote = remote;
+ }
+ @Override public android.os.IBinder asBinder()
+ {
+ return mRemote;
+ }
+ public java.lang.String getInterfaceDescriptor()
+ {
+ return DESCRIPTOR;
+ }
+ @Override public void Method1() throws android.os.RemoteException
+ {
+ android.os.Parcel _data = android.os.Parcel.obtain(asBinder());
+ android.os.Parcel _reply = android.os.Parcel.obtain();
+ try {
+ _data.writeInterfaceToken(DESCRIPTOR);
+ boolean _status = mRemote.transact(Stub.TRANSACTION_Method1, _data, _reply, 0);
+ _reply.readException();
+ }
+ finally {
+ _reply.recycle();
+ _data.recycle();
+ }
+ }
+ @Override public void Method2() throws android.os.RemoteException
+ {
+ android.os.Parcel _data = android.os.Parcel.obtain(asBinder());
+ android.os.Parcel _reply = android.os.Parcel.obtain();
+ try {
+ _data.writeInterfaceToken(DESCRIPTOR);
+ boolean _status = mRemote.transact(Stub.TRANSACTION_Method2, _data, _reply, 0);
+ _reply.readException();
+ }
+ finally {
+ _reply.recycle();
+ _data.recycle();
+ }
+ }
+ }
+ static final int TRANSACTION_Method1 = (android.os.IBinder.FIRST_CALL_TRANSACTION + 0);
+ /** Helper method to enforce permissions for Method1 */
+ protected void Method1_enforcePermission() throws SecurityException {
+ android.content.AttributionSource source = new android.content.AttributionSource(getCallingUid(), null, null);
+ mEnforcer.enforcePermission(android.Manifest.permission.ACCESS_FINE_LOCATION, source);
+ }
+ static final int TRANSACTION_Method2 = (android.os.IBinder.FIRST_CALL_TRANSACTION + 1);
+ /** Helper method to enforce permissions for Method2 */
+ protected void Method2_enforcePermission() throws SecurityException {
+ android.content.AttributionSource source = new android.content.AttributionSource(getCallingUid(), null, null);
+ mEnforcer.enforcePermission(android.Manifest.permission.ACCESS_FINE_LOCATION, source);
+ }
+ /** @hide */
+ public int getMaxTransactionId()
+ {
+ return 1;
+ }
+ }
+
+ @android.annotation.EnforcePermission(android.Manifest.permission.ACCESS_FINE_LOCATION)
+ public void Method1() throws android.os.RemoteException;
+ @android.annotation.EnforcePermission(android.Manifest.permission.ACCESS_FINE_LOCATION)
+ public void Method2() throws android.os.RemoteException;
+ }
+ """
+ ).indented(),
+ *stubs
+ )
+ .run()
+ .expectClean()
+ }
+
+ /* Stubs */
+
+ private val manifestPermissionStub: TestFile = java(
+ """
+ package android.Manifest;
+ class permission {
+ public static final String READ_PHONE_STATE = "android.permission.READ_PHONE_STATE";
+ public static final String INTERNET = "android.permission.INTERNET";
+ }
+ """
+ ).indented()
+
+ private val enforcePermissionAnnotationStub: TestFile = java(
+ """
+ package android.annotation;
+ public @interface EnforcePermission {}
+ """
+ ).indented()
+
+ private val stubs = arrayOf(manifestPermissionStub, enforcePermissionAnnotationStub)
+}
diff --git a/tools/lint/global/checks/src/test/java/com/google/android/lint/aidl/EnforcePermissionDetectorTest.kt b/tools/lint/global/checks/src/test/java/com/google/android/lint/aidl/EnforcePermissionDetectorTest.kt
new file mode 100644
index 0000000..75b0073
--- /dev/null
+++ b/tools/lint/global/checks/src/test/java/com/google/android/lint/aidl/EnforcePermissionDetectorTest.kt
@@ -0,0 +1,425 @@
+/*
+ * Copyright (C) 2022 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 EnforcePermissionDetectorTest : LintDetectorTest() {
+ override fun getDetector(): Detector = EnforcePermissionDetector()
+
+ override fun getIssues(): List<Issue> = listOf(
+ EnforcePermissionDetector.ISSUE_MISSING_ENFORCE_PERMISSION,
+ EnforcePermissionDetector.ISSUE_MISMATCHING_ENFORCE_PERMISSION
+ )
+
+ override fun lint(): TestLintTask = super.lint().allowMissingSdk(true)
+
+ fun testDoesNotDetectIssuesCorrectAnnotationOnMethod() {
+ lint().files(java(
+ """
+ package test.pkg;
+ import android.annotation.EnforcePermission;
+ public class TestClass2 extends IFooMethod.Stub {
+ @Override
+ @EnforcePermission(android.Manifest.permission.READ_PHONE_STATE)
+ public void testMethod() {}
+ }
+ """).indented(),
+ *stubs
+ )
+ .run()
+ .expectClean()
+ }
+
+ fun testDoesNotDetectIssuesCorrectAnnotationAllOnMethod() {
+ lint().files(java(
+ """
+ package test.pkg;
+ import android.annotation.EnforcePermission;
+ public class TestClass11 extends IFooMethod.Stub {
+ @Override
+ @EnforcePermission(allOf={android.Manifest.permission.INTERNET, android.Manifest.permission.READ_PHONE_STATE})
+ public void testMethodAll() {}
+ }
+ """).indented(),
+ *stubs
+ )
+ .run()
+ .expectClean()
+ }
+
+ fun testDoesNotDetectIssuesCorrectAnnotationAllLiteralOnMethod() {
+ lint().files(java(
+ """
+ package test.pkg;
+ import android.annotation.EnforcePermission;
+ public class TestClass111 extends IFooMethod.Stub {
+ @Override
+ @EnforcePermission(allOf={"android.permission.INTERNET", android.Manifest.permission.READ_PHONE_STATE})
+ public void testMethodAllLiteral() {}
+ }
+ """).indented(),
+ *stubs
+ )
+ .run()
+ .expectClean()
+ }
+
+ fun testDoesNotDetectIssuesCorrectAnnotationAnyOnMethod() {
+ lint().files(java(
+ """
+ package test.pkg;
+ import android.annotation.EnforcePermission;
+ public class TestClass12 extends IFooMethod.Stub {
+ @Override
+ @EnforcePermission(anyOf={android.Manifest.permission.INTERNET, android.Manifest.permission.READ_PHONE_STATE})
+ public void testMethodAny() {}
+ }
+ """).indented(),
+ *stubs
+ )
+ .run()
+ .expectClean()
+ }
+
+ fun testDoesNotDetectIssuesCorrectAnnotationAnyLiteralOnMethod() {
+ lint().files(java(
+ """
+ package test.pkg;
+ import android.annotation.EnforcePermission;
+ public class TestClass121 extends IFooMethod.Stub {
+ @Override
+ @EnforcePermission(anyOf={"android.permission.INTERNET", android.Manifest.permission.READ_PHONE_STATE})
+ public void testMethodAnyLiteral() {}
+ }
+ """).indented(),
+ *stubs
+ )
+ .run()
+ .expectClean()
+ }
+
+ fun testDetectIssuesMismatchingAnnotationOnMethod() {
+ lint().files(java(
+ """
+ package test.pkg;
+ public class TestClass4 extends IFooMethod.Stub {
+ @android.annotation.EnforcePermission(android.Manifest.permission.INTERNET)
+ public void testMethod() {}
+ }
+ """).indented(),
+ *stubs
+ )
+ .run()
+ .expect("""
+ src/test/pkg/TestClass4.java:4: Error: The method TestClass4.testMethod is annotated with @android.annotation.EnforcePermission(android.Manifest.permission.INTERNET) \
+ which differs from the overridden method Stub.testMethod: @android.annotation.EnforcePermission(android.Manifest.permission.READ_PHONE_STATE). \
+ The same annotation must be used for both methods. [MismatchingEnforcePermissionAnnotation]
+ public void testMethod() {}
+ ~~~~~~~~~~
+ 1 errors, 0 warnings
+ """.addLineContinuation())
+ }
+
+ fun testDetectIssuesEmptyAnnotationOnMethod() {
+ lint().files(java(
+ """
+ package test.pkg;
+ public class TestClass41 extends IFooMethod.Stub {
+ @android.annotation.EnforcePermission
+ public void testMethod() {}
+ }
+ """).indented(),
+ *stubs
+ )
+ .run()
+ .expect("""
+ src/test/pkg/TestClass41.java:4: Error: The method TestClass41.testMethod is annotated with @android.annotation.EnforcePermission \
+ which differs from the overridden method Stub.testMethod: @android.annotation.EnforcePermission(android.Manifest.permission.READ_PHONE_STATE). \
+ The same annotation must be used for both methods. [MismatchingEnforcePermissionAnnotation]
+ public void testMethod() {}
+ ~~~~~~~~~~
+ 1 errors, 0 warnings
+ """.addLineContinuation())
+ }
+
+ fun testDetectIssuesMismatchingAnyAnnotationOnMethod() {
+ lint().files(java(
+ """
+ package test.pkg;
+ public class TestClass9 extends IFooMethod.Stub {
+ @android.annotation.EnforcePermission(anyOf={android.Manifest.permission.INTERNET, android.Manifest.permission.NFC})
+ public void testMethodAny() {}
+ }
+ """).indented(),
+ *stubs
+ )
+ .run()
+ .expect("""
+ src/test/pkg/TestClass9.java:4: Error: The method TestClass9.testMethodAny is annotated with \
+ @android.annotation.EnforcePermission(anyOf={android.Manifest.permission.INTERNET, android.Manifest.permission.NFC}) \
+ which differs from the overridden method Stub.testMethodAny: \
+ @android.annotation.EnforcePermission(anyOf={android.Manifest.permission.INTERNET, android.Manifest.permission.READ_PHONE_STATE}). \
+ The same annotation must be used for both methods. [MismatchingEnforcePermissionAnnotation]
+ public void testMethodAny() {}
+ ~~~~~~~~~~~~~
+ 1 errors, 0 warnings
+ """.addLineContinuation())
+ }
+
+ fun testDetectIssuesMismatchingAnyLiteralAnnotationOnMethod() {
+ lint().files(java(
+ """
+ package test.pkg;
+ public class TestClass91 extends IFooMethod.Stub {
+ @android.annotation.EnforcePermission(anyOf={"android.permission.INTERNET", "android.permissionoopsthisisatypo.READ_PHONE_STATE"})
+ public void testMethodAnyLiteral() {}
+ }
+ """).indented(),
+ *stubs
+ )
+ .run()
+ .expect("""
+ src/test/pkg/TestClass91.java:4: Error: The method TestClass91.testMethodAnyLiteral is annotated with \
+ @android.annotation.EnforcePermission(anyOf={"android.permission.INTERNET", "android.permissionoopsthisisatypo.READ_PHONE_STATE"}) \
+ which differs from the overridden method Stub.testMethodAnyLiteral: \
+ @android.annotation.EnforcePermission(anyOf={android.Manifest.permission.INTERNET, "android.permission.READ_PHONE_STATE"}). \
+ The same annotation must be used for both methods. [MismatchingEnforcePermissionAnnotation]
+ public void testMethodAnyLiteral() {}
+ ~~~~~~~~~~~~~~~~~~~~
+ 1 errors, 0 warnings
+ """.addLineContinuation())
+ }
+
+ fun testDetectIssuesMismatchingAllAnnotationOnMethod() {
+ lint().files(java(
+ """
+ package test.pkg;
+ public class TestClass10 extends IFooMethod.Stub {
+ @android.annotation.EnforcePermission(allOf={android.Manifest.permission.INTERNET, android.Manifest.permission.NFC})
+ public void testMethodAll() {}
+ }
+ """).indented(),
+ *stubs
+ )
+ .run()
+ .expect("""
+ src/test/pkg/TestClass10.java:4: Error: The method TestClass10.testMethodAll is annotated with \
+ @android.annotation.EnforcePermission(allOf={android.Manifest.permission.INTERNET, android.Manifest.permission.NFC}) \
+ which differs from the overridden method Stub.testMethodAll: \
+ @android.annotation.EnforcePermission(allOf={android.Manifest.permission.INTERNET, android.Manifest.permission.READ_PHONE_STATE}). \
+ The same annotation must be used for both methods. [MismatchingEnforcePermissionAnnotation]
+ public void testMethodAll() {}
+ ~~~~~~~~~~~~~
+ 1 errors, 0 warnings
+ """.addLineContinuation())
+ }
+
+ fun testDetectIssuesMismatchingAllLiteralAnnotationOnMethod() {
+ lint().files(java(
+ """
+ package test.pkg;
+ public class TestClass101 extends IFooMethod.Stub {
+ @android.annotation.EnforcePermission(allOf={"android.permission.INTERNET", "android.permissionoopsthisisatypo.READ_PHONE_STATE"})
+ public void testMethodAllLiteral() {}
+ }
+ """).indented(),
+ *stubs
+ )
+ .run()
+ .expect("""
+ src/test/pkg/TestClass101.java:4: Error: The method TestClass101.testMethodAllLiteral is annotated with \
+ @android.annotation.EnforcePermission(allOf={"android.permission.INTERNET", "android.permissionoopsthisisatypo.READ_PHONE_STATE"}) \
+ which differs from the overridden method Stub.testMethodAllLiteral: \
+ @android.annotation.EnforcePermission(allOf={android.Manifest.permission.INTERNET, "android.permission.READ_PHONE_STATE"}). \
+ The same annotation must be used for both methods. [MismatchingEnforcePermissionAnnotation]
+ public void testMethodAllLiteral() {}
+ ~~~~~~~~~~~~~~~~~~~~
+ 1 errors, 0 warnings
+ """.addLineContinuation())
+ }
+
+ fun testDetectIssuesMissingAnnotationOnMethod() {
+ lint().files(java(
+ """
+ package test.pkg;
+ public class TestClass6 extends IFooMethod.Stub {
+ public void testMethod() {}
+ }
+ """).indented(),
+ *stubs
+ )
+ .run()
+ .expect("""
+ src/test/pkg/TestClass6.java:3: Error: The method TestClass6.testMethod overrides the method Stub.testMethod which is annotated with @EnforcePermission. \
+ The same annotation must be used on TestClass6.testMethod [MissingEnforcePermissionAnnotation]
+ public void testMethod() {}
+ ~~~~~~~~~~
+ 1 errors, 0 warnings
+ """.addLineContinuation())
+ }
+
+ fun testDetectIssuesExtraAnnotationMethod() {
+ lint().files(java(
+ """
+ package test.pkg;
+ public class TestClass7 extends IBar.Stub {
+ @android.annotation.EnforcePermission(android.Manifest.permission.INTERNET)
+ public void testMethod() {}
+ }
+ """).indented(),
+ *stubs
+ )
+ .run()
+ .expect("""
+ src/test/pkg/TestClass7.java:4: Error: The method TestClass7.testMethod overrides the method Stub.testMethod which is not annotated with @EnforcePermission. \
+ The same annotation must be used on Stub.testMethod. Did you forget to annotate the AIDL definition? [MissingEnforcePermissionAnnotation]
+ public void testMethod() {}
+ ~~~~~~~~~~
+ 1 errors, 0 warnings
+ """.addLineContinuation())
+ }
+
+ fun testDetectIssuesMissingAnnotationOnMethodWhenClassIsCalledDefault() {
+ lint().files(java(
+ """
+ package test.pkg;
+ public class Default extends IFooMethod.Stub {
+ public void testMethod() {}
+ }
+ """).indented(),
+ *stubs
+ )
+ .run()
+ .expect(
+ """
+ src/test/pkg/Default.java:3: Error: The method Default.testMethod \
+ overrides the method Stub.testMethod which is annotated with @EnforcePermission. The same annotation must be used on Default.testMethod [MissingEnforcePermissionAnnotation]
+ public void testMethod() {}
+ ~~~~~~~~~~
+ 1 errors, 0 warnings
+ """.addLineContinuation()
+ )
+ }
+
+ fun testDoesDetectIssuesShortStringsNotAllowed() {
+ lint().files(java(
+ """
+ package test.pkg;
+ import android.annotation.EnforcePermission;
+ public class TestClass121 extends IFooMethod.Stub {
+ @Override
+ @EnforcePermission(anyOf={"INTERNET", "READ_PHONE_STATE"})
+ public void testMethodAnyLiteral() {}
+ }
+ """).indented(),
+ *stubs
+ )
+ .run()
+ .expect(
+ """
+ src/test/pkg/TestClass121.java:6: Error: The method \
+ TestClass121.testMethodAnyLiteral is annotated with @EnforcePermission(anyOf={"INTERNET", "READ_PHONE_STATE"}) \
+ which differs from the overridden method Stub.testMethodAnyLiteral: \
+ @android.annotation.EnforcePermission(anyOf={android.Manifest.permission.INTERNET, "android.permission.READ_PHONE_STATE"}). \
+ The same annotation must be used for both methods. [MismatchingEnforcePermissionAnnotation]
+ public void testMethodAnyLiteral() {}
+ ~~~~~~~~~~~~~~~~~~~~
+ 1 errors, 0 warnings
+ """.addLineContinuation()
+ )
+ }
+
+ /* Stubs */
+
+ // A service with permission annotation on the method.
+ private val interfaceIFooMethodStub: TestFile = java(
+ """
+ public interface IFooMethod extends android.os.IInterface {
+ public static abstract class Stub extends android.os.Binder implements IFooMethod {
+ @Override
+ @android.annotation.EnforcePermission(android.Manifest.permission.READ_PHONE_STATE)
+ public void testMethod() {}
+ @Override
+ @android.annotation.EnforcePermission(anyOf={android.Manifest.permission.INTERNET, android.Manifest.permission.READ_PHONE_STATE})
+ public void testMethodAny() {}
+ @Override
+ @android.annotation.EnforcePermission(anyOf={android.Manifest.permission.INTERNET, "android.permission.READ_PHONE_STATE"})
+ public void testMethodAnyLiteral() {}
+ @Override
+ @android.annotation.EnforcePermission(allOf={android.Manifest.permission.INTERNET, android.Manifest.permission.READ_PHONE_STATE})
+ public void testMethodAll() {}
+ @Override
+ @android.annotation.EnforcePermission(allOf={android.Manifest.permission.INTERNET, "android.permission.READ_PHONE_STATE"})
+ public void testMethodAllLiteral() {}
+ }
+ @android.annotation.EnforcePermission(android.Manifest.permission.READ_PHONE_STATE)
+ public void testMethod();
+ @android.annotation.EnforcePermission(anyOf={android.Manifest.permission.INTERNET, android.Manifest.permission.READ_PHONE_STATE})
+ public void testMethodAny() {}
+ @android.annotation.EnforcePermission(anyOf={android.Manifest.permission.INTERNET, "android.permission.READ_PHONE_STATE"})
+ public void testMethodAnyLiteral() {}
+ @android.annotation.EnforcePermission(allOf={android.Manifest.permission.INTERNET, android.Manifest.permission.READ_PHONE_STATE})
+ public void testMethodAll() {}
+ @android.annotation.EnforcePermission(allOf={android.Manifest.permission.INTERNET, "android.permission.READ_PHONE_STATE"})
+ public void testMethodAllLiteral() {}
+ }
+ """
+ ).indented()
+
+ // A service without any permission annotation.
+ private val interfaceIBarStub: TestFile = java(
+ """
+ public interface IBar extends android.os.IInterface {
+ public static abstract class Stub extends android.os.Binder implements IBar {
+ @Override
+ public void testMethod() {}
+ }
+ public void testMethod();
+ }
+ """
+ ).indented()
+
+ private val manifestPermissionStub: TestFile = java(
+ """
+ package android.Manifest;
+ class permission {
+ public static final String READ_PHONE_STATE = "android.permission.READ_PHONE_STATE";
+ public static final String NFC = "android.permission.NFC";
+ public static final String INTERNET = "android.permission.INTERNET";
+ }
+ """
+ ).indented()
+
+ private val enforcePermissionAnnotationStub: TestFile = java(
+ """
+ package android.annotation;
+ public @interface EnforcePermission {}
+ """
+ ).indented()
+
+ private val stubs = arrayOf(interfaceIFooMethodStub, interfaceIBarStub,
+ manifestPermissionStub, enforcePermissionAnnotationStub)
+
+ // Substitutes "backslash + new line" with an empty string to imitate line continuation
+ private fun String.addLineContinuation(): String = this.trimIndent().replace("\\\n", "")
+}
diff --git a/tools/lint/global/checks/src/test/java/com/google/android/lint/aidl/EnforcePermissionHelperDetectorCodegenTest.kt b/tools/lint/global/checks/src/test/java/com/google/android/lint/aidl/EnforcePermissionHelperDetectorCodegenTest.kt
new file mode 100644
index 0000000..5a63bb4
--- /dev/null
+++ b/tools/lint/global/checks/src/test/java/com/google/android/lint/aidl/EnforcePermissionHelperDetectorCodegenTest.kt
@@ -0,0 +1,557 @@
+/*
+ * Copyright (C) 2022 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.checks.infrastructure.TestMode
+import com.android.tools.lint.detector.api.Detector
+import com.android.tools.lint.detector.api.Issue
+
+@Suppress("UnstableApiUsage")
+class EnforcePermissionHelperDetectorCodegenTest : LintDetectorTest() {
+ override fun getDetector(): Detector = EnforcePermissionHelperDetector()
+
+ override fun getIssues(): List<Issue> = listOf(
+ EnforcePermissionHelperDetector.ISSUE_ENFORCE_PERMISSION_HELPER
+ )
+
+ override fun lint(): TestLintTask = super.lint().allowMissingSdk(true)
+
+ fun test_generated_IProtected() {
+ lint().testModes(TestMode.DEFAULT).files(
+ java(
+ """
+ /*
+ * This file is auto-generated. DO NOT MODIFY.
+ */
+ package android.aidl.tests.permission;
+ public interface IProtected extends android.os.IInterface
+ {
+ /** Default implementation for IProtected. */
+ public static class Default implements android.aidl.tests.permission.IProtected
+ {
+ @Override public void PermissionProtected() throws android.os.RemoteException
+ {
+ }
+ @Override public void MultiplePermissionsAll() throws android.os.RemoteException
+ {
+ }
+ @Override public void MultiplePermissionsAny() throws android.os.RemoteException
+ {
+ }
+ @Override public void NonManifestPermission() throws android.os.RemoteException
+ {
+ }
+ // Used by the integration tests to dynamically set permissions that are considered granted.
+ @Override public void SetGranted(java.util.List<java.lang.String> permissions) throws android.os.RemoteException
+ {
+ }
+ @Override
+ public android.os.IBinder asBinder() {
+ return null;
+ }
+ }
+ /** Local-side IPC implementation stub class. */
+ public static abstract class Stub extends android.os.Binder implements android.aidl.tests.permission.IProtected
+ {
+ private final android.os.PermissionEnforcer mEnforcer;
+ /** Construct the stub using the Enforcer provided. */
+ public Stub(android.os.PermissionEnforcer enforcer)
+ {
+ this.attachInterface(this, DESCRIPTOR);
+ if (enforcer == null) {
+ throw new IllegalArgumentException("enforcer cannot be null");
+ }
+ mEnforcer = enforcer;
+ }
+ @Deprecated
+ /** Default constructor. */
+ public Stub() {
+ this(android.os.PermissionEnforcer.fromContext(
+ android.app.ActivityThread.currentActivityThread().getSystemContext()));
+ }
+ /**
+ * Cast an IBinder object into an android.aidl.tests.permission.IProtected interface,
+ * generating a proxy if needed.
+ */
+ public static android.aidl.tests.permission.IProtected asInterface(android.os.IBinder obj)
+ {
+ if ((obj==null)) {
+ return null;
+ }
+ android.os.IInterface iin = obj.queryLocalInterface(DESCRIPTOR);
+ if (((iin!=null)&&(iin instanceof android.aidl.tests.permission.IProtected))) {
+ return ((android.aidl.tests.permission.IProtected)iin);
+ }
+ return new android.aidl.tests.permission.IProtected.Stub.Proxy(obj);
+ }
+ @Override public android.os.IBinder asBinder()
+ {
+ return this;
+ }
+ /** @hide */
+ public static java.lang.String getDefaultTransactionName(int transactionCode)
+ {
+ switch (transactionCode)
+ {
+ case TRANSACTION_PermissionProtected:
+ {
+ return "PermissionProtected";
+ }
+ case TRANSACTION_MultiplePermissionsAll:
+ {
+ return "MultiplePermissionsAll";
+ }
+ case TRANSACTION_MultiplePermissionsAny:
+ {
+ return "MultiplePermissionsAny";
+ }
+ case TRANSACTION_NonManifestPermission:
+ {
+ return "NonManifestPermission";
+ }
+ case TRANSACTION_SetGranted:
+ {
+ return "SetGranted";
+ }
+ default:
+ {
+ return null;
+ }
+ }
+ }
+ /** @hide */
+ public java.lang.String getTransactionName(int transactionCode)
+ {
+ return this.getDefaultTransactionName(transactionCode);
+ }
+ @Override public boolean onTransact(int code, android.os.Parcel data, android.os.Parcel reply, int flags) throws android.os.RemoteException
+ {
+ java.lang.String descriptor = DESCRIPTOR;
+ if (code >= android.os.IBinder.FIRST_CALL_TRANSACTION && code <= android.os.IBinder.LAST_CALL_TRANSACTION) {
+ data.enforceInterface(descriptor);
+ }
+ switch (code)
+ {
+ case INTERFACE_TRANSACTION:
+ {
+ reply.writeString(descriptor);
+ return true;
+ }
+ }
+ switch (code)
+ {
+ case TRANSACTION_PermissionProtected:
+ {
+ this.PermissionProtected();
+ reply.writeNoException();
+ break;
+ }
+ case TRANSACTION_MultiplePermissionsAll:
+ {
+ this.MultiplePermissionsAll();
+ reply.writeNoException();
+ break;
+ }
+ case TRANSACTION_MultiplePermissionsAny:
+ {
+ this.MultiplePermissionsAny();
+ reply.writeNoException();
+ break;
+ }
+ case TRANSACTION_NonManifestPermission:
+ {
+ this.NonManifestPermission();
+ reply.writeNoException();
+ break;
+ }
+ case TRANSACTION_SetGranted:
+ {
+ java.util.List<java.lang.String> _arg0;
+ _arg0 = data.createStringArrayList();
+ data.enforceNoDataAvail();
+ this.SetGranted(_arg0);
+ reply.writeNoException();
+ break;
+ }
+ default:
+ {
+ return super.onTransact(code, data, reply, flags);
+ }
+ }
+ return true;
+ }
+ private static class Proxy implements android.aidl.tests.permission.IProtected
+ {
+ private android.os.IBinder mRemote;
+ Proxy(android.os.IBinder remote)
+ {
+ mRemote = remote;
+ }
+ @Override public android.os.IBinder asBinder()
+ {
+ return mRemote;
+ }
+ public java.lang.String getInterfaceDescriptor()
+ {
+ return DESCRIPTOR;
+ }
+ @Override public void PermissionProtected() throws android.os.RemoteException
+ {
+ android.os.Parcel _data = android.os.Parcel.obtain(asBinder());
+ android.os.Parcel _reply = android.os.Parcel.obtain();
+ try {
+ _data.writeInterfaceToken(DESCRIPTOR);
+ boolean _status = mRemote.transact(Stub.TRANSACTION_PermissionProtected, _data, _reply, 0);
+ _reply.readException();
+ }
+ finally {
+ _reply.recycle();
+ _data.recycle();
+ }
+ }
+ @Override public void MultiplePermissionsAll() throws android.os.RemoteException
+ {
+ android.os.Parcel _data = android.os.Parcel.obtain(asBinder());
+ android.os.Parcel _reply = android.os.Parcel.obtain();
+ try {
+ _data.writeInterfaceToken(DESCRIPTOR);
+ boolean _status = mRemote.transact(Stub.TRANSACTION_MultiplePermissionsAll, _data, _reply, 0);
+ _reply.readException();
+ }
+ finally {
+ _reply.recycle();
+ _data.recycle();
+ }
+ }
+ @Override public void MultiplePermissionsAny() throws android.os.RemoteException
+ {
+ android.os.Parcel _data = android.os.Parcel.obtain(asBinder());
+ android.os.Parcel _reply = android.os.Parcel.obtain();
+ try {
+ _data.writeInterfaceToken(DESCRIPTOR);
+ boolean _status = mRemote.transact(Stub.TRANSACTION_MultiplePermissionsAny, _data, _reply, 0);
+ _reply.readException();
+ }
+ finally {
+ _reply.recycle();
+ _data.recycle();
+ }
+ }
+ @Override public void NonManifestPermission() throws android.os.RemoteException
+ {
+ android.os.Parcel _data = android.os.Parcel.obtain(asBinder());
+ android.os.Parcel _reply = android.os.Parcel.obtain();
+ try {
+ _data.writeInterfaceToken(DESCRIPTOR);
+ boolean _status = mRemote.transact(Stub.TRANSACTION_NonManifestPermission, _data, _reply, 0);
+ _reply.readException();
+ }
+ finally {
+ _reply.recycle();
+ _data.recycle();
+ }
+ }
+ // Used by the integration tests to dynamically set permissions that are considered granted.
+ @Override public void SetGranted(java.util.List<java.lang.String> permissions) throws android.os.RemoteException
+ {
+ android.os.Parcel _data = android.os.Parcel.obtain(asBinder());
+ android.os.Parcel _reply = android.os.Parcel.obtain();
+ try {
+ _data.writeInterfaceToken(DESCRIPTOR);
+ _data.writeStringList(permissions);
+ boolean _status = mRemote.transact(Stub.TRANSACTION_SetGranted, _data, _reply, 0);
+ _reply.readException();
+ }
+ finally {
+ _reply.recycle();
+ _data.recycle();
+ }
+ }
+ }
+ static final int TRANSACTION_PermissionProtected = (android.os.IBinder.FIRST_CALL_TRANSACTION + 0);
+ /** Helper method to enforce permissions for PermissionProtected */
+ protected void PermissionProtected_enforcePermission() throws SecurityException {
+ android.content.AttributionSource source = new android.content.AttributionSource(getCallingUid(), null, null);
+ mEnforcer.enforcePermission(android.Manifest.permission.READ_PHONE_STATE, source);
+ }
+ static final int TRANSACTION_MultiplePermissionsAll = (android.os.IBinder.FIRST_CALL_TRANSACTION + 1);
+ /** Helper method to enforce permissions for MultiplePermissionsAll */
+ protected void MultiplePermissionsAll_enforcePermission() throws SecurityException {
+ android.content.AttributionSource source = new android.content.AttributionSource(getCallingUid(), null, null);
+ mEnforcer.enforcePermissionAllOf(new String[]{android.Manifest.permission.INTERNET, android.Manifest.permission.VIBRATE}, source);
+ }
+ static final int TRANSACTION_MultiplePermissionsAny = (android.os.IBinder.FIRST_CALL_TRANSACTION + 2);
+ /** Helper method to enforce permissions for MultiplePermissionsAny */
+ protected void MultiplePermissionsAny_enforcePermission() throws SecurityException {
+ android.content.AttributionSource source = new android.content.AttributionSource(getCallingUid(), null, null);
+ mEnforcer.enforcePermissionAnyOf(new String[]{android.Manifest.permission.INTERNET, android.Manifest.permission.VIBRATE}, source);
+ }
+ static final int TRANSACTION_NonManifestPermission = (android.os.IBinder.FIRST_CALL_TRANSACTION + 3);
+ /** Helper method to enforce permissions for NonManifestPermission */
+ protected void NonManifestPermission_enforcePermission() throws SecurityException {
+ android.content.AttributionSource source = new android.content.AttributionSource(getCallingUid(), null, null);
+ mEnforcer.enforcePermission(android.net.NetworkStack.PERMISSION_MAINLINE_NETWORK_STACK, source);
+ }
+ static final int TRANSACTION_SetGranted = (android.os.IBinder.FIRST_CALL_TRANSACTION + 4);
+ /** @hide */
+ public int getMaxTransactionId()
+ {
+ return 4;
+ }
+ }
+
+ @android.annotation.EnforcePermission(android.Manifest.permission.READ_PHONE_STATE)
+ public void PermissionProtected() throws android.os.RemoteException;
+ @android.annotation.EnforcePermission(allOf = {android.Manifest.permission.INTERNET, android.Manifest.permission.VIBRATE})
+ public void MultiplePermissionsAll() throws android.os.RemoteException;
+ @android.annotation.EnforcePermission(anyOf = {android.Manifest.permission.INTERNET, android.Manifest.permission.VIBRATE})
+ public void MultiplePermissionsAny() throws android.os.RemoteException;
+ @android.annotation.EnforcePermission(android.net.NetworkStack.PERMISSION_MAINLINE_NETWORK_STACK)
+ public void NonManifestPermission() throws android.os.RemoteException;
+ // Used by the integration tests to dynamically set permissions that are considered granted.
+ @android.annotation.RequiresNoPermission
+ public void SetGranted(java.util.List<java.lang.String> permissions) throws android.os.RemoteException;
+ }
+ """
+ ).indented(),
+ *stubs
+ )
+ .run()
+ .expectClean()
+ }
+
+ fun test_generated_IProtectedInterface() {
+ lint().files(
+ java(
+ """
+ /*
+ * This file is auto-generated. DO NOT MODIFY.
+ */
+ package android.aidl.tests.permission;
+ public interface IProtectedInterface extends android.os.IInterface
+ {
+ /** Default implementation for IProtectedInterface. */
+ public static class Default implements android.aidl.tests.permission.IProtectedInterface
+ {
+ @Override public void Method1() throws android.os.RemoteException
+ {
+ }
+ @Override public void Method2() throws android.os.RemoteException
+ {
+ }
+ @Override
+ public android.os.IBinder asBinder() {
+ return null;
+ }
+ }
+ /** Local-side IPC implementation stub class. */
+ public static abstract class Stub extends android.os.Binder implements android.aidl.tests.permission.IProtectedInterface
+ {
+ private final android.os.PermissionEnforcer mEnforcer;
+ /** Construct the stub using the Enforcer provided. */
+ public Stub(android.os.PermissionEnforcer enforcer)
+ {
+ this.attachInterface(this, DESCRIPTOR);
+ if (enforcer == null) {
+ throw new IllegalArgumentException("enforcer cannot be null");
+ }
+ mEnforcer = enforcer;
+ }
+ @Deprecated
+ /** Default constructor. */
+ public Stub() {
+ this(android.os.PermissionEnforcer.fromContext(
+ android.app.ActivityThread.currentActivityThread().getSystemContext()));
+ }
+ /**
+ * Cast an IBinder object into an android.aidl.tests.permission.IProtectedInterface interface,
+ * generating a proxy if needed.
+ */
+ public static android.aidl.tests.permission.IProtectedInterface asInterface(android.os.IBinder obj)
+ {
+ if ((obj==null)) {
+ return null;
+ }
+ android.os.IInterface iin = obj.queryLocalInterface(DESCRIPTOR);
+ if (((iin!=null)&&(iin instanceof android.aidl.tests.permission.IProtectedInterface))) {
+ return ((android.aidl.tests.permission.IProtectedInterface)iin);
+ }
+ return new android.aidl.tests.permission.IProtectedInterface.Stub.Proxy(obj);
+ }
+ @Override public android.os.IBinder asBinder()
+ {
+ return this;
+ }
+ /** @hide */
+ public static java.lang.String getDefaultTransactionName(int transactionCode)
+ {
+ switch (transactionCode)
+ {
+ case TRANSACTION_Method1:
+ {
+ return "Method1";
+ }
+ case TRANSACTION_Method2:
+ {
+ return "Method2";
+ }
+ default:
+ {
+ return null;
+ }
+ }
+ }
+ /** @hide */
+ public java.lang.String getTransactionName(int transactionCode)
+ {
+ return this.getDefaultTransactionName(transactionCode);
+ }
+ @Override public boolean onTransact(int code, android.os.Parcel data, android.os.Parcel reply, int flags) throws android.os.RemoteException
+ {
+ java.lang.String descriptor = DESCRIPTOR;
+ if (code >= android.os.IBinder.FIRST_CALL_TRANSACTION && code <= android.os.IBinder.LAST_CALL_TRANSACTION) {
+ data.enforceInterface(descriptor);
+ }
+ switch (code)
+ {
+ case INTERFACE_TRANSACTION:
+ {
+ reply.writeString(descriptor);
+ return true;
+ }
+ }
+ switch (code)
+ {
+ case TRANSACTION_Method1:
+ {
+ this.Method1();
+ reply.writeNoException();
+ break;
+ }
+ case TRANSACTION_Method2:
+ {
+ this.Method2();
+ reply.writeNoException();
+ break;
+ }
+ default:
+ {
+ return super.onTransact(code, data, reply, flags);
+ }
+ }
+ return true;
+ }
+ private static class Proxy implements android.aidl.tests.permission.IProtectedInterface
+ {
+ private android.os.IBinder mRemote;
+ Proxy(android.os.IBinder remote)
+ {
+ mRemote = remote;
+ }
+ @Override public android.os.IBinder asBinder()
+ {
+ return mRemote;
+ }
+ public java.lang.String getInterfaceDescriptor()
+ {
+ return DESCRIPTOR;
+ }
+ @Override public void Method1() throws android.os.RemoteException
+ {
+ android.os.Parcel _data = android.os.Parcel.obtain(asBinder());
+ android.os.Parcel _reply = android.os.Parcel.obtain();
+ try {
+ _data.writeInterfaceToken(DESCRIPTOR);
+ boolean _status = mRemote.transact(Stub.TRANSACTION_Method1, _data, _reply, 0);
+ _reply.readException();
+ }
+ finally {
+ _reply.recycle();
+ _data.recycle();
+ }
+ }
+ @Override public void Method2() throws android.os.RemoteException
+ {
+ android.os.Parcel _data = android.os.Parcel.obtain(asBinder());
+ android.os.Parcel _reply = android.os.Parcel.obtain();
+ try {
+ _data.writeInterfaceToken(DESCRIPTOR);
+ boolean _status = mRemote.transact(Stub.TRANSACTION_Method2, _data, _reply, 0);
+ _reply.readException();
+ }
+ finally {
+ _reply.recycle();
+ _data.recycle();
+ }
+ }
+ }
+ static final int TRANSACTION_Method1 = (android.os.IBinder.FIRST_CALL_TRANSACTION + 0);
+ /** Helper method to enforce permissions for Method1 */
+ protected void Method1_enforcePermission() throws SecurityException {
+ android.content.AttributionSource source = new android.content.AttributionSource(getCallingUid(), null, null);
+ mEnforcer.enforcePermission(android.Manifest.permission.ACCESS_FINE_LOCATION, source);
+ }
+ static final int TRANSACTION_Method2 = (android.os.IBinder.FIRST_CALL_TRANSACTION + 1);
+ /** Helper method to enforce permissions for Method2 */
+ protected void Method2_enforcePermission() throws SecurityException {
+ android.content.AttributionSource source = new android.content.AttributionSource(getCallingUid(), null, null);
+ mEnforcer.enforcePermission(android.Manifest.permission.ACCESS_FINE_LOCATION, source);
+ }
+ /** @hide */
+ public int getMaxTransactionId()
+ {
+ return 1;
+ }
+ }
+
+ @android.annotation.EnforcePermission(android.Manifest.permission.ACCESS_FINE_LOCATION)
+ public void Method1() throws android.os.RemoteException;
+ @android.annotation.EnforcePermission(android.Manifest.permission.ACCESS_FINE_LOCATION)
+ public void Method2() throws android.os.RemoteException;
+ }
+ """
+ ).indented(),
+ *stubs
+ )
+ .run()
+ .expectClean()
+ }
+
+ /* Stubs */
+
+ private val manifestPermissionStub: TestFile = java(
+ """
+ package android.Manifest;
+ class permission {
+ public static final String READ_PHONE_STATE = "android.permission.READ_PHONE_STATE";
+ public static final String INTERNET = "android.permission.INTERNET";
+ }
+ """
+ ).indented()
+
+ private val enforcePermissionAnnotationStub: TestFile = java(
+ """
+ package android.annotation;
+ public @interface EnforcePermission {}
+ """
+ ).indented()
+
+ private val stubs = arrayOf(manifestPermissionStub, enforcePermissionAnnotationStub)
+}
diff --git a/tools/lint/global/checks/src/test/java/com/google/android/lint/aidl/EnforcePermissionHelperDetectorTest.kt b/tools/lint/global/checks/src/test/java/com/google/android/lint/aidl/EnforcePermissionHelperDetectorTest.kt
new file mode 100644
index 0000000..10a6e1d
--- /dev/null
+++ b/tools/lint/global/checks/src/test/java/com/google/android/lint/aidl/EnforcePermissionHelperDetectorTest.kt
@@ -0,0 +1,443 @@
+/*
+* Copyright (C) 2022 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.TestLintTask
+
+class EnforcePermissionHelperDetectorTest : LintDetectorTest() {
+ override fun getDetector() = EnforcePermissionHelperDetector()
+ override fun getIssues() = listOf(
+ EnforcePermissionHelperDetector.ISSUE_ENFORCE_PERMISSION_HELPER,
+ EnforcePermissionHelperDetector.ISSUE_MISUSING_ENFORCE_PERMISSION
+ )
+
+ override fun lint(): TestLintTask = super.lint().allowMissingSdk()
+
+ fun testFirstExpressionIsFunctionCall() {
+ lint().files(
+ java(
+ """
+ import android.content.Context;
+ import android.test.ITest;
+ public class Foo extends ITest.Stub {
+ private Context mContext;
+ @Override
+ @android.annotation.EnforcePermission("android.Manifest.permission.READ_CONTACTS")
+ public void test() throws android.os.RemoteException {
+ Binder.getCallingUid();
+ }
+ }
+ """
+ ).indented(),
+ *stubs
+ )
+ .run()
+ .expect(
+ """
+ src/Foo.java:5: Error: Method must start with test_enforcePermission() or super.test(), if applicable [MissingEnforcePermissionHelper]
+ @Override
+ ^
+ 1 errors, 0 warnings
+ """
+ )
+ .expectFixDiffs(
+ """
+ Autofix for src/Foo.java line 5: Replace with test_enforcePermission();...:
+ @@ -8 +8
+ + test_enforcePermission();
+ +
+ """
+ )
+ }
+
+ fun testFirstExpressionIsVariableDeclaration() {
+ lint().files(
+ java(
+ """
+ import android.content.Context;
+ import android.test.ITest;
+ public class Foo extends ITest.Stub {
+ private Context mContext;
+ @Override
+ @android.annotation.EnforcePermission("android.Manifest.permission.READ_CONTACTS")
+ public void test() throws android.os.RemoteException {
+ String foo = "bar";
+ Binder.getCallingUid();
+ }
+ }
+ """
+ ).indented(),
+ *stubs
+ )
+ .run()
+ .expect(
+ """
+ src/Foo.java:5: Error: Method must start with test_enforcePermission() or super.test(), if applicable [MissingEnforcePermissionHelper]
+ @Override
+ ^
+ 1 errors, 0 warnings
+ """
+ )
+ .expectFixDiffs(
+ """
+ Autofix for src/Foo.java line 5: Replace with test_enforcePermission();...:
+ @@ -8 +8
+ + test_enforcePermission();
+ +
+ """
+ )
+ }
+
+ fun testMethodIsEmpty() {
+ lint().files(
+ java(
+ """
+ import android.content.Context;
+ import android.test.ITest;
+ public class Foo extends ITest.Stub {
+ private Context mContext;
+ @Override
+ @android.annotation.EnforcePermission("android.Manifest.permission.READ_CONTACTS")
+ public void test() throws android.os.RemoteException {}
+ }
+ """
+ ).indented(),
+ *stubs
+ )
+ .run()
+ .expect(
+ """
+ src/Foo.java:5: Error: Method must start with test_enforcePermission() or super.test(), if applicable [MissingEnforcePermissionHelper]
+ @Override
+ ^
+ 1 errors, 0 warnings
+ """
+ )
+ }
+
+ fun testOkay() {
+ lint().files(
+ java(
+ """
+ import android.content.Context;
+ import android.test.ITest;
+ public class Foo extends ITest.Stub {
+ private Context mContext;
+ @Override
+ @android.annotation.EnforcePermission("android.Manifest.permission.READ_CONTACTS")
+ public void test() throws android.os.RemoteException {
+ super.test_enforcePermission();
+ }
+ }
+ """
+ ).indented(),
+ *stubs
+ )
+ .run()
+ .expectClean()
+ }
+
+ fun testHelperWithoutSuperPrefix_Okay() {
+ lint().files(
+ java(
+ """
+ import android.content.Context;
+ import android.test.ITest;
+ public class Foo extends ITest.Stub {
+ private Context mContext;
+ @Override
+ @android.annotation.EnforcePermission("android.Manifest.permission.READ_CONTACTS")
+ public void test() throws android.os.RemoteException {
+ test_enforcePermission();
+ }
+ }
+ """
+ ).indented(),
+ *stubs
+ )
+ .run()
+ .expectClean()
+ }
+
+ fun testInterfaceDefaultMethod_notStubAncestor_error() {
+ lint().files(
+ java(
+ """
+ public interface IProtected extends android.os.IInterface {
+ @android.annotation.EnforcePermission(android.Manifest.permission.READ_PHONE_STATE)
+ default void PermissionProtected() throws android.os.RemoteException {
+ String foo = "bar";
+ }
+ }
+ """
+ ).indented(),
+ *stubs
+ )
+ .run()
+ .expect(
+ """
+ src/IProtected.java:2: Error: The class of PermissionProtected does not inherit from an AIDL generated Stub class [MisusingEnforcePermissionAnnotation]
+ @android.annotation.EnforcePermission(android.Manifest.permission.READ_PHONE_STATE)
+ ^
+ 1 errors, 0 warnings
+ """
+ )
+ }
+
+ fun testInheritance_callSuper_okay() {
+ lint().files(
+ java(
+ """
+ package test;
+ import android.content.Context;
+ import android.test.ITest;
+ public class Foo extends ITest.Stub {
+ private Context mContext;
+ @Override
+ @android.annotation.EnforcePermission("android.Manifest.permission.READ_CONTACTS")
+ public void test() throws android.os.RemoteException {
+ super.test_enforcePermission();
+ }
+ }
+ """
+ ).indented(),
+ java(
+ """
+ package test;
+ import test.Foo;
+ public class Bar extends Foo {
+ @Override
+ @android.annotation.EnforcePermission("android.Manifest.permission.READ_CONTACTS")
+ public void test() throws android.os.RemoteException {
+ super.test();
+ }
+ }
+ """
+ ).indented(),
+ java(
+ """
+ package test;
+ import test.Bar;
+ public class Baz extends Bar {
+ @Override
+ @android.annotation.EnforcePermission("android.Manifest.permission.READ_CONTACTS")
+ public void test() throws android.os.RemoteException {
+ super.test();
+ }
+ }
+ """
+ ).indented(),
+ *stubs
+ )
+ .run()
+ .expectClean()
+ }
+
+ fun testInheritance_callHelper_okay() {
+ lint().files(
+ java(
+ """
+ package test;
+ import android.content.Context;
+ import android.test.ITest;
+ public class Foo extends ITest.Stub {
+ private Context mContext;
+ @Override
+ @android.annotation.EnforcePermission("android.Manifest.permission.READ_CONTACTS")
+ public void test() throws android.os.RemoteException {
+ super.test_enforcePermission();
+ }
+ }
+ """
+ ).indented(),
+ java(
+ """
+ package test;
+ import test.Foo;
+ public class Bar extends Foo {
+ @Override
+ @android.annotation.EnforcePermission("android.Manifest.permission.READ_CONTACTS")
+ public void test() throws android.os.RemoteException {
+ super.test();
+ }
+ }
+ """
+ ).indented(),
+ java(
+ """
+ package test;
+ import test.Bar;
+ public class Baz extends Bar {
+ @Override
+ @android.annotation.EnforcePermission("android.Manifest.permission.READ_CONTACTS")
+ public void test() throws android.os.RemoteException {
+ super.test_enforcePermission();
+ }
+ }
+ """
+ ).indented(),
+ *stubs
+ )
+ .run()
+ .expectClean()
+ }
+
+ fun testInheritance_missingCallInChain_error() {
+ lint().files(
+ java(
+ """
+ package test;
+ import android.content.Context;
+ import android.test.ITest;
+ public class Foo extends ITest.Stub {
+ private Context mContext;
+ @Override
+ @android.annotation.EnforcePermission("android.Manifest.permission.READ_CONTACTS")
+ public void test() throws android.os.RemoteException {
+ super.test_enforcePermission();
+ }
+ }
+ """
+ ).indented(),
+ java(
+ """
+ package test;
+ import test.Foo;
+ public class Bar extends Foo {
+ @Override
+ @android.annotation.EnforcePermission("android.Manifest.permission.READ_CONTACTS")
+ public void test() throws android.os.RemoteException {
+ doStuff();
+ }
+ }
+ """
+ ).indented(),
+ java(
+ """
+ package test;
+ import test.Bar;
+ public class Baz extends Bar {
+ @Override
+ @android.annotation.EnforcePermission("android.Manifest.permission.READ_CONTACTS")
+ public void test() throws android.os.RemoteException {
+ super.test();
+ }
+ }
+ """
+ ).indented(),
+ *stubs
+ )
+ .run()
+ .expect(
+ """
+ src/test/Bar.java:4: Error: Method must start with test_enforcePermission() or super.test(), if applicable [MissingEnforcePermissionHelper]
+ @Override
+ ^
+ 1 errors, 0 warnings
+ """
+ )
+ }
+
+ fun testInheritance_missingCall_error() {
+ lint().files(
+ java(
+ """
+ package test;
+ import android.content.Context;
+ import android.test.ITest;
+ public class Foo extends ITest.Stub {
+ private Context mContext;
+ @Override
+ @android.annotation.EnforcePermission("android.Manifest.permission.READ_CONTACTS")
+ public void test() throws android.os.RemoteException {
+ super.test_enforcePermission();
+ }
+ }
+ """
+ ).indented(),
+ java(
+ """
+ package test;
+ import test.Foo;
+ public class Bar extends Foo {
+ @Override
+ @android.annotation.EnforcePermission("android.Manifest.permission.READ_CONTACTS")
+ public void test() throws android.os.RemoteException {
+ super.test();
+ }
+ }
+ """
+ ).indented(),
+ java(
+ """
+ package test;
+ import test.Bar;
+ public class Baz extends Bar {
+ @Override
+ @android.annotation.EnforcePermission("android.Manifest.permission.READ_CONTACTS")
+ public void test() throws android.os.RemoteException {
+ doStuff();
+ }
+ }
+ """
+ ).indented(),
+ *stubs
+ )
+ .run()
+ .expect(
+ """
+ src/test/Baz.java:4: Error: Method must start with test_enforcePermission() or super.test(), if applicable [MissingEnforcePermissionHelper]
+ @Override
+ ^
+ 1 errors, 0 warnings
+ """
+ )
+ }
+
+ fun testRandomClass_notStubAncestor_error() {
+ lint().files(
+ java(
+ """
+ public class Foo {
+ @android.annotation.EnforcePermission(android.Manifest.permission.READ_PHONE_STATE)
+ void PermissionProtected() throws android.os.RemoteException {
+ String foo = "bar";
+ }
+ }
+ """
+ ).indented(),
+ *stubs
+ )
+ .run()
+ .expect(
+ """
+ src/Foo.java:2: Error: The class of PermissionProtected does not inherit from an AIDL generated Stub class [MisusingEnforcePermissionAnnotation]
+ @android.annotation.EnforcePermission(android.Manifest.permission.READ_PHONE_STATE)
+ ^
+ 1 errors, 0 warnings
+ """
+ )
+ }
+
+ companion object {
+ val stubs = arrayOf(aidlStub, contextStub, binderStub)
+ }
+}
+
+
+
diff --git a/tools/lint/global/checks/src/test/java/com/google/android/lint/aidl/SimpleManualPermissionEnforcementDetectorTest.kt b/tools/lint/global/checks/src/test/java/com/google/android/lint/aidl/SimpleManualPermissionEnforcementDetectorTest.kt
new file mode 100644
index 0000000..6b8e72cf
--- /dev/null
+++ b/tools/lint/global/checks/src/test/java/com/google/android/lint/aidl/SimpleManualPermissionEnforcementDetectorTest.kt
@@ -0,0 +1,843 @@
+/*
+ * Copyright (C) 2022 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.TestLintTask
+import com.android.tools.lint.detector.api.Detector
+import com.android.tools.lint.detector.api.Issue
+
+@Suppress("UnstableApiUsage")
+class SimpleManualPermissionEnforcementDetectorTest : LintDetectorTest() {
+ override fun getDetector(): Detector = SimpleManualPermissionEnforcementDetector()
+ override fun getIssues(): List<Issue> = listOf(
+ SimpleManualPermissionEnforcementDetector
+ .ISSUE_SIMPLE_MANUAL_PERMISSION_ENFORCEMENT
+ )
+
+ override fun lint(): TestLintTask = super.lint().allowMissingSdk()
+
+ fun testClass() {
+ lint().files(
+ java(
+ """
+ import android.content.Context;
+ import android.test.ITest;
+ public class Foo extends ITest.Stub {
+ private Context mContext;
+ @Override
+ public void test() throws android.os.RemoteException {
+ mContext.enforceCallingOrSelfPermission("android.permission.READ_CONTACTS", "foo");
+ }
+ }
+ """
+ ).indented(),
+ *stubs
+ )
+ .run()
+ .expect(
+ """
+ src/Foo.java:7: Warning: ITest permission check should be converted to @EnforcePermission annotation [SimpleManualPermissionEnforcement]
+ mContext.enforceCallingOrSelfPermission("android.permission.READ_CONTACTS", "foo");
+ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+ 0 errors, 1 warnings
+ """
+ )
+ .expectFixDiffs(
+ """
+ Fix for src/Foo.java line 7: Annotate with @EnforcePermission:
+ @@ -5 +5
+ + @android.annotation.EnforcePermission("android.permission.READ_CONTACTS")
+ @@ -7 +8
+ - mContext.enforceCallingOrSelfPermission("android.permission.READ_CONTACTS", "foo");
+ + test_enforcePermission();
+ """
+ )
+ }
+
+ fun testClass_orSelfFalse_warning() {
+ lint().files(
+ java(
+ """
+ import android.content.Context;
+ import android.test.ITest;
+ public class Foo extends ITest.Stub {
+ private Context mContext;
+ @Override
+ public void test() throws android.os.RemoteException {
+ mContext.enforceCallingPermission("android.permission.READ_CONTACTS", "foo");
+ }
+ }
+ """
+ ).indented(),
+ *stubs
+ )
+ .run()
+ .expect(
+ """
+ src/Foo.java:7: Warning: ITest permission check can be converted to @EnforcePermission annotation [SimpleManualPermissionEnforcement]
+ mContext.enforceCallingPermission("android.permission.READ_CONTACTS", "foo");
+ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+ 0 errors, 1 warnings
+ """
+ )
+ .expectFixDiffs(
+ """
+ Fix for src/Foo.java line 7: Annotate with @EnforcePermission:
+ @@ -5 +5
+ + @android.annotation.EnforcePermission("android.permission.READ_CONTACTS")
+ @@ -7 +8
+ - mContext.enforceCallingPermission("android.permission.READ_CONTACTS", "foo");
+ + test_enforcePermission();
+ """
+ )
+ }
+
+ fun testClass_enforcesFalse_warning() {
+ lint().files(
+ java(
+ """
+ import android.content.Context;
+ import android.test.ITest;
+ public class Foo extends ITest.Stub {
+ private Context mContext;
+ @Override
+ public void test() throws android.os.RemoteException {
+ mContext.checkCallingOrSelfPermission("android.permission.READ_CONTACTS", "foo");
+ }
+ }
+ """
+ ).indented(),
+ *stubs
+ )
+ .run()
+ .expect(
+ """
+ src/Foo.java:7: Warning: ITest permission check can be converted to @EnforcePermission annotation [SimpleManualPermissionEnforcement]
+ mContext.checkCallingOrSelfPermission("android.permission.READ_CONTACTS", "foo");
+ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+ 0 errors, 1 warnings
+ """
+ )
+ .expectFixDiffs(
+ """
+ Fix for src/Foo.java line 7: Annotate with @EnforcePermission:
+ @@ -5 +5
+ + @android.annotation.EnforcePermission("android.permission.READ_CONTACTS")
+ @@ -7 +8
+ - mContext.checkCallingOrSelfPermission("android.permission.READ_CONTACTS", "foo");
+ + test_enforcePermission();
+ """
+ )
+ }
+
+ fun testAnonClass() {
+ lint().files(
+ java(
+ """
+ import android.content.Context;
+ import android.test.ITest;
+ public class Foo {
+ private Context mContext;
+ private ITest itest = new ITest.Stub() {
+ @Override
+ public void test() throws android.os.RemoteException {
+ mContext.enforceCallingOrSelfPermission(
+ "android.permission.READ_CONTACTS", "foo");
+ }
+ };
+ }
+ """
+ ).indented(),
+ *stubs
+ )
+ .run()
+ .expect(
+ """
+ src/Foo.java:8: Warning: ITest permission check should be converted to @EnforcePermission annotation [SimpleManualPermissionEnforcement]
+ mContext.enforceCallingOrSelfPermission(
+ ^
+ 0 errors, 1 warnings
+ """
+ )
+ .expectFixDiffs(
+ """
+ Fix for src/Foo.java line 8: Annotate with @EnforcePermission:
+ @@ -6 +6
+ + @android.annotation.EnforcePermission("android.permission.READ_CONTACTS")
+ @@ -8 +9
+ - mContext.enforceCallingOrSelfPermission(
+ - "android.permission.READ_CONTACTS", "foo");
+ + test_enforcePermission();
+ """
+ )
+ }
+
+ fun testConstantEvaluation() {
+ lint().files(
+ java(
+ """
+ import android.content.Context;
+ import android.test.ITest;
+
+ public class Foo extends ITest.Stub {
+ private Context mContext;
+ @Override
+ public void test() throws android.os.RemoteException {
+ mContext.enforceCallingOrSelfPermission(android.Manifest.permission.READ_CONTACTS, "foo");
+ }
+ }
+ """
+ ).indented(),
+ *stubs,
+ manifestStub
+ )
+ .run()
+ .expect(
+ """
+ src/Foo.java:8: Warning: ITest permission check should be converted to @EnforcePermission annotation [SimpleManualPermissionEnforcement]
+ mContext.enforceCallingOrSelfPermission(android.Manifest.permission.READ_CONTACTS, "foo");
+ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+ 0 errors, 1 warnings
+ """
+ )
+ .expectFixDiffs(
+ """
+ Fix for src/Foo.java line 8: Annotate with @EnforcePermission:
+ @@ -6 +6
+ + @android.annotation.EnforcePermission("android.permission.READ_CONTACTS")
+ @@ -8 +9
+ - mContext.enforceCallingOrSelfPermission(android.Manifest.permission.READ_CONTACTS, "foo");
+ + test_enforcePermission();
+ """
+ )
+ }
+
+ fun testAllOf() {
+ lint().files(
+ java(
+ """
+ import android.content.Context;
+ import android.test.ITest;
+ public class Foo {
+ private Context mContext;
+ private ITest itest = new ITest.Stub() {
+ @Override
+ public void test() throws android.os.RemoteException {
+ mContext.enforceCallingOrSelfPermission(
+ "android.permission.READ_CONTACTS", "foo");
+ mContext.enforceCallingOrSelfPermission(
+ "android.permission.WRITE_CONTACTS", "foo");
+ }
+ };
+ }
+ """
+ ).indented(),
+ *stubs
+ )
+ .run()
+ .expect(
+ """
+ src/Foo.java:10: Warning: ITest permission check should be converted to @EnforcePermission annotation [SimpleManualPermissionEnforcement]
+ mContext.enforceCallingOrSelfPermission(
+ ^
+ 0 errors, 1 warnings
+ """
+ )
+ .expectFixDiffs(
+ """
+ Fix for src/Foo.java line 10: Annotate with @EnforcePermission:
+ @@ -6 +6
+ + @android.annotation.EnforcePermission(allOf={"android.permission.READ_CONTACTS", "android.permission.WRITE_CONTACTS"})
+ @@ -8 +9
+ - mContext.enforceCallingOrSelfPermission(
+ - "android.permission.READ_CONTACTS", "foo");
+ - mContext.enforceCallingOrSelfPermission(
+ - "android.permission.WRITE_CONTACTS", "foo");
+ + test_enforcePermission();
+ """
+ )
+ }
+
+ fun testAllOf_mixedOrSelf_warning() {
+ lint().files(
+ java(
+ """
+ import android.content.Context;
+ import android.test.ITest;
+ public class Foo {
+ private Context mContext;
+ private ITest itest = new ITest.Stub() {
+ @Override
+ public void test() throws android.os.RemoteException {
+ mContext.enforceCallingOrSelfPermission(
+ "android.permission.READ_CONTACTS", "foo");
+ mContext.enforceCallingPermission(
+ "android.permission.WRITE_CONTACTS", "foo");
+ }
+ };
+ }
+ """
+ ).indented(),
+ *stubs
+ )
+ .run()
+ .expect(
+ """
+ src/Foo.java:10: Warning: ITest permission check can be converted to @EnforcePermission annotation [SimpleManualPermissionEnforcement]
+ mContext.enforceCallingPermission(
+ ^
+ 0 errors, 1 warnings
+ """
+ )
+ .expectFixDiffs(
+ """
+ Fix for src/Foo.java line 10: Annotate with @EnforcePermission:
+ @@ -6 +6
+ + @android.annotation.EnforcePermission(allOf={"android.permission.READ_CONTACTS", "android.permission.WRITE_CONTACTS"})
+ @@ -8 +9
+ - mContext.enforceCallingOrSelfPermission(
+ - "android.permission.READ_CONTACTS", "foo");
+ - mContext.enforceCallingPermission(
+ - "android.permission.WRITE_CONTACTS", "foo");
+ + test_enforcePermission();
+ """
+ )
+ }
+
+ fun testAllOf_mixedEnforces_warning() {
+ lint().files(
+ java(
+ """
+ import android.content.Context;
+ import android.test.ITest;
+ public class Foo {
+ private Context mContext;
+ private ITest itest = new ITest.Stub() {
+ @Override
+ public void test() throws android.os.RemoteException {
+ mContext.enforceCallingOrSelfPermission(
+ "android.permission.READ_CONTACTS", "foo");
+ mContext.checkCallingOrSelfPermission(
+ "android.permission.WRITE_CONTACTS", "foo");
+ }
+ };
+ }
+ """
+ ).indented(),
+ *stubs
+ )
+ .run()
+ .expect(
+ """
+ src/Foo.java:10: Warning: ITest permission check can be converted to @EnforcePermission annotation [SimpleManualPermissionEnforcement]
+ mContext.checkCallingOrSelfPermission(
+ ^
+ 0 errors, 1 warnings
+ """
+ )
+ .expectFixDiffs(
+ """
+ Fix for src/Foo.java line 10: Annotate with @EnforcePermission:
+ @@ -6 +6
+ + @android.annotation.EnforcePermission(allOf={"android.permission.READ_CONTACTS", "android.permission.WRITE_CONTACTS"})
+ @@ -8 +9
+ - mContext.enforceCallingOrSelfPermission(
+ - "android.permission.READ_CONTACTS", "foo");
+ - mContext.checkCallingOrSelfPermission(
+ - "android.permission.WRITE_CONTACTS", "foo");
+ + test_enforcePermission();
+ """
+ )
+ }
+
+ fun testPrecedingExpressions() {
+ lint().files(
+ java(
+ """
+ import android.os.Binder;
+ import android.test.ITest;
+ public class Foo extends ITest.Stub {
+ private mContext Context;
+ @Override
+ public void test() throws android.os.RemoteException {
+ long uid = Binder.getCallingUid();
+ mContext.enforceCallingOrSelfPermission("android.permission.READ_CONTACTS", "foo");
+ }
+ }
+ """
+ ).indented(),
+ *stubs
+ )
+ .run()
+ .expectClean()
+ }
+
+ fun testPermissionHelper() {
+ lint().files(
+ java(
+ """
+ import android.content.Context;
+ import android.test.ITest;
+
+ public class Foo extends ITest.Stub {
+ private Context mContext;
+
+ @android.annotation.PermissionMethod(orSelf = true)
+ private void helper() {
+ mContext.enforceCallingOrSelfPermission("android.permission.READ_CONTACTS", "foo");
+ }
+
+ @Override
+ public void test() throws android.os.RemoteException {
+ helper();
+ }
+ }
+ """
+ ).indented(),
+ *stubs
+ )
+ .run()
+ .expect(
+ """
+ src/Foo.java:14: Warning: ITest permission check should be converted to @EnforcePermission annotation [SimpleManualPermissionEnforcement]
+ helper();
+ ~~~~~~~~~
+ 0 errors, 1 warnings
+ """
+ )
+ .expectFixDiffs(
+ """
+ Fix for src/Foo.java line 14: Annotate with @EnforcePermission:
+ @@ -12 +12
+ + @android.annotation.EnforcePermission("android.permission.READ_CONTACTS")
+ @@ -14 +15
+ - helper();
+ + test_enforcePermission();
+ """
+ )
+ }
+
+ fun testPermissionHelper_orSelfNotBubbledUp_warning() {
+ lint().files(
+ java(
+ """
+ import android.content.Context;
+ import android.test.ITest;
+
+ public class Foo extends ITest.Stub {
+ private Context mContext;
+
+ @android.annotation.PermissionMethod
+ private void helper() {
+ mContext.enforceCallingOrSelfPermission("android.permission.READ_CONTACTS", "foo");
+ }
+
+ @Override
+ public void test() throws android.os.RemoteException {
+ helper();
+ }
+ }
+ """
+ ).indented(),
+ *stubs
+ )
+ .run()
+ .expect(
+ """
+ src/Foo.java:14: Warning: ITest permission check can be converted to @EnforcePermission annotation [SimpleManualPermissionEnforcement]
+ helper();
+ ~~~~~~~~~
+ 0 errors, 1 warnings
+ """
+ )
+ .expectFixDiffs(
+ """
+ Fix for src/Foo.java line 14: Annotate with @EnforcePermission:
+ @@ -12 +12
+ + @android.annotation.EnforcePermission("android.permission.READ_CONTACTS")
+ @@ -14 +15
+ - helper();
+ + test_enforcePermission();
+ """
+ )
+ }
+
+ fun testPermissionHelperAllOf() {
+ lint().files(
+ java(
+ """
+ import android.content.Context;
+ import android.test.ITest;
+
+ public class Foo extends ITest.Stub {
+ private Context mContext;
+
+ @android.annotation.PermissionMethod(orSelf = true)
+ private void helper() {
+ mContext.enforceCallingOrSelfPermission("android.permission.READ_CONTACTS", "foo");
+ mContext.enforceCallingOrSelfPermission("android.permission.WRITE_CONTACTS", "foo");
+ }
+
+ @Override
+ public void test() throws android.os.RemoteException {
+ helper();
+ mContext.enforceCallingOrSelfPermission("FOO", "foo");
+ }
+ }
+ """
+ ).indented(),
+ *stubs
+ )
+ .run()
+ .expect(
+ """
+ src/Foo.java:16: Warning: ITest permission check should be converted to @EnforcePermission annotation [SimpleManualPermissionEnforcement]
+ mContext.enforceCallingOrSelfPermission("FOO", "foo");
+ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+ 0 errors, 1 warnings
+ """
+ )
+ .expectFixDiffs(
+ """
+ Fix for src/Foo.java line 16: Annotate with @EnforcePermission:
+ @@ -13 +13
+ + @android.annotation.EnforcePermission(allOf={"android.permission.READ_CONTACTS", "android.permission.WRITE_CONTACTS", "FOO"})
+ @@ -15 +16
+ - helper();
+ - mContext.enforceCallingOrSelfPermission("FOO", "foo");
+ + test_enforcePermission();
+ """
+ )
+ }
+
+
+ fun testPermissionHelperNested() {
+ lint().files(
+ java(
+ """
+ import android.content.Context;
+ import android.test.ITest;
+
+ public class Foo extends ITest.Stub {
+ private Context mContext;
+
+ @android.annotation.PermissionMethod(orSelf = true)
+ private void helperHelper() {
+ helper("android.permission.WRITE_CONTACTS");
+ }
+
+ @android.annotation.PermissionMethod(orSelf = true)
+ private void helper(@android.annotation.PermissionName String extraPermission) {
+ mContext.enforceCallingOrSelfPermission("android.permission.READ_CONTACTS", "foo");
+ }
+
+ @Override
+ public void test() throws android.os.RemoteException {
+ helperHelper();
+ }
+ }
+ """
+ ).indented(),
+ *stubs
+ )
+ .run()
+ .expect(
+ """
+ src/Foo.java:19: Warning: ITest permission check should be converted to @EnforcePermission annotation [SimpleManualPermissionEnforcement]
+ helperHelper();
+ ~~~~~~~~~~~~~~~
+ 0 errors, 1 warnings
+ """
+ )
+ .expectFixDiffs(
+ """
+ Fix for src/Foo.java line 19: Annotate with @EnforcePermission:
+ @@ -17 +17
+ + @android.annotation.EnforcePermission(allOf={"android.permission.WRITE_CONTACTS", "android.permission.READ_CONTACTS"})
+ @@ -19 +20
+ - helperHelper();
+ + test_enforcePermission();
+ """
+ )
+ }
+
+ fun testIfExpression() {
+ lint().files(
+ java(
+ """
+ import android.content.Context;
+ import android.test.ITest;
+ public class Foo extends ITest.Stub {
+ private Context mContext;
+ @Override
+ public void test() throws android.os.RemoteException {
+ if (mContext.checkCallingOrSelfPermission("android.permission.READ_CONTACTS", "foo")
+ != PackageManager.PERMISSION_GRANTED) {
+ throw new SecurityException("yikes!");
+ }
+ }
+ }
+ """
+ ).indented(),
+ *stubs
+ )
+ .run()
+ .expect(
+ """
+ src/Foo.java:7: Warning: ITest permission check should be converted to @EnforcePermission annotation [SimpleManualPermissionEnforcement]
+ if (mContext.checkCallingOrSelfPermission("android.permission.READ_CONTACTS", "foo")
+ ^
+ 0 errors, 1 warnings
+ """
+ )
+ .expectFixDiffs(
+ """
+ Fix for src/Foo.java line 7: Annotate with @EnforcePermission:
+ @@ -5 +5
+ + @android.annotation.EnforcePermission("android.permission.READ_CONTACTS")
+ @@ -7 +8
+ - if (mContext.checkCallingOrSelfPermission("android.permission.READ_CONTACTS", "foo")
+ - != PackageManager.PERMISSION_GRANTED) {
+ - throw new SecurityException("yikes!");
+ - }
+ + test_enforcePermission();
+ """
+ )
+ }
+
+ fun testIfExpression_orSelfFalse_warning() {
+ lint().files(
+ java(
+ """
+ import android.content.Context;
+ import android.test.ITest;
+ public class Foo extends ITest.Stub {
+ private Context mContext;
+ @Override
+ public void test() throws android.os.RemoteException {
+ if (mContext.checkCallingPermission("android.permission.READ_CONTACTS", "foo")
+ != PackageManager.PERMISSION_GRANTED) {
+ throw new SecurityException("yikes!");
+ }
+ }
+ }
+ """
+ ).indented(),
+ *stubs
+ )
+ .run()
+ .expect(
+ """
+ src/Foo.java:7: Warning: ITest permission check can be converted to @EnforcePermission annotation [SimpleManualPermissionEnforcement]
+ if (mContext.checkCallingPermission("android.permission.READ_CONTACTS", "foo")
+ ^
+ 0 errors, 1 warnings
+ """
+ )
+ .expectFixDiffs(
+ """
+ Fix for src/Foo.java line 7: Annotate with @EnforcePermission:
+ @@ -5 +5
+ + @android.annotation.EnforcePermission("android.permission.READ_CONTACTS")
+ @@ -7 +8
+ - if (mContext.checkCallingPermission("android.permission.READ_CONTACTS", "foo")
+ - != PackageManager.PERMISSION_GRANTED) {
+ - throw new SecurityException("yikes!");
+ - }
+ + test_enforcePermission();
+ """
+ )
+ }
+
+ fun testIfExpression_otherSideEffect_ignored() {
+ lint().files(
+ java(
+ """
+ import android.content.Context;
+ import android.test.ITest;
+ public class Foo extends ITest.Stub {
+ private Context mContext;
+ @Override
+ public void test() throws android.os.RemoteException {
+ if (mContext.checkCallingPermission("android.permission.READ_CONTACTS", "foo")
+ != PackageManager.PERMISSION_GRANTED) {
+ doSomethingElse();
+ throw new SecurityException("yikes!");
+ }
+ }
+ }
+ """
+ ).indented(),
+ *stubs
+ )
+ .run()
+ .expectClean()
+ }
+
+ fun testIfExpression_inlinedWithSideEffect_ignored() {
+ lint().files(
+ java(
+ """
+ import android.content.Context;
+ import android.test.ITest;
+ public class Foo extends ITest.Stub {
+ private Context mContext;
+ @Override
+ public void test() throws android.os.RemoteException {
+ if (somethingElse() && mContext.checkCallingPermission("android.permission.READ_CONTACTS", "foo")
+ != PackageManager.PERMISSION_GRANTED) {
+ throw new SecurityException("yikes!");
+ }
+ }
+
+ private boolean somethingElse() {
+ return true;
+ }
+ }
+ """
+ ).indented(),
+ *stubs
+ )
+ .run()
+ .expectClean()
+ }
+
+ fun testAnyOf_hardCodedAndVarArgs() {
+ lint().files(
+ java(
+ """
+ import android.content.Context;
+ import android.test.ITest;
+
+ public class Foo extends ITest.Stub {
+ private Context mContext;
+
+ @android.annotation.PermissionMethod(anyOf = true)
+ private void helperHelper() {
+ helper("FOO", "BAR");
+ }
+
+ @android.annotation.PermissionMethod(anyOf = true, value = {"BAZ", "BUZZ"})
+ private void helper(@android.annotation.PermissionName String... extraPermissions) {}
+
+ @Override
+ public void test() throws android.os.RemoteException {
+ helperHelper();
+ }
+ }
+ """
+ ).indented(),
+ *stubs
+ )
+ .run()
+ .expect(
+ """
+ src/Foo.java:17: Warning: ITest permission check can be converted to @EnforcePermission annotation [SimpleManualPermissionEnforcement]
+ helperHelper();
+ ~~~~~~~~~~~~~~~
+ 0 errors, 1 warnings
+ """
+ )
+ .expectFixDiffs(
+ """
+ Fix for src/Foo.java line 17: Annotate with @EnforcePermission:
+ @@ -15 +15
+ + @android.annotation.EnforcePermission(anyOf={"BAZ", "BUZZ", "FOO", "BAR"})
+ @@ -17 +18
+ - helperHelper();
+ + test_enforcePermission();
+ """
+ )
+ }
+
+
+ fun testAnyOfAllOf_mixedConsecutiveCalls_ignored() {
+ lint().files(
+ java(
+ """
+ import android.content.Context;
+ import android.test.ITest;
+
+ public class Foo extends ITest.Stub {
+ private Context mContext;
+
+ @android.annotation.PermissionMethod
+ private void allOfhelper() {
+ mContext.enforceCallingOrSelfPermission("FOO");
+ mContext.enforceCallingOrSelfPermission("BAR");
+ }
+
+ @android.annotation.PermissionMethod(anyOf = true, permissions = {"BAZ", "BUZZ"})
+ private void anyOfHelper() {}
+
+ @Override
+ public void test() throws android.os.RemoteException {
+ allOfhelper();
+ anyOfHelper();
+ }
+ }
+ """
+ ).indented(),
+ *stubs
+ )
+ .run()
+ .expectClean()
+ }
+
+ fun testAnyOfAllOf_mixedNestedCalls_ignored() {
+ lint().files(
+ java(
+ """
+ import android.content.Context;
+ import android.annotation.PermissionName;
+ import android.test.ITest;
+
+ public class Foo extends ITest.Stub {
+ private Context mContext;
+
+ @android.annotation.PermissionMethod(anyOf = true)
+ private void anyOfCheck(@PermissionName String... permissions) {
+ allOfCheck("BAZ", "BUZZ");
+ }
+
+ @android.annotation.PermissionMethod
+ private void allOfCheck(@PermissionName String... permissions) {}
+
+ @Override
+ public void test() throws android.os.RemoteException {
+ anyOfCheck("FOO", "BAR");
+ }
+ }
+ """
+ ).indented(),
+ *stubs
+ )
+ .run()
+ .expectClean()
+ }
+
+ companion object {
+ val stubs = arrayOf(
+ aidlStub,
+ contextStub,
+ binderStub,
+ permissionMethodStub,
+ permissionNameStub
+ )
+ }
+}
diff --git a/tools/lint/global/checks/src/test/java/com/google/android/lint/aidl/Stubs.kt b/tools/lint/global/checks/src/test/java/com/google/android/lint/aidl/Stubs.kt
new file mode 100644
index 0000000..2ec8fdd
--- /dev/null
+++ b/tools/lint/global/checks/src/test/java/com/google/android/lint/aidl/Stubs.kt
@@ -0,0 +1,88 @@
+package com.google.android.lint.aidl
+
+import com.android.tools.lint.checks.infrastructure.LintDetectorTest.java
+import com.android.tools.lint.checks.infrastructure.TestFile
+
+val aidlStub: TestFile = java(
+ """
+ package android.test;
+ public interface ITest extends android.os.IInterface {
+ public static abstract class Stub extends android.os.Binder implements android.test.ITest {
+ protected void test_enforcePermission() throws SecurityException {}
+ }
+ public void test() throws android.os.RemoteException;
+ }
+ """
+).indented()
+
+val contextStub: TestFile = java(
+ """
+ package android.content;
+ public class Context {
+ @android.annotation.PermissionMethod(orSelf = true)
+ public void enforceCallingOrSelfPermission(@android.annotation.PermissionName String permission, String message) {}
+ @android.annotation.PermissionMethod
+ public void enforceCallingPermission(@android.annotation.PermissionName String permission, String message) {}
+ @android.annotation.PermissionMethod(orSelf = true)
+ public int checkCallingOrSelfPermission(@android.annotation.PermissionName String permission, String message) {}
+ @android.annotation.PermissionMethod
+ public int checkCallingPermission(@android.annotation.PermissionName String permission, String message) {}
+ }
+ """
+).indented()
+
+val binderStub: TestFile = java(
+ """
+ package android.os;
+ public class Binder {
+ public static int getCallingUid() {}
+ }
+ """
+).indented()
+
+val permissionMethodStub: TestFile = java(
+ """
+ package android.annotation;
+
+ import static java.lang.annotation.ElementType.METHOD;
+ import static java.lang.annotation.RetentionPolicy.CLASS;
+
+ import java.lang.annotation.Retention;
+ import java.lang.annotation.Target;
+
+ @Retention(CLASS)
+ @Target({METHOD})
+ public @interface PermissionMethod {}
+ """
+).indented()
+
+val permissionNameStub: TestFile = java(
+ """
+ package android.annotation;
+
+ import static java.lang.annotation.ElementType.FIELD;
+ import static java.lang.annotation.ElementType.LOCAL_VARIABLE;
+ import static java.lang.annotation.ElementType.METHOD;
+ import static java.lang.annotation.ElementType.PARAMETER;
+ import static java.lang.annotation.RetentionPolicy.CLASS;
+
+ import java.lang.annotation.Retention;
+ import java.lang.annotation.Target;
+
+ @Retention(CLASS)
+ @Target({PARAMETER, METHOD, LOCAL_VARIABLE, FIELD})
+ public @interface PermissionName {}
+ """
+).indented()
+
+val manifestStub: TestFile = java(
+ """
+ package android;
+
+ public final class Manifest {
+ public static final class permission {
+ public static final String READ_CONTACTS="android.permission.READ_CONTACTS";
+ }
+ }
+ """.trimIndent()
+)
\ No newline at end of file