Merge EnforcePermissionHelperDetector with EnforcePermissionDetector (aec895aa) · Commits · e / os / android_frameworks_base

tools/lint/global/checks/src/main/java/com/google/android/lint/AndroidGlobalIssueRegistry.kt

+3 −3

Original line number	Diff line number	Diff line
		@@ -20,7 +20,6 @@ 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

		@@ -30,7 +29,8 @@ class AndroidGlobalIssueRegistry : IssueRegistry() {
		override val issues = listOf(
		EnforcePermissionDetector.ISSUE_MISSING_ENFORCE_PERMISSION,
		EnforcePermissionDetector.ISSUE_MISMATCHING_ENFORCE_PERMISSION,
		EnforcePermissionHelperDetector.ISSUE_ENFORCE_PERMISSION_HELPER,
		EnforcePermissionDetector.ISSUE_ENFORCE_PERMISSION_HELPER,
		EnforcePermissionDetector.ISSUE_MISUSING_ENFORCE_PERMISSION,
		SimpleManualPermissionEnforcementDetector.ISSUE_SIMPLE_MANUAL_PERMISSION_ENFORCEMENT,
		)

tools/lint/global/checks/src/main/java/com/google/android/lint/aidl/EnforcePermissionDetector.kt

+135 −37

Original line number	Diff line number	Diff line
		@@ -30,31 +30,34 @@ 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.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.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.toUElement
		import org.jetbrains.uast.skipParenthesizedExprDown

		import java.util.EnumSet

		/**
		* 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.
		* Lint Detector that ensures consistency when using the @EnforcePermission
		* annotation. Multiple verifications are implemented:
		*
		* 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
		* the correct annotation on each methods. Even if the subclass does not
		* have the annotation, visitAnnotationUsage will be called which allows us
		* to capture the issue.
		* 2. Visit any method, 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.
		* 3. When annotated, ensures that the first instruction is to call the helper
		* method (or the parent helper).
		*/
		class EnforcePermissionDetector : Detector(), SourceCodeScanner {

		@@ -62,9 +65,8 @@ class EnforcePermissionDetector : Detector(), SourceCodeScanner {
		return listOf(ANNOTATION_ENFORCE_PERMISSION)
		}

		override fun getApplicableUastTypes(): List<Class<out UElement>> {
		return listOf(UAnnotation::class.java)
		}
		override fun getApplicableUastTypes(): List<Class<out UElement?>> =
		listOf(UMethod::class.java)

		private fun annotationValueGetChildren(elem: PsiElement): Array<PsiElement> {
		if (elem is PsiArrayInitializerMemberValue)
		@@ -123,11 +125,6 @@ class EnforcePermissionDetector : Detector(), SourceCodeScanner {
		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)
		@@ -163,40 +160,102 @@ class EnforcePermissionDetector : Detector(), SourceCodeScanner {
		) {
		if (usageInfo.type == AnnotationUsageType.METHOD_OVERRIDE &&
		annotationInfo.origin == AnnotationOrigin.METHOD) {
		/* Ignore implementations that are not a sub-class of Stub (i.e., Proxy). */
		val uMethod = element as? UMethod ?: return
		if (!isContainedInSubclassOfStub(context, uMethod)) {
		return
		}
		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) {
		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 method = node.uastParent as? UMethod ?: return
		val overridingMethod = method as PsiMethod

		/* Check that we are connected to the super class */
		val overridingMethod = node 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,
		compareMethods(context, node, overridingMethod,
		overriddenMethod, checkEquivalence = false)
		}

		/* Check that the helper is called as a first instruction */
		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"

		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.
		The @EnforcePermission annotation is used to delegate the verification of the caller's
		permissions to a generated AIDL method.

		In order to surface that information to platform developers, the same annotation must be
		used on the implementation class or methods.

		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_MISSING_ENFORCE_PERMISSION: Issue = Issue.create(
		@@ -224,5 +283,44 @@ class EnforcePermissionDetector : Detector(), SourceCodeScanner {
		EnumSet.of(Scope.JAVA_FILE, Scope.TEST_SOURCES)
		)
		)

		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(
		EnforcePermissionDetector::class.java,
		EnumSet.of(Scope.JAVA_FILE, Scope.TEST_SOURCES)
		)
		)

		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,
		EnumSet.of(Scope.JAVA_FILE, Scope.TEST_SOURCES)
		)
		)

		/**
		* 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
		}
		}
		}

tools/lint/global/checks/src/main/java/com/google/android/lint/aidl/EnforcePermissionHelperDetector.kt

deleted100644 → 0

+0 −151

Original line number	Diff line number	Diff line
		/*
		* 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

		import java.util.EnumSet

		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,
		EnumSet.of(Scope.JAVA_FILE, Scope.TEST_SOURCES)
		)
		)

		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(
		EnforcePermissionHelperDetector::class.java,
		EnumSet.of(Scope.JAVA_FILE, Scope.TEST_SOURCES)
		)
		)

		/**
		* 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
		}
		}
		}

tools/lint/global/checks/src/test/java/com/google/android/lint/aidl/EnforcePermissionHelperDetectorCodegenTest.kt

+2 −2

Original line number	Diff line number	Diff line
		@@ -25,10 +25,10 @@ import com.android.tools.lint.detector.api.Issue

		@Suppress("UnstableApiUsage")
		class EnforcePermissionHelperDetectorCodegenTest : LintDetectorTest() {
		override fun getDetector(): Detector = EnforcePermissionHelperDetector()
		override fun getDetector(): Detector = EnforcePermissionDetector()

		override fun getIssues(): List<Issue> = listOf(
		EnforcePermissionHelperDetector.ISSUE_ENFORCE_PERMISSION_HELPER
		EnforcePermissionDetector.ISSUE_ENFORCE_PERMISSION_HELPER
		)

		override fun lint(): TestLintTask = super.lint().allowMissingSdk(true)

tools/lint/global/checks/src/test/java/com/google/android/lint/aidl/EnforcePermissionHelperDetectorTest.kt

+3 −3

Original line number	Diff line number	Diff line
		@@ -20,10 +20,10 @@ 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 getDetector() = EnforcePermissionDetector()
		override fun getIssues() = listOf(
		EnforcePermissionHelperDetector.ISSUE_ENFORCE_PERMISSION_HELPER,
		EnforcePermissionHelperDetector.ISSUE_MISUSING_ENFORCE_PERMISSION
		EnforcePermissionDetector.ISSUE_ENFORCE_PERMISSION_HELPER,
		EnforcePermissionDetector.ISSUE_MISUSING_ENFORCE_PERMISSION
		)

		override fun lint(): TestLintTask = super.lint().allowMissingSdk()