Merge "Add EnforcePermissionHelper lint" (6a624d9e) · Commits · e / os / android_frameworks_base

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

+2 −0

Original line number	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.detector.api.CURRENT_API
		import com.google.android.lint.aidl.EnforcePermissionDetector
		import com.google.android.lint.aidl.EnforcePermissionHelperDetector
		import com.google.android.lint.aidl.ManualPermissionCheckDetector
		import com.google.android.lint.parcel.SaferParcelChecker
		import com.google.auto.service.AutoService
		@@ -38,6 +39,7 @@ class AndroidFrameworkIssueRegistry : IssueRegistry() {
		CallingSettingsNonUserGetterMethodsDetector.ISSUE_NON_USER_GETTER_CALLED,
		EnforcePermissionDetector.ISSUE_MISSING_ENFORCE_PERMISSION,
		EnforcePermissionDetector.ISSUE_MISMATCHING_ENFORCE_PERMISSION,
		EnforcePermissionHelperDetector.ISSUE_ENFORCE_PERMISSION_HELPER,
		ManualPermissionCheckDetector.ISSUE_USE_ENFORCE_PERMISSION_ANNOTATION,
		SaferParcelChecker.ISSUE_UNSAFE_API_USAGE,
		PackageVisibilityDetector.ISSUE_PACKAGE_NAME_NO_PACKAGE_VISIBILITY_FILTERS,

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

0 → 100644

+130 −0

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.intellij.psi.PsiElement
		import org.jetbrains.uast.UBlockExpression
		import org.jetbrains.uast.UDeclarationsExpression
		import org.jetbrains.uast.UExpression
		import org.jetbrains.uast.UElement
		import org.jetbrains.uast.UMethod

		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 (!node.hasAnnotation(ANNOTATION_ENFORCE_PERMISSION)) return

		val targetExpression = "super.${node.name}$HELPER_SUFFIX()"

		val body = node.uastBody as? UBlockExpression
		if (body == null) {
		context.report(
		ISSUE_ENFORCE_PERMISSION_HELPER,
		context.getLocation(node),
		"Method must start with $targetExpression",
		)
		return
		}

		val firstExpression = body.expressions.firstOrNull()
		if (firstExpression == null) {
		context.report(
		ISSUE_ENFORCE_PERMISSION_HELPER,
		context.getLocation(node),
		"Method must start with $targetExpression",
		)
		return
		}

		val firstExpressionSource = firstExpression.asSourceString()
		.filterNot(Char::isWhitespace)

		if (firstExpressionSource != targetExpression) {
		val locationTarget = getLocationTarget(firstExpression)
		val expressionLocation = context.getLocation(locationTarget)
		val indent = " ".repeat(expressionLocation.start?.column ?: 0)

		val fix = fix()
		.replace()
		.range(expressionLocation)
		.beginning()
		.with("$targetExpression;\n\n$indent")
		.reformat(true)
		.autoFix()
		.build()

		context.report(
		ISSUE_ENFORCE_PERMISSION_HELPER,
		context.getLocation(node),
		"Method must start with $targetExpression",
		fix
		)
		}
		}
		}

		companion object {
		private const val HELPER_SUFFIX = "_enforcePermission"

		private const val EXPLANATION = """
		When @EnforcePermission is applied, the AIDL compiler generates a Stub method to do the
		permission check called yourMethodName$HELPER_SUFFIX.

		You must call this method as the first expression in your implementation.
		"""

		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
		)
		)

		/**
		* 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/checks/src/test/java/com/google/android/lint/aidl/EnforcePermissionHelperDetectorTest.kt

0 → 100644

+159 −0

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.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)

		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 super.test_enforcePermission() [MissingEnforcePermissionHelper]
		@Override
		^
		1 errors, 0 warnings
		"""
		)
		.expectFixDiffs(
		"""
		Autofix for src/Foo.java line 5: Replace with super.test_enforcePermission();...:
		@@ -8 +8
		+ super.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 super.test_enforcePermission() [MissingEnforcePermissionHelper]
		@Override
		^
		1 errors, 0 warnings
		"""
		)
		.expectFixDiffs(
		"""
		Autofix for src/Foo.java line 5: Replace with super.test_enforcePermission();...:
		@@ -8 +8
		+ super.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 super.test_enforcePermission() [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()
		}

		companion object {
		val stubs = arrayOf(aidlStub, contextStub, binderStub)
		}
		}

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

+0 −77

Original line number	Diff line number	Diff line
		@@ -17,7 +17,6 @@
		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
		@@ -361,82 +360,6 @@ class ManualPermissionCheckDetectorTest : LintDetectorTest() {


		companion object {
		private 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 {}
		public void test() throws android.os.RemoteException;
		}
		"""
		).indented()

		private val contextStub: TestFile = java(
		"""
		package android.content;
		public class Context {
		@android.content.pm.PermissionMethod
		public void enforceCallingOrSelfPermission(@android.content.pm.PermissionName String permission, String message) {}
		}
		"""
		).indented()

		private val binderStub: TestFile = java(
		"""
		package android.os;
		public class Binder {
		public static int getCallingUid() {}
		}
		"""
		).indented()

		private val permissionMethodStub: TestFile = java(
		"""
		package android.content.pm;

		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()

		private val permissionNameStub: TestFile = java(
		"""
		package android.content.pm;

		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()

		private 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()
		)

		val stubs = arrayOf(
		aidlStub,
		contextStub,

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

0 → 100644

+80 −0

Original line number	Diff line number	Diff line
		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 {}
		public void test() throws android.os.RemoteException;
		}
		"""
		).indented()

		val contextStub: TestFile = java(
		"""
		package android.content;
		public class Context {
		@android.content.pm.PermissionMethod
		public void enforceCallingOrSelfPermission(@android.content.pm.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.content.pm;

		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.content.pm;

		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