Merge changes I0fbd41b6,If791b6d8 into main am: dcd948462b (1d11f085) · Commits · e / os / android_frameworks_base

tools/lint/common/src/main/java/com/google/android/lint/aidl/EnforcePermissionUtils.kt

+15 −17

Original line number	Original line	Diff line number	Diff line
	@@ -24,33 +24,31 @@ import com.intellij.psi.PsiReferenceList
	import org.jetbrains.uast.UMethod		import org.jetbrains.uast.UMethod

	/**		/**
	* Given a UMethod, determine if this method is		* Given a UMethod, determine if this method is the entrypoint to an interface
	* the entrypoint to an interface generated by AIDL,		* generated by AIDL, returning the interface name if so, otherwise returning
	* returning the interface name if so, otherwise returning null		* null
	*/		*/
	fun getContainingAidlInterface(context: JavaContext, node: UMethod): String? {		fun getContainingAidlInterface(context: JavaContext, node: UMethod): String? {
	if (!isContainedInSubclassOfStub(context, node)) return null		val containingStub = containingStub(context, node) ?: return null
	for (superMethod in node.findSuperMethods()) {		val superMethod = node.findSuperMethods(containingStub)
	for (extendsInterface in superMethod.containingClass?.extendsList?.referenceElements		if (superMethod.isEmpty()) return null
	?: continue) {		return containingStub.containingClass?.name
	if (extendsInterface.qualifiedName == IINTERFACE_INTERFACE) {
	return superMethod.containingClass?.name
	}
	}
	}
	return null
	}		}

	fun isContainedInSubclassOfStub(context: JavaContext, node: UMethod?): Boolean {		/* Returns the containing Stub class if any. This is not sufficient to infer
			* that the method itself extends an AIDL generated method. See
			* getContainingAidlInterface for that purpose.
			*/
			fun containingStub(context: JavaContext, node: UMethod?): PsiClass? {
	var superClass = node?.containingClass?.superClass		var superClass = node?.containingClass?.superClass
	while (superClass != null) {		while (superClass != null) {
	if (isStub(context, superClass)) return true		if (isStub(context, superClass)) return superClass
	superClass = superClass.superClass		superClass = superClass.superClass
	}		}
	return false		return null
	}		}

	fun isStub(context: JavaContext, psiClass: PsiClass?): Boolean {		private fun isStub(context: JavaContext, psiClass: PsiClass?): Boolean {
	if (psiClass == null) return false		if (psiClass == null) return false
	if (psiClass.name != "Stub") return false		if (psiClass.name != "Stub") return false
	if (!context.evaluator.isStatic(psiClass)) return false		if (!context.evaluator.isStatic(psiClass)) return false

tools/lint/framework/checks/src/main/java/com/google/android/lint/AndroidFrameworkIssueRegistry.kt

+2 −0

Original line number	Original line	Diff line number	Diff line
	@@ -20,6 +20,7 @@ import com.android.tools.lint.client.api.IssueRegistry
	import com.android.tools.lint.client.api.Vendor		import com.android.tools.lint.client.api.Vendor
	import com.android.tools.lint.detector.api.CURRENT_API		import com.android.tools.lint.detector.api.CURRENT_API
	import com.google.android.lint.parcel.SaferParcelChecker		import com.google.android.lint.parcel.SaferParcelChecker
			import com.google.android.lint.aidl.PermissionAnnotationDetector
	import com.google.auto.service.AutoService		import com.google.auto.service.AutoService

	@AutoService(IssueRegistry::class)		@AutoService(IssueRegistry::class)
	@@ -37,6 +38,7 @@ class AndroidFrameworkIssueRegistry : IssueRegistry() {
	SaferParcelChecker.ISSUE_UNSAFE_API_USAGE,		SaferParcelChecker.ISSUE_UNSAFE_API_USAGE,
	// TODO: Currently crashes due to OOM issue		// TODO: Currently crashes due to OOM issue
	// PackageVisibilityDetector.ISSUE_PACKAGE_NAME_NO_PACKAGE_VISIBILITY_FILTERS,		// PackageVisibilityDetector.ISSUE_PACKAGE_NAME_NO_PACKAGE_VISIBILITY_FILTERS,
			PermissionAnnotationDetector.ISSUE_MISSING_PERMISSION_ANNOTATION,
	PermissionMethodDetector.ISSUE_PERMISSION_METHOD_USAGE,		PermissionMethodDetector.ISSUE_PERMISSION_METHOD_USAGE,
	PermissionMethodDetector.ISSUE_CAN_BE_PERMISSION_METHOD,		PermissionMethodDetector.ISSUE_CAN_BE_PERMISSION_METHOD,
	)		)

tools/lint/framework/checks/src/main/java/com/google/android/lint/PermissionAnnotationDetector.kt

0 → 100644

+87 −0

Original line number	Original line	Diff line number	Diff line
			/*
			* Copyright (C) 2023 The Android Open Source Project
			*
			* Licensed under the Apache License, Version 2.0 (the "License");
			* you may not use this file except in compliance with the License.
			* You may obtain a copy of the License at
			*
			* http://www.apache.org/licenses/LICENSE-2.0
			*
			* Unless required by applicable law or agreed to in writing, software
			* distributed under the License is distributed on an "AS IS" BASIS,
			* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
			* See the License for the specific language governing permissions and
			* limitations under the License.
			*/

			package com.google.android.lint.aidl

			import com.android.tools.lint.detector.api.Category
			import com.android.tools.lint.detector.api.Implementation
			import com.android.tools.lint.detector.api.Issue
			import com.android.tools.lint.detector.api.JavaContext
			import com.android.tools.lint.detector.api.Scope
			import com.android.tools.lint.detector.api.Severity
			import org.jetbrains.uast.UBlockExpression
			import org.jetbrains.uast.UMethod

			/**
			* Ensures all AIDL-generated methods are annotated.
			*
			* This detector is run on system_server to validate that any method that may
			* be exposed via an AIDL interface is permission-annotated. That is, it must
			* have one of the following annotation:
			* - @EnforcePermission
			* - @RequiresNoPermission
			* - @PermissionManuallyEnforced
			*/
			class PermissionAnnotationDetector : AidlImplementationDetector() {

			override fun visitAidlMethod(
			context: JavaContext,
			node: UMethod,
			interfaceName: String,
			body: UBlockExpression
			) {
			if (context.evaluator.isAbstract(node)) return

			if (AIDL_PERMISSION_ANNOTATIONS.any { node.hasAnnotation(it) }) return

			context.report(
			ISSUE_MISSING_PERMISSION_ANNOTATION,
			node,
			context.getLocation(node),
			"The method ${node.name} is not permission-annotated."
			)
			}

			companion object {

			private val EXPLANATION_MISSING_PERMISSION_ANNOTATION = """
			Interfaces that are exposed by system_server are required to have an annotation which
			denotes the type of permission enforced. There are 3 possible options:
			- @EnforcePermission
			- @RequiresNoPermission
			- @PermissionManuallyEnforced
			See the documentation of each annotation for further details.

			The annotation on the Java implementation must be the same that the AIDL interface
			definition. This is verified by a lint in the build system.
			""".trimIndent()

			@JvmField
			val ISSUE_MISSING_PERMISSION_ANNOTATION = Issue.create(
			id = "MissingPermissionAnnotation",
			briefDescription = "No permission annotation on exposed AIDL interface.",
			explanation = EXPLANATION_MISSING_PERMISSION_ANNOTATION,
			category = Category.CORRECTNESS,
			priority = 5,
			severity = Severity.ERROR,
			implementation = Implementation(
			PermissionAnnotationDetector::class.java,
			Scope.JAVA_FILE_SCOPE
			),
			enabledByDefault = false
			)
			}
			}

tools/lint/framework/checks/src/test/java/com/google/android/lint/PermissionAnnotationDetectorTest.kt

0 → 100644

+134 −0

Original line number	Original line	Diff line number	Diff line
			/*
			* Copyright (C) 2023 The Android Open Source Project
			*
			* Licensed under the Apache License, Version 2.0 (the "License");
			* you may not use this file except in compliance with the License.
			* You may obtain a copy of the License at
			*
			* http://www.apache.org/licenses/LICENSE-2.0
			*
			* Unless required by applicable law or agreed to in writing, software
			* distributed under the License is distributed on an "AS IS" BASIS,
			* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
			* See the License for the specific language governing permissions and
			* limitations under the License.
			*/

			package com.google.android.lint.aidl

			import com.android.tools.lint.checks.infrastructure.LintDetectorTest
			import com.android.tools.lint.checks.infrastructure.TestFile
			import com.android.tools.lint.checks.infrastructure.TestLintTask
			import com.android.tools.lint.detector.api.Detector
			import com.android.tools.lint.detector.api.Issue

			@Suppress("UnstableApiUsage")
			class PermissionAnnotationDetectorTest : LintDetectorTest() {
			override fun getDetector(): Detector = PermissionAnnotationDetector()

			override fun getIssues(): List<Issue> = listOf(
			PermissionAnnotationDetector.ISSUE_MISSING_PERMISSION_ANNOTATION,
			)

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

			/** No issue scenario */

			fun testDoesNotDetectIssuesInCorrectScenario() {
			lint().files(
			java(
			"""
			public class Foo extends IFoo.Stub {
			@Override
			@android.annotation.EnforcePermission("android.Manifest.permission.READ_CONTACTS")
			public void testMethod() { }
			}
			"""
			).indented(),
			*stubs
			)
			.run()
			.expectClean()
			}

			fun testMissingAnnotation() {
			lint().files(
			java(
			"""
			public class Bar extends IBar.Stub {
			public void testMethod() { }
			}
			"""
			).indented(),
			*stubs
			)
			.run()
			.expect(
			"""
			src/Bar.java:2: Error: The method testMethod is not permission-annotated. [MissingPermissionAnnotation]
			public void testMethod() { }
			~~~~~~~~~~~~~~~~~~~~~~~~~~~~
			1 errors, 0 warnings
			"""
			)
			}

			fun testNoIssueWhenExtendingWithAnotherSubclass() {
			lint().files(
			java(
			"""
			public class Foo extends IFoo.Stub {
			@Override
			@android.annotation.EnforcePermission(android.Manifest.permission.READ_PHONE_STATE)
			public void testMethod() { }
			// not an AIDL method, just another method
			public void someRandomMethod() { }
			}
			""").indented(),
			java(
			"""
			public class Baz extends Bar {
			@Override
			public void someRandomMethod() { }
			}
			""").indented(),
			*stubs
			)
			.run()
			.expectClean()
			}

			/* Stubs */

			// A service with permission annotation on the method.
			private val interfaceIFoo: TestFile = java(
			"""
			public interface IFoo extends android.os.IInterface {
			public static abstract class Stub extends android.os.Binder implements IFoo {
			}
			@Override
			@android.annotation.EnforcePermission(android.Manifest.permission.READ_PHONE_STATE)
			public void testMethod();
			@Override
			@android.annotation.RequiresNoPermission
			public void testMethodNoPermission();
			@Override
			@android.annotation.PermissionManuallyEnforced
			public void testMethodManual();
			}
			"""
			).indented()

			// A service with no permission annotation.
			private val interfaceIBar: TestFile = java(
			"""
			public interface IBar extends android.os.IInterface {
			public static abstract class Stub extends android.os.Binder implements IBar {
			}
			public void testMethod();
			}
			"""
			).indented()

			private val stubs = arrayOf(interfaceIFoo, interfaceIBar)
			}

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

+4 −3

Original line number	Original line	Diff line number	Diff line
	@@ -168,7 +168,7 @@ class EnforcePermissionDetector : Detector(), SourceCodeScanner {
	annotationInfo.origin == AnnotationOrigin.METHOD) {		annotationInfo.origin == AnnotationOrigin.METHOD) {
	/* Ignore implementations that are not a sub-class of Stub (i.e., Proxy). */		/* Ignore implementations that are not a sub-class of Stub (i.e., Proxy). */
	val uMethod = element as? UMethod ?: return		val uMethod = element as? UMethod ?: return
	if (!isContainedInSubclassOfStub(context, uMethod)) {		if (getContainingAidlInterface(context, uMethod) == null) {
	return		return
	}		}
	val overridingMethod = element.sourcePsi as PsiMethod		val overridingMethod = element.sourcePsi as PsiMethod
	@@ -184,7 +184,8 @@ class EnforcePermissionDetector : Detector(), SourceCodeScanner {
	if (context.evaluator.isAbstract(node)) return		if (context.evaluator.isAbstract(node)) return
	if (!node.hasAnnotation(ANNOTATION_ENFORCE_PERMISSION)) return		if (!node.hasAnnotation(ANNOTATION_ENFORCE_PERMISSION)) return

	if (!isContainedInSubclassOfStub(context, node)) {		val stubClass = containingStub(context, node)
			if (stubClass == null) {
	context.report(		context.report(
	ISSUE_MISUSING_ENFORCE_PERMISSION,		ISSUE_MISUSING_ENFORCE_PERMISSION,
	node,		node,
	@@ -196,7 +197,7 @@ class EnforcePermissionDetector : Detector(), SourceCodeScanner {

	/* Check that we are connected to the super class */		/* Check that we are connected to the super class */
	val overridingMethod = node as PsiMethod		val overridingMethod = node as PsiMethod
	val parents = overridingMethod.findSuperMethods()		val parents = overridingMethod.findSuperMethods(stubClass)
	if (parents.isEmpty()) {		if (parents.isEmpty()) {
	context.report(		context.report(
	ISSUE_MISUSING_ENFORCE_PERMISSION,		ISSUE_MISUSING_ENFORCE_PERMISSION,