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

Commit a4664b66 authored by mattgilbride's avatar mattgilbride
Browse files

Add ManualPermissionCheckDetector

This linter looks at methods that implement an AIDL interface.
If a given method contains a simple permission check, it suggests
moving that check to an @EnforcePermission annotation. The intent
is to keep as many permission checks as possible at a lower-level
to the service implementation, thus mitigating permission bypass
vulnerabilities.

Also rearranges some helpers/constants for reuse, and moves everything related to aidl to its own package.

Test: atest ManualPermissionCheckDetectorTest --host
Bug: 232058525
Change-Id: Ie6eaf061d74bd773742aa47f731e95e4b137f438
parent 2a7eb382
Loading
Loading
Loading
Loading
+3 −0
Original line number Diff line number Diff line
@@ -19,6 +19,8 @@ package com.google.android.lint
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.ManualPermissionCheckDetector
import com.google.android.lint.parcel.SaferParcelChecker
import com.google.auto.service.AutoService

@@ -36,6 +38,7 @@ class AndroidFrameworkIssueRegistry : IssueRegistry() {
        CallingSettingsNonUserGetterMethodsDetector.ISSUE_NON_USER_GETTER_CALLED,
        EnforcePermissionDetector.ISSUE_MISSING_ENFORCE_PERMISSION,
        EnforcePermissionDetector.ISSUE_MISMATCHING_ENFORCE_PERMISSION,
        ManualPermissionCheckDetector.ISSUE_USE_ENFORCE_PERMISSION_ANNOTATION,
        SaferParcelChecker.ISSUE_UNSAFE_API_USAGE,
        PackageVisibilityDetector.ISSUE_PACKAGE_NAME_NO_PACKAGE_VISIBILITY_FILTERS
    )
+36 −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

import com.google.android.lint.model.Method

const val CLASS_STUB = "Stub"
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"

// Enforce permission APIs
val ENFORCE_PERMISSION_METHODS = listOf(
        Method(CLASS_CONTEXT, "checkPermission"),
        Method(CLASS_CONTEXT, "checkCallingPermission"),
        Method(CLASS_CONTEXT, "checkCallingOrSelfPermission"),
        Method(CLASS_CONTEXT, "enforcePermission"),
        Method(CLASS_CONTEXT, "enforceCallingPermission"),
        Method(CLASS_CONTEXT, "enforceCallingOrSelfPermission"),
        Method(CLASS_ACTIVITY_MANAGER_SERVICE, "checkPermission"),
        Method(CLASS_ACTIVITY_MANAGER_INTERNAL, "enforceCallingPermission")
)
+11 −20
Original line number Diff line number Diff line
@@ -30,13 +30,13 @@ import com.android.tools.lint.detector.api.interprocedural.CallGraphResult
import com.android.tools.lint.detector.api.interprocedural.searchForPaths
import com.intellij.psi.PsiAnonymousClass
import com.intellij.psi.PsiMethod
import java.util.LinkedList
import org.jetbrains.uast.UCallExpression
import org.jetbrains.uast.UElement
import org.jetbrains.uast.UMethod
import org.jetbrains.uast.UParameter
import org.jetbrains.uast.USimpleNameReferenceExpression
import org.jetbrains.uast.visitor.AbstractUastVisitor
import java.util.LinkedList

/**
 * A lint checker to detect potential package visibility issues for system's APIs. APIs working
@@ -370,6 +370,10 @@ class PackageVisibilityDetector : Detector(), SourceCodeScanner {
        constructor(
            method: PsiMethod
        ) : this(method.containingClass?.qualifiedName ?: "", method.name)

        constructor(
            method: com.google.android.lint.model.Method
        ) : this(method.clazz, method.name)
    }

    /**
@@ -405,19 +409,13 @@ class PackageVisibilityDetector : Detector(), SourceCodeScanner {
        // A valid call path list needs to contain a start node and a sink node
        private const val VALID_CALL_PATH_NODES_SIZE = 2

        private const val CLASS_STUB = "Stub"
        private const val CLASS_STRING = "java.lang.String"
        private const val CLASS_PACKAGE_MANAGER = "android.content.pm.PackageManager"
        private const val CLASS_IPACKAGE_MANAGER = "android.content.pm.IPackageManager"
        private const val CLASS_APPOPS_MANAGER = "android.app.AppOpsManager"
        private const val CLASS_CONTEXT = "android.content.Context"
        private const val CLASS_BINDER = "android.os.Binder"
        private const val CLASS_PACKAGE_MANAGER_INTERNAL =
            "android.content.pm.PackageManagerInternal"
        private const val CLASS_ACTIVITY_MANAGER_SERVICE =
            "com.android.server.am.ActivityManagerService"
        private const val CLASS_ACTIVITY_MANAGER_INTERNAL =
            "android.app.ActivityManagerInternal"

        // Patterns of package name parameter
        private val PACKAGE_NAME_PATTERNS = setOf(
@@ -455,16 +453,9 @@ class PackageVisibilityDetector : Detector(), SourceCodeScanner {
        )

        // Enforce permission APIs
        private val ENFORCE_PERMISSION_METHODS = listOf(
            Method(CLASS_CONTEXT, "checkPermission"),
            Method(CLASS_CONTEXT, "checkCallingPermission"),
            Method(CLASS_CONTEXT, "checkCallingOrSelfPermission"),
            Method(CLASS_CONTEXT, "enforcePermission"),
            Method(CLASS_CONTEXT, "enforceCallingPermission"),
            Method(CLASS_CONTEXT, "enforceCallingOrSelfPermission"),
            Method(CLASS_ACTIVITY_MANAGER_SERVICE, "checkPermission"),
            Method(CLASS_ACTIVITY_MANAGER_INTERNAL, "enforceCallingPermission")
        )
        private val ENFORCE_PERMISSION_METHODS =
                com.google.android.lint.ENFORCE_PERMISSION_METHODS
                        .map(PackageVisibilityDetector::Method)

        private val BYPASS_STUBS = listOf(
            "android.content.pm.IPackageDataObserver.Stub",
+73 −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

const val ANNOTATION_ENFORCE_PERMISSION = "android.annotation.EnforcePermission"
const val ANNOTATION_REQUIRES_NO_PERMISSION = "android.annotation.RequiresNoPermission"
const val ANNOTATION_PERMISSION_MANUALLY_ENFORCED = "android.annotation.PermissionManuallyEnforced"

val AIDL_PERMISSION_ANNOTATIONS = listOf(
        ANNOTATION_ENFORCE_PERMISSION,
        ANNOTATION_REQUIRES_NO_PERMISSION,
        ANNOTATION_PERMISSION_MANUALLY_ENFORCED
)

const val IINTERFACE_INTERFACE = "android.os.IInterface"

/**
 * If a non java (e.g. c++) backend is enabled, the @EnforcePermission
 * annotation cannot be used.  At time of writing, the mechanism
 * is not implemented for non java backends.
 * TODO: b/242564874 (have lint know which interfaces have the c++ backend enabled)
 * rather than hard coding this list?
 */
val EXCLUDED_CPP_INTERFACES = listOf(
        "AdbTransportType",
        "FingerprintAndPairDevice",
        "IAdbCallback",
        "IAdbManager",
        "PairDevice",
        "IStatsBootstrapAtomService",
        "StatsBootstrapAtom",
        "StatsBootstrapAtomValue",
        "FixedSizeArrayExample",
        "PlaybackTrackMetadata",
        "RecordTrackMetadata",
        "SinkMetadata",
        "SourceMetadata",
        "IUpdateEngineStable",
        "IUpdateEngineStableCallback",
        "AudioCapabilities",
        "ConfidenceLevel",
        "ModelParameter",
        "ModelParameterRange",
        "Phrase",
        "PhraseRecognitionEvent",
        "PhraseRecognitionExtra",
        "PhraseSoundModel",
        "Properties",
        "RecognitionConfig",
        "RecognitionEvent",
        "RecognitionMode",
        "RecognitionStatus",
        "SoundModel",
        "SoundModelType",
        "Status",
        "IThermalService",
        "IPowerManager",
        "ITunerResourceManager"
)
+9 −10
Original line number Diff line number Diff line
@@ -14,15 +14,15 @@
 * limitations under the License.
 */

package com.google.android.lint
package com.google.android.lint.aidl

import com.android.tools.lint.client.api.UElementHandler
import com.android.tools.lint.detector.api.AnnotationInfo
import com.android.tools.lint.detector.api.AnnotationOrigin
import com.android.tools.lint.detector.api.AnnotationUsageInfo
import com.android.tools.lint.detector.api.AnnotationUsageType
import com.android.tools.lint.detector.api.ConstantEvaluator
import com.android.tools.lint.detector.api.Category
import com.android.tools.lint.detector.api.ConstantEvaluator
import com.android.tools.lint.detector.api.Detector
import com.android.tools.lint.detector.api.Implementation
import com.android.tools.lint.detector.api.Issue
@@ -34,8 +34,8 @@ import com.intellij.psi.PsiAnnotation
import com.intellij.psi.PsiClass
import com.intellij.psi.PsiMethod
import org.jetbrains.uast.UAnnotation
import org.jetbrains.uast.UElement
import org.jetbrains.uast.UClass
import org.jetbrains.uast.UElement
import org.jetbrains.uast.UMethod

/**
@@ -54,12 +54,11 @@ import org.jetbrains.uast.UMethod
 */
class EnforcePermissionDetector : Detector(), SourceCodeScanner {

    val ENFORCE_PERMISSION = "android.annotation.EnforcePermission"
    val BINDER_CLASS = "android.os.Binder"
    val JAVA_OBJECT = "java.lang.Object"

    override fun applicableAnnotations(): List<String> {
        return listOf(ENFORCE_PERMISSION)
        return listOf(ANNOTATION_ENFORCE_PERMISSION)
    }

    override fun getApplicableUastTypes(): List<Class<out UElement>> {
@@ -99,8 +98,8 @@ class EnforcePermissionDetector : Detector(), SourceCodeScanner {
        overriddenMethod: PsiMethod,
        checkEquivalence: Boolean = true
    ) {
        val overridingAnnotation = overridingMethod.getAnnotation(ENFORCE_PERMISSION)
        val overriddenAnnotation = overriddenMethod.getAnnotation(ENFORCE_PERMISSION)
        val overridingAnnotation = overridingMethod.getAnnotation(ANNOTATION_ENFORCE_PERMISSION)
        val overriddenAnnotation = overriddenMethod.getAnnotation(ANNOTATION_ENFORCE_PERMISSION)
        val location = context.getLocation(element)
        val overridingClass = overridingMethod.parent as PsiClass
        val overriddenClass = overriddenMethod.parent as PsiClass
@@ -133,8 +132,8 @@ class EnforcePermissionDetector : Detector(), SourceCodeScanner {
        extendedClass: PsiClass,
        checkEquivalence: Boolean = true
    ) {
        val newAnnotation = newClass.getAnnotation(ENFORCE_PERMISSION)
        val extendedAnnotation = extendedClass.getAnnotation(ENFORCE_PERMISSION)
        val newAnnotation = newClass.getAnnotation(ANNOTATION_ENFORCE_PERMISSION)
        val extendedAnnotation = extendedClass.getAnnotation(ANNOTATION_ENFORCE_PERMISSION)

        val location = context.getLocation(element)
        val newClassName = newClass.qualifiedName
@@ -180,7 +179,7 @@ class EnforcePermissionDetector : Detector(), SourceCodeScanner {
    override fun createUastHandler(context: JavaContext): UElementHandler {
        return object : UElementHandler() {
            override fun visitAnnotation(node: UAnnotation) {
                if (node.qualifiedName != ENFORCE_PERMISSION) {
                if (node.qualifiedName != ANNOTATION_ENFORCE_PERMISSION) {
                    return
                }
                val method = node.uastParent as? UMethod
Loading