Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit a91ea4a1 authored by Ömer Faruk Yılmaz's avatar Ömer Faruk Yılmaz
Browse files

Add PermissionManager enforcement support to RequiresPermissionDetector 

The `RequiresPermissionDetector` did not recognize permission enforcement
checks performed using `android.permission.PermissionManager`. This
caused the lint check to incorrectly flag methods as having "too broad" a
`@RequiresPermission` annotation, even when the permission was correctly
enforced at runtime.

Bug: 409768276
Test: atest --host AndroidGlobalLintCheckerTest:com.google.android.lint.aidl.RequiresPermissionDetectorTest
Flag: EXEMPT new lint
Change-Id: I990f0fe7d8edb126aaf75179328298edd9880407
parent f67e48e3

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

+1 −0
Original line number Diff line number Diff line
@@ -23,6 +23,7 @@ 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"

const val CLASS_PERMISSION_CHECKER = "android.content.PermissionChecker"

const val CLASS_PERMISSION_MANAGER = "android.permission.PermissionManager"



// Enforce permission APIs

val ENFORCE_PERMISSION_METHODS = listOf(

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

+15 −10
Original line number Diff line number Diff line
@@ -28,6 +28,7 @@ import com.android.tools.lint.detector.api.Severity 
import com.android.tools.lint.detector.api.SourceCodeScanner

import com.google.android.lint.CLASS_CONTEXT

import com.google.android.lint.CLASS_PERMISSION_CHECKER

import com.google.android.lint.CLASS_PERMISSION_MANAGER

import com.intellij.psi.PsiClass

import com.intellij.psi.PsiMethod

import org.jetbrains.uast.UAnnotation
@@ -168,20 +169,22 @@ class RequiresPermissionDetector : Detector(), SourceCodeScanner { 
        }



        private fun checkEnforcement(node: UCallExpression, method: PsiMethod) {

            if (context.evaluator.isMemberInSubClassOf(method, CLASS_CONTEXT, false) &&

                method.name.matches(CONTEXT_ENFORCEMENT_METHOD_REGEX)) {

                node.valueArguments.getOrNull(0)?.let { arg ->

            fun extractPermissionFromArgument(node: UCallExpression, index: Int) {

                node.valueArguments.getOrNull(index)?.let { arg ->

                    ConstantEvaluator.evaluate(context, arg)?.toString()?.let {

                        enforcedPermissions.allOf.add(it)

                    }

                }

            } else if (context.evaluator.isMemberInSubClassOf(method, CLASS_PERMISSION_CHECKER, false) &&

                method.name.matches(PERMISSION_CHECKER_ENFORCEMENT_METHOD_REGEX)) {

                node.valueArguments.getOrNull(1)?.let { arg ->

                    ConstantEvaluator.evaluate(context, arg)?.toString()?.let {

                        enforcedPermissions.allOf.add(it)

                    }

            }

            if (context.evaluator.isMemberInSubClassOf(method, CLASS_CONTEXT, false)

                && method.name.matches(CONTEXT_ENFORCEMENT_METHOD_REGEX)) {

                extractPermissionFromArgument(node, 0)

            } else if (context.evaluator.isMemberInSubClassOf(method, CLASS_PERMISSION_CHECKER, false)

                && method.name.matches(PERMISSION_CHECKER_ENFORCEMENT_METHOD_REGEX)) {

                extractPermissionFromArgument(node, 1)

            } else if (context.evaluator.isMemberInSubClassOf(method, CLASS_PERMISSION_MANAGER, false)

                && method.name.matches(PERMISSION_MANAGER_ENFORCEMENT_METHOD_REGEX)) {

                extractPermissionFromArgument(node, 0)

            }

        }

    }
@@ -269,6 +272,8 @@ class RequiresPermissionDetector : Detector(), SourceCodeScanner { 
            "^(enforce|check)(Calling)?(OrSelf)?Permission$".toRegex()

        private val PERMISSION_CHECKER_ENFORCEMENT_METHOD_REGEX =

            "^check.*Permission$".toRegex()

        private val PERMISSION_MANAGER_ENFORCEMENT_METHOD_REGEX =

            "^checkPermission.*".toRegex()



        @JvmField

        val ISSUE_MISSING_OR_MISMATCHED_REQUIRES_PERMISSION_ANNOTATION = Issue.create(

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

+34 −2
Original line number Diff line number Diff line
@@ -252,6 +252,23 @@ class RequiresPermissionDetectorTest : LintDetectorTest() { 
            .expectClean()

    }



    fun testPermissionEnforcementViaPermissionManager_CorrectlyAnnotatedAndEnforced_Passes() {

        lint().files(kotlin("""

            package test.pkg

            import android.permission.PermissionManager

            class FooBinder(val permissionManager: PermissionManager): IFoo.Stub() {

                @android.annotation.RequiresPermission(android.Manifest.permission.BLUETOOTH_CONNECT)

                override fun connect() {

                    permissionManager.checkPermissionForDataDeliveryFromDataSource(android.Manifest.permission.BLUETOOTH_CONNECT, null, "connect")

                }

            }

            """).indented(),

            *stubs

        )

            .run()

            .expectClean()

    }



    fun testAllOf_MatchingAnnotationButIncorrectAnyOfInsteadOfAllOf_Fails() {

        lint().files(kotlin("""

            package test.pkg
@@ -602,7 +619,7 @@ class RequiresPermissionDetectorTest : LintDetectorTest() { 
        """

    ).indented()



    private val permissionCheckStub: TestFile = java(

    private val permissionCheckerStub: TestFile = java(

        """

        package android.content;

        public class PermissionChecker {
@@ -611,6 +628,20 @@ class RequiresPermissionDetectorTest : LintDetectorTest() { 
        """

    ).indented()



    private val permissionManagerStub: TestFile = java(

        """

        package android.permission;

        public class PermissionManager {

            public int checkPermissionForDataDeliveryFromDataSource(

                    String permission,

                    AttributionSource attributionSource,

                    String message) {

                return 0;

            }

        }

        """

    ).indented()



    private val binderStub: TestFile = java("""

        package android.os;

        public class Binder {
@@ -658,7 +689,8 @@ class RequiresPermissionDetectorTest : LintDetectorTest() { 
        contextStub,

        broadcastStub,

        intentStub,

        permissionCheckStub,

        permissionCheckerStub,

        permissionManagerStub,

        binderStub,

        interfaceIFooStub,

    )