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

Commit 598be551 authored by Azhara Assanova's avatar Azhara Assanova Committed by Android (Google) Code Review
Browse files

Merge "Create parent class for permission enforcement annotation detectors"

parents 0c0b929d 635b108c
Loading
Loading
Loading
Loading
+2 −2
Original line number Diff line number Diff line
@@ -21,7 +21,7 @@ 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.aidl.SimpleManualPermissionEnforcementDetector
import com.google.android.lint.parcel.SaferParcelChecker
import com.google.auto.service.AutoService

@@ -40,7 +40,7 @@ class AndroidFrameworkIssueRegistry : IssueRegistry() {
        EnforcePermissionDetector.ISSUE_MISSING_ENFORCE_PERMISSION,
        EnforcePermissionDetector.ISSUE_MISMATCHING_ENFORCE_PERMISSION,
        EnforcePermissionHelperDetector.ISSUE_ENFORCE_PERMISSION_HELPER,
        ManualPermissionCheckDetector.ISSUE_USE_ENFORCE_PERMISSION_ANNOTATION,
        SimpleManualPermissionEnforcementDetector.ISSUE_USE_ENFORCE_PERMISSION_ANNOTATION,
        SaferParcelChecker.ISSUE_UNSAFE_API_USAGE,
        PackageVisibilityDetector.ISSUE_PACKAGE_NAME_NO_PACKAGE_VISIBILITY_FILTERS,
        RegisterReceiverFlagDetector.ISSUE_RECEIVER_EXPORTED_FLAG,
+52 −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.Detector
import com.android.tools.lint.detector.api.JavaContext
import com.android.tools.lint.detector.api.SourceCodeScanner
import org.jetbrains.uast.UBlockExpression
import org.jetbrains.uast.UElement
import org.jetbrains.uast.UMethod

/**
 * Abstract class for detectors that look for methods implementing
 * generated AIDL interface stubs
 */
abstract class AidlImplementationDetector : 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) {
            val interfaceName = getContainingAidlInterface(node)
                    .takeUnless(EXCLUDED_CPP_INTERFACES::contains) ?: return
            val body = (node.uastBody as? UBlockExpression) ?: return
            visitAidlMethod(context, node, interfaceName, body)
        }
    }

    abstract fun visitAidlMethod(
            context: JavaContext,
            node: UMethod,
            interfaceName: String,
            body: UBlockExpression,
    )
}
+150 −0
Original line number Diff line number Diff line
@@ -16,15 +16,12 @@

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 org.jetbrains.uast.UBlockExpression
import org.jetbrains.uast.UCallExpression
import org.jetbrains.uast.UElement
@@ -40,18 +37,14 @@ import org.jetbrains.uast.UQualifiedReferenceExpression
 * TODO: b/242564870 (enable parse and autoFix of .aidl files)
 */
@Suppress("UnstableApiUsage")
class ManualPermissionCheckDetector : 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) {
            val interfaceName = getContainingAidlInterface(node)
                .takeUnless(EXCLUDED_CPP_INTERFACES::contains) ?: return
            val body = (node.uastBody as? UBlockExpression) ?: return
            val fix = accumulateSimplePermissionCheckFixes(body) ?: return
class SimpleManualPermissionEnforcementDetector : AidlImplementationDetector() {
    override fun visitAidlMethod(
            context: JavaContext,
            node: UMethod,
            interfaceName: String,
            body: UBlockExpression
    ) {
        val fix = accumulateSimplePermissionCheckFixes(body, context) ?: return

        val javaRemoveFixes = fix.locations.map {
            fix()
@@ -69,13 +62,10 @@ class ManualPermissionCheckDetector : Detector(), SourceCodeScanner {
                .autoFix()
                .build()

            val message =
                "$interfaceName permission check can be converted to @EnforcePermission annotation"

        context.report(
                ISSUE_USE_ENFORCE_PERMISSION_ANNOTATION,
                fix.locations.last(),
                message,
                "$interfaceName permission check can be converted to @EnforcePermission annotation",
                fix().composite(*javaRemoveFixes.toTypedArray(), javaAnnotateFix)
        )
    }
@@ -92,11 +82,14 @@ class ManualPermissionCheckDetector : Detector(), SourceCodeScanner {
     * As soon as something other than a permission check is encountered, stop looking,
     * as some other business logic is happening that prevents an automated fix.
     */
        private fun accumulateSimplePermissionCheckFixes(methodBody: UBlockExpression):
    private fun accumulateSimplePermissionCheckFixes(
            methodBody: UBlockExpression,
            context: JavaContext
    ):
            EnforcePermissionFix? {
        val singleFixes = mutableListOf<EnforcePermissionFix>()
        for (expression in methodBody.expressions) {
                singleFixes.add(getPermissionCheckFix(expression) ?: break)
            singleFixes.add(getPermissionCheckFix(expression, context) ?: break)
        }
        return when (singleFixes.size) {
            0 -> null
@@ -109,14 +102,14 @@ class ManualPermissionCheckDetector : Detector(), SourceCodeScanner {
     * If an expression boils down to a permission check, return
     * the helper for creating a lint auto fix to @EnforcePermission
     */
        private fun getPermissionCheckFix(startingExpression: UElement?):
    private fun getPermissionCheckFix(startingExpression: UElement?, context: JavaContext):
            EnforcePermissionFix? {
        return when (startingExpression) {
            is UQualifiedReferenceExpression -> getPermissionCheckFix(
                    startingExpression.selector
                    startingExpression.selector, context
            )

                is UIfExpression -> getPermissionCheckFix(startingExpression.condition)
            is UIfExpression -> getPermissionCheckFix(startingExpression.condition, context)

            is UCallExpression -> return EnforcePermissionFix
                    .fromCallExpression(context, startingExpression)
@@ -124,7 +117,6 @@ class ManualPermissionCheckDetector : Detector(), SourceCodeScanner {
            else -> null
        }
    }
    }

    companion object {

@@ -142,14 +134,14 @@ class ManualPermissionCheckDetector : Detector(), SourceCodeScanner {

        @JvmField
        val ISSUE_USE_ENFORCE_PERMISSION_ANNOTATION = Issue.create(
            id = "UseEnforcePermissionAnnotation",
                id = "SimpleManualPermissionEnforcement",
                briefDescription = "Manual permission check can be @EnforcePermission annotation",
                explanation = EXPLANATION,
                category = Category.SECURITY,
                priority = 5,
                severity = Severity.WARNING,
                implementation = Implementation(
                ManualPermissionCheckDetector::class.java,
                        SimpleManualPermissionEnforcementDetector::class.java,
                        Scope.JAVA_FILE_SCOPE
                ),
                enabledByDefault = false, // TODO: enable once b/241171714 is resolved
+10 −10
Original line number Diff line number Diff line
@@ -23,10 +23,10 @@ import com.android.tools.lint.detector.api.Detector
import com.android.tools.lint.detector.api.Issue

@Suppress("UnstableApiUsage")
class ManualPermissionCheckDetectorTest : LintDetectorTest() {
    override fun getDetector(): Detector = ManualPermissionCheckDetector()
class SimpleManualPermissionEnforcementDetectorTest : LintDetectorTest() {
    override fun getDetector(): Detector = SimpleManualPermissionEnforcementDetector()
    override fun getIssues(): List<Issue> = listOf(
        ManualPermissionCheckDetector
            SimpleManualPermissionEnforcementDetector
            .ISSUE_USE_ENFORCE_PERMISSION_ANNOTATION
    )

@@ -52,7 +52,7 @@ class ManualPermissionCheckDetectorTest : LintDetectorTest() {
            .run()
            .expect(
                """
                src/Foo.java:7: Warning: ITest permission check can be converted to @EnforcePermission annotation [UseEnforcePermissionAnnotation]
                src/Foo.java:7: Warning: ITest permission check can be converted to @EnforcePermission annotation [SimpleManualPermissionEnforcement]
                        mContext.enforceCallingOrSelfPermission("android.permission.READ_CONTACTS", "foo");
                        ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
                0 errors, 1 warnings
@@ -92,7 +92,7 @@ class ManualPermissionCheckDetectorTest : LintDetectorTest() {
            .run()
            .expect(
                """
                src/Foo.java:8: Warning: ITest permission check can be converted to @EnforcePermission annotation [UseEnforcePermissionAnnotation]
                src/Foo.java:8: Warning: ITest permission check can be converted to @EnforcePermission annotation [SimpleManualPermissionEnforcement]
                            mContext.enforceCallingOrSelfPermission(
                            ^
                0 errors, 1 warnings
@@ -132,7 +132,7 @@ class ManualPermissionCheckDetectorTest : LintDetectorTest() {
            .run()
            .expect(
                """
                src/Foo.java:8: Warning: ITest permission check can be converted to @EnforcePermission annotation [UseEnforcePermissionAnnotation]
                src/Foo.java:8: Warning: ITest permission check can be converted to @EnforcePermission annotation [SimpleManualPermissionEnforcement]
                        mContext.enforceCallingOrSelfPermission(android.Manifest.permission.READ_CONTACTS, "foo");
                        ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
                0 errors, 1 warnings
@@ -174,7 +174,7 @@ class ManualPermissionCheckDetectorTest : LintDetectorTest() {
            .run()
            .expect(
                """
                src/Foo.java:10: Warning: ITest permission check can be converted to @EnforcePermission annotation [UseEnforcePermissionAnnotation]
                src/Foo.java:10: Warning: ITest permission check can be converted to @EnforcePermission annotation [SimpleManualPermissionEnforcement]
                            mContext.enforceCallingOrSelfPermission(
                            ^
                0 errors, 1 warnings
@@ -243,7 +243,7 @@ class ManualPermissionCheckDetectorTest : LintDetectorTest() {
            .run()
            .expect(
                """
                src/Foo.java:14: Warning: ITest permission check can be converted to @EnforcePermission annotation [UseEnforcePermissionAnnotation]
                src/Foo.java:14: Warning: ITest permission check can be converted to @EnforcePermission annotation [SimpleManualPermissionEnforcement]
                        helper();
                        ~~~~~~~~~
                0 errors, 1 warnings
@@ -289,7 +289,7 @@ class ManualPermissionCheckDetectorTest : LintDetectorTest() {
            .run()
            .expect(
                """
                src/Foo.java:16: Warning: ITest permission check can be converted to @EnforcePermission annotation [UseEnforcePermissionAnnotation]
                src/Foo.java:16: Warning: ITest permission check can be converted to @EnforcePermission annotation [SimpleManualPermissionEnforcement]
                        mContext.enforceCallingOrSelfPermission("FOO", "foo");
                        ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
                0 errors, 1 warnings
@@ -340,7 +340,7 @@ class ManualPermissionCheckDetectorTest : LintDetectorTest() {
            .run()
            .expect(
                """
                src/Foo.java:19: Warning: ITest permission check can be converted to @EnforcePermission annotation [UseEnforcePermissionAnnotation]
                src/Foo.java:19: Warning: ITest permission check can be converted to @EnforcePermission annotation [SimpleManualPermissionEnforcement]
                        helperHelper();
                        ~~~~~~~~~~~~~~~
                0 errors, 1 warnings