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

Commit 69dec36a authored by mattgilbride's avatar mattgilbride
Browse files

Refactor ManualPermissionCheckDetector for @PermissionMethod 

Uses @PermissionMethod/@PermissionName annotations to find and auto-fix
"manual" permission checks that can be migrated to @EnforcePermission
(as opposed to a hard-coded list of known permission methods).

Bug: 247537842
Test: ManualPermissionCheckDetectorTest
Change-Id: Ibd842f0e64e2006954bffa24c7fe394903dd7b61
parent d43602bb

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

+1 −5
Original line number Diff line number Diff line
@@ -26,6 +26,7 @@ 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.google.android.lint.aidl.hasPermissionMethodAnnotation

import com.intellij.psi.PsiType

import org.jetbrains.uast.UAnnotation

import org.jetbrains.uast.UBlockExpression
@@ -149,11 +150,6 @@ class PermissionMethodDetector : Detector(), SourceCodeScanner { 
            enabledByDefault = false

        )



        private fun hasPermissionMethodAnnotation(method: UMethod): Boolean = method.annotations

            .any {

                it.hasQualifiedName(ANNOTATION_PERMISSION_METHOD)

            }



        private fun isPermissionMethodReturnType(method: UMethod): Boolean =

            listOf(PsiType.VOID, PsiType.INT, PsiType.BOOLEAN).contains(method.returnType)

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

+74 −55
Original line number Diff line number Diff line
@@ -18,37 +18,54 @@ package com.google.android.lint.aidl 


import com.android.tools.lint.detector.api.JavaContext

import com.android.tools.lint.detector.api.Location

import com.intellij.psi.PsiVariable

import com.android.tools.lint.detector.api.getUMethod

import org.jetbrains.kotlin.psi.psiUtil.parameterIndex

import org.jetbrains.uast.UCallExpression

import org.jetbrains.uast.ULiteralExpression

import org.jetbrains.uast.UQualifiedReferenceExpression

import org.jetbrains.uast.USimpleNameReferenceExpression

import org.jetbrains.uast.asRecursiveLogString

import org.jetbrains.uast.evaluateString

import org.jetbrains.uast.visitor.AbstractUastVisitor



/**

 * Helper ADT class that facilitates the creation of lint auto fixes

 * Helper class that facilitates the creation of lint auto fixes

 *

 * Handles "Single" permission checks that should be migrated to @EnforcePermission(...), as well as consecutive checks

 * that should be migrated to @EnforcePermission(allOf={...})

 *

 * TODO: handle anyOf style annotations

 */

sealed class EnforcePermissionFix {

    abstract fun locations(): List<Location>

    abstract fun javaAnnotationParameter(): String



    fun javaAnnotation(): String = "@$ANNOTATION_ENFORCE_PERMISSION(${javaAnnotationParameter()})"

data class EnforcePermissionFix(

    val locations: List<Location>,

    val permissionNames: List<String>

) {

    val annotation: String

        get() {

            val quotedPermissions = permissionNames.joinToString(", ") { """"$it"""" }

            val annotationParameter =

                if (permissionNames.size > 1) "allOf={$quotedPermissions}" else quotedPermissions

            return "@$ANNOTATION_ENFORCE_PERMISSION($annotationParameter)"

        }



    companion object {

        fun fromCallExpression(callExpression: UCallExpression, context: JavaContext): SingleFix =

            SingleFix(

                getPermissionCheckLocation(context, callExpression),

                getPermissionCheckArgumentValue(callExpression)

        /**

         * 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? =

            if (isPermissionMethodCall(callExpression)) {

                EnforcePermissionFix(

                    listOf(getPermissionCheckLocation(context, callExpression)),

                    getPermissionCheckValues(callExpression)

                )

            } else null





        fun maybeAddManifestPrefix(permissionName: String): String =

            if (permissionName.contains(".")) permissionName

            else "android.Manifest.permission.$permissionName"

        fun compose(individuals: List<EnforcePermissionFix>): EnforcePermissionFix =

            EnforcePermissionFix(

                individuals.flatMap { it.locations },

                individuals.flatMap { it.permissionNames }

            )



        /**

         * Given a permission check, get its proper location
@@ -70,49 +87,51 @@ sealed class EnforcePermissionFix { 
        }



        /**

         * Given a permission check and an argument,

         * pull out the permission value that is being used

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

         */

        private fun getPermissionCheckArgumentValue(

            callExpression: UCallExpression,

            argumentPosition: Int = 0

        ): String {



            val identifier = when (

                val argument = callExpression.valueArguments.getOrNull(argumentPosition)

            ) {

                is UQualifiedReferenceExpression -> when (val selector = argument.selector) {

                    is USimpleNameReferenceExpression ->

                        ((selector.resolve() as PsiVariable).computeConstantValue() as String)



                    else -> throw RuntimeException(

                        "Couldn't resolve argument: ${selector.asRecursiveLogString()}"

                    )

                }

        private fun getPermissionCheckValues(

            callExpression: UCallExpression

        ): List<String> {

            if (!isPermissionMethodCall(callExpression)) return emptyList()



                is USimpleNameReferenceExpression -> (

                        (argument.resolve() as PsiVariable).computeConstantValue() as String)

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



                is ULiteralExpression -> argument.value as String

            // Breadth First Search - evalutaing nested @PermissionMethod(s) in the available

            // source code for @PermissionName(s).

            while (bfsQueue.isNotEmpty()) {

                val current = bfsQueue.removeFirst()

                visitedCalls.add(current)

                result.addAll(findPermissions(current))



                else -> throw RuntimeException(

                    "Couldn't resolve argument: ${argument?.asRecursiveLogString()}"

                )

                current.resolve()?.getUMethod()?.accept(object : AbstractUastVisitor() {

                    override fun visitCallExpression(node: UCallExpression): Boolean {

                        if (isPermissionMethodCall(node) && node !in visitedCalls) {

                            bfsQueue.add(node)

                        }



            return identifier.substringAfterLast(".")

                        return false

                    }

                })

            }



            return result.toList()

        }



data class SingleFix(val location: Location, val permissionName: String) : EnforcePermissionFix() {

    override fun locations(): List<Location> = listOf(this.location)

    override fun javaAnnotationParameter(): String = maybeAddManifestPrefix(this.permissionName)

        private fun findPermissions(

            callExpression: UCallExpression,

        ): List<String> {

            val indices = callExpression.resolve()?.getUMethod()

                ?.uastParameters

                ?.filter(::hasPermissionNameAnnotation)

                ?.mapNotNull { it.sourcePsi?.parameterIndex() }

                ?: emptyList()



            return indices.mapNotNull {

                callExpression.getArgumentForParameter(it)?.evaluateString()

            }

        }

    }

data class AllOfFix(val checks: List<SingleFix>) : EnforcePermissionFix() {

    override fun locations(): List<Location> = this.checks.map { it.location }

    override fun javaAnnotationParameter(): String =

        "allOf={${

            this.checks.joinToString(", ") { maybeAddManifestPrefix(it.permissionName) }

        }}"

}

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

0 → 100644
+67 −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.detector.api.getUMethod

import com.google.android.lint.ANNOTATION_PERMISSION_METHOD

import com.google.android.lint.ANNOTATION_PERMISSION_NAME

import com.google.android.lint.CLASS_STUB

import com.intellij.psi.PsiAnonymousClass

import org.jetbrains.uast.UCallExpression

import org.jetbrains.uast.UMethod

import org.jetbrains.uast.UParameter



/**

 * Given a UMethod, determine if this method is

 * an entrypoint to an interface generated by AIDL,

 * returning the interface name if so

 */

fun getContainingAidlInterface(node: UMethod): String? {

    if (!isInClassCalledStub(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

}



private fun isInClassCalledStub(node: UMethod): Boolean {

    (node.containingClass as? PsiAnonymousClass)?.let {

        return it.baseClassReference.referenceName == CLASS_STUB

    }

    return node.containingClass?.extendsList?.referenceElements?.any {

        it.referenceName == CLASS_STUB

    } ?: false

}



fun isPermissionMethodCall(callExpression: UCallExpression): Boolean {

    val method = callExpression.resolve()?.getUMethod() ?: return false

    return hasPermissionMethodAnnotation(method)

}



fun hasPermissionMethodAnnotation(method: UMethod): Boolean = method.annotations

    .any {

        it.hasQualifiedName(ANNOTATION_PERMISSION_METHOD)

    }



fun hasPermissionNameAnnotation(parameter: UParameter) = parameter.annotations.any {

    it.hasQualifiedName(ANNOTATION_PERMISSION_NAME)

}

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

+8 −49
Original line number Diff line number Diff line
@@ -25,9 +25,6 @@ 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.CLASS_STUB

import com.google.android.lint.ENFORCE_PERMISSION_METHODS

import com.intellij.psi.PsiAnonymousClass

import org.jetbrains.uast.UBlockExpression

import org.jetbrains.uast.UCallExpression

import org.jetbrains.uast.UElement
@@ -56,7 +53,7 @@ class ManualPermissionCheckDetector : Detector(), SourceCodeScanner { 
            val body = (node.uastBody as? UBlockExpression) ?: return

            val fix = accumulateSimplePermissionCheckFixes(body) ?: return



            val javaRemoveFixes = fix.locations().map {

            val javaRemoveFixes = fix.locations.map {

                fix()

                    .replace()

                    .reformat(true)
@@ -67,7 +64,7 @@ class ManualPermissionCheckDetector : Detector(), SourceCodeScanner { 
            }



            val javaAnnotateFix = fix()

                .annotate(fix.javaAnnotation())

                .annotate(fix.annotation)

                .range(context.getLocation(node))

                .autoFix()

                .build()
@@ -77,7 +74,7 @@ class ManualPermissionCheckDetector : Detector(), SourceCodeScanner { 


            context.report(

                ISSUE_USE_ENFORCE_PERMISSION_ANNOTATION,

                fix.locations().last(),

                fix.locations.last(),

                message,

                fix().composite(*javaRemoveFixes.toTypedArray(), javaAnnotateFix)

            )
@@ -97,14 +94,14 @@ class ManualPermissionCheckDetector : Detector(), SourceCodeScanner { 
         */

        private fun accumulateSimplePermissionCheckFixes(methodBody: UBlockExpression):

                EnforcePermissionFix? {

            val singleFixes = mutableListOf<SingleFix>()

            val singleFixes = mutableListOf<EnforcePermissionFix>()

            for (expression in methodBody.expressions) {

                singleFixes.add(getPermissionCheckFix(expression) ?: break)

            }

            return when (singleFixes.size) {

                0 -> null

                1 -> singleFixes[0]

                else -> AllOfFix(singleFixes)

                else -> EnforcePermissionFix.compose(singleFixes)

            }

        }
@@ -113,7 +110,7 @@ class ManualPermissionCheckDetector : Detector(), SourceCodeScanner { 
         * the helper for creating a lint auto fix to @EnforcePermission

         */

        private fun getPermissionCheckFix(startingExpression: UElement?):

                SingleFix? {

                EnforcePermissionFix? {

            return when (startingExpression) {

                is UQualifiedReferenceExpression -> getPermissionCheckFix(

                    startingExpression.selector
@@ -121,11 +118,8 @@ class ManualPermissionCheckDetector : Detector(), SourceCodeScanner { 


                is UIfExpression -> getPermissionCheckFix(startingExpression.condition)



                is UCallExpression -> {

                    return if (isPermissionCheck(startingExpression))

                        EnforcePermissionFix.fromCallExpression(startingExpression, context)

                    else null

                }

                is UCallExpression -> return EnforcePermissionFix

                            .fromCallExpression(context, startingExpression)



                else -> null

            }
@@ -160,40 +154,5 @@ class ManualPermissionCheckDetector : Detector(), SourceCodeScanner { 
            ),

            enabledByDefault = false, // TODO: enable once b/241171714 is resolved

        )



        private fun isPermissionCheck(callExpression: UCallExpression): Boolean {

            val method = callExpression.resolve() ?: return false

            val className = method.containingClass?.qualifiedName

            return ENFORCE_PERMISSION_METHODS.any {

                it.clazz == className && it.name == method.name

            }

        }



        /**

         * given a UMethod, determine if this method is

         * an entrypoint to an interface generated by AIDL,

         * returning the interface name if so

         */

        fun getContainingAidlInterface(node: UMethod): String? {

            if (!isInClassCalledStub(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

        }



        private fun isInClassCalledStub(node: UMethod): Boolean {

            (node.containingClass as? PsiAnonymousClass)?.let {

                return it.baseClassReference.referenceName == CLASS_STUB

            }

            return node.containingClass?.extendsList?.referenceElements?.any {

                it.referenceName == CLASS_STUB

            } ?: false

        }

    }

}

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

+254 −17

File changed.

Preview size limit exceeded, changes collapsed.