Merge "Add ManualPermissionCheckDetector"
This commit is contained in:
committed by
Android (Google) Code Review
commit
cc9f88ae77
@@ -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
|
||||
)
|
||||
|
||||
@@ -0,0 +1,36 @@
|
||||
/*
|
||||
* 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")
|
||||
)
|
||||
@@ -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
|
||||
@@ -362,14 +362,18 @@ class PackageVisibilityDetector : Detector(), SourceCodeScanner {
|
||||
name: String,
|
||||
matchArgument: Boolean = true,
|
||||
checkCaller: Boolean = false
|
||||
): this(clazz, name) {
|
||||
) : this(clazz, name) {
|
||||
this.matchArgument = matchArgument
|
||||
this.checkCaller = checkCaller
|
||||
}
|
||||
|
||||
constructor(
|
||||
method: PsiMethod
|
||||
): this(method.containingClass?.qualifiedName ?: "", method.name)
|
||||
) : this(method.containingClass?.qualifiedName ?: "", method.name)
|
||||
|
||||
constructor(
|
||||
method: com.google.android.lint.model.Method
|
||||
) : this(method.clazz, method.name)
|
||||
}
|
||||
|
||||
/**
|
||||
@@ -380,7 +384,7 @@ class PackageVisibilityDetector : Detector(), SourceCodeScanner {
|
||||
val typeName: String,
|
||||
val parameterName: String
|
||||
) {
|
||||
constructor(uParameter: UParameter): this(
|
||||
constructor(uParameter: UParameter) : this(
|
||||
uParameter.type.canonicalText,
|
||||
uParameter.name.lowercase()
|
||||
)
|
||||
@@ -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",
|
||||
|
||||
@@ -0,0 +1,73 @@
|
||||
/*
|
||||
* 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"
|
||||
)
|
||||
@@ -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
|
||||
@@ -0,0 +1,118 @@
|
||||
/*
|
||||
* 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.JavaContext
|
||||
import com.android.tools.lint.detector.api.Location
|
||||
import com.intellij.psi.PsiVariable
|
||||
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
|
||||
|
||||
/**
|
||||
* Helper ADT 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()})"
|
||||
|
||||
companion object {
|
||||
fun fromCallExpression(callExpression: UCallExpression, context: JavaContext): SingleFix =
|
||||
SingleFix(
|
||||
getPermissionCheckLocation(context, callExpression),
|
||||
getPermissionCheckArgumentValue(callExpression)
|
||||
)
|
||||
|
||||
fun maybeAddManifestPrefix(permissionName: String): String =
|
||||
if (permissionName.contains(".")) permissionName
|
||||
else "android.Manifest.permission.$permissionName"
|
||||
|
||||
/**
|
||||
* Given a permission check, get its proper location
|
||||
* so that a lint fix can remove the entire expression
|
||||
*/
|
||||
private fun getPermissionCheckLocation(
|
||||
context: JavaContext,
|
||||
callExpression: UCallExpression
|
||||
):
|
||||
Location {
|
||||
val javaPsi = callExpression.javaPsi!!
|
||||
return Location.create(
|
||||
context.file,
|
||||
javaPsi.containingFile?.text,
|
||||
javaPsi.textRange.startOffset,
|
||||
// unfortunately the element doesn't include the ending semicolon
|
||||
javaPsi.textRange.endOffset + 1
|
||||
)
|
||||
}
|
||||
|
||||
/**
|
||||
* Given a permission check and an argument,
|
||||
* pull out the permission value that is being used
|
||||
*/
|
||||
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()}"
|
||||
)
|
||||
}
|
||||
|
||||
is USimpleNameReferenceExpression -> (
|
||||
(argument.resolve() as PsiVariable).computeConstantValue() as String)
|
||||
|
||||
is ULiteralExpression -> argument.value as String
|
||||
|
||||
else -> throw RuntimeException(
|
||||
"Couldn't resolve argument: ${argument?.asRecursiveLogString()}"
|
||||
)
|
||||
}
|
||||
|
||||
return identifier.substringAfterLast(".")
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
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)
|
||||
}
|
||||
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) }
|
||||
}}"
|
||||
}
|
||||
@@ -0,0 +1,199 @@
|
||||
/*
|
||||
* 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.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
|
||||
import org.jetbrains.uast.UIfExpression
|
||||
import org.jetbrains.uast.UMethod
|
||||
import org.jetbrains.uast.UQualifiedReferenceExpression
|
||||
|
||||
/**
|
||||
* Looks for methods implementing generated AIDL interface stubs
|
||||
* that can have simple permission checks migrated to
|
||||
* @EnforcePermission annotations
|
||||
*
|
||||
* 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
|
||||
|
||||
val javaRemoveFixes = fix.locations().map {
|
||||
fix()
|
||||
.replace()
|
||||
.reformat(true)
|
||||
.range(it)
|
||||
.with("")
|
||||
.autoFix()
|
||||
.build()
|
||||
}
|
||||
|
||||
val javaAnnotateFix = fix()
|
||||
.annotate(fix.javaAnnotation())
|
||||
.range(context.getLocation(node))
|
||||
.autoFix()
|
||||
.build()
|
||||
|
||||
val message =
|
||||
"$interfaceName permission check can be converted to @EnforcePermission annotation"
|
||||
|
||||
context.report(
|
||||
ISSUE_USE_ENFORCE_PERMISSION_ANNOTATION,
|
||||
fix.locations().last(),
|
||||
message,
|
||||
fix().composite(*javaRemoveFixes.toTypedArray(), javaAnnotateFix)
|
||||
)
|
||||
}
|
||||
|
||||
/**
|
||||
* Walk the expressions in the method, looking for simple permission checks.
|
||||
*
|
||||
* If a single permission check is found at the beginning of the method,
|
||||
* this should be migrated to @EnforcePermission(value).
|
||||
*
|
||||
* If multiple consecutive permission checks are found,
|
||||
* these should be migrated to @EnforcePermission(allOf={value1, value2, ...})
|
||||
*
|
||||
* 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):
|
||||
EnforcePermissionFix? {
|
||||
val singleFixes = mutableListOf<SingleFix>()
|
||||
for (expression in methodBody.expressions) {
|
||||
singleFixes.add(getPermissionCheckFix(expression) ?: break)
|
||||
}
|
||||
return when (singleFixes.size) {
|
||||
0 -> null
|
||||
1 -> singleFixes[0]
|
||||
else -> AllOfFix(singleFixes)
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* 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?):
|
||||
SingleFix? {
|
||||
return when (startingExpression) {
|
||||
is UQualifiedReferenceExpression -> getPermissionCheckFix(
|
||||
startingExpression.selector
|
||||
)
|
||||
|
||||
is UIfExpression -> getPermissionCheckFix(startingExpression.condition)
|
||||
|
||||
is UCallExpression -> {
|
||||
return if (isPermissionCheck(startingExpression))
|
||||
EnforcePermissionFix.fromCallExpression(startingExpression, context)
|
||||
else null
|
||||
}
|
||||
|
||||
else -> null
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
companion object {
|
||||
|
||||
private val EXPLANATION = """
|
||||
Whenever possible, method implementations of AIDL interfaces should use the @EnforcePermission
|
||||
annotation to declare the permissions to be enforced. The verification code is then
|
||||
generated by the AIDL compiler, which also takes care of annotating the generated java
|
||||
code.
|
||||
|
||||
This reduces the risk of bugs around these permission checks (that often become vulnerabilities).
|
||||
It also enables easier auditing and review.
|
||||
|
||||
Please migrate to an @EnforcePermission annotation. (See: go/aidl-enforce-howto)
|
||||
""".trimIndent()
|
||||
|
||||
@JvmField
|
||||
val ISSUE_USE_ENFORCE_PERMISSION_ANNOTATION = Issue.create(
|
||||
id = "UseEnforcePermissionAnnotation",
|
||||
briefDescription = "Manual permission check can be @EnforcePermission annotation",
|
||||
explanation = EXPLANATION,
|
||||
category = Category.SECURITY,
|
||||
priority = 5,
|
||||
severity = Severity.WARNING,
|
||||
implementation = Implementation(
|
||||
ManualPermissionCheckDetector::class.java,
|
||||
Scope.JAVA_FILE_SCOPE
|
||||
),
|
||||
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
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -0,0 +1,26 @@
|
||||
/*
|
||||
* 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.model
|
||||
|
||||
/**
|
||||
* Data class to represent a Method
|
||||
*/
|
||||
data class Method(val clazz: String, val name: String) {
|
||||
override fun toString(): String {
|
||||
return "$clazz#$name"
|
||||
}
|
||||
}
|
||||
@@ -14,7 +14,7 @@
|
||||
* limitations under the License.
|
||||
*/
|
||||
|
||||
package com.google.android.lint
|
||||
package com.google.android.lint.aidl
|
||||
|
||||
import com.android.tools.lint.checks.infrastructure.LintDetectorTest
|
||||
import com.android.tools.lint.checks.infrastructure.TestFile
|
||||
@@ -0,0 +1,211 @@
|
||||
/*
|
||||
* 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.TestFile
|
||||
import com.android.tools.lint.checks.infrastructure.TestLintTask
|
||||
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()
|
||||
override fun getIssues(): List<Issue> = listOf(
|
||||
ManualPermissionCheckDetector
|
||||
.ISSUE_USE_ENFORCE_PERMISSION_ANNOTATION
|
||||
)
|
||||
|
||||
override fun lint(): TestLintTask = super.lint().allowMissingSdk()
|
||||
|
||||
fun testClass() {
|
||||
lint().files(
|
||||
java(
|
||||
"""
|
||||
import android.content.Context;
|
||||
import android.test.ITest;
|
||||
public class Foo extends ITest.Stub {
|
||||
private Context mContext;
|
||||
@Override
|
||||
public void test() throws android.os.RemoteException {
|
||||
mContext.enforceCallingOrSelfPermission("android.Manifest.permission.READ_CONTACTS", "foo");
|
||||
}
|
||||
}
|
||||
"""
|
||||
).indented(),
|
||||
*stubs
|
||||
)
|
||||
.run()
|
||||
.expect(
|
||||
"""
|
||||
src/Foo.java:7: Warning: ITest permission check can be converted to @EnforcePermission annotation [UseEnforcePermissionAnnotation]
|
||||
mContext.enforceCallingOrSelfPermission("android.Manifest.permission.READ_CONTACTS", "foo");
|
||||
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
|
||||
0 errors, 1 warnings
|
||||
"""
|
||||
)
|
||||
.expectFixDiffs(
|
||||
"""
|
||||
Fix for src/Foo.java line 7: Annotate with @EnforcePermission:
|
||||
@@ -5 +5
|
||||
+ @android.annotation.EnforcePermission(android.Manifest.permission.READ_CONTACTS)
|
||||
@@ -7 +8
|
||||
- mContext.enforceCallingOrSelfPermission("android.Manifest.permission.READ_CONTACTS", "foo");
|
||||
"""
|
||||
)
|
||||
}
|
||||
|
||||
fun testAnonClass() {
|
||||
lint().files(
|
||||
java(
|
||||
"""
|
||||
import android.content.Context;
|
||||
import android.test.ITest;
|
||||
public class Foo {
|
||||
private Context mContext;
|
||||
private ITest itest = new ITest.Stub() {
|
||||
@Override
|
||||
public void test() throws android.os.RemoteException {
|
||||
mContext.enforceCallingOrSelfPermission(
|
||||
"android.Manifest.permission.READ_CONTACTS", "foo");
|
||||
}
|
||||
};
|
||||
}
|
||||
"""
|
||||
).indented(),
|
||||
*stubs
|
||||
)
|
||||
.run()
|
||||
.expect(
|
||||
"""
|
||||
src/Foo.java:8: Warning: ITest permission check can be converted to @EnforcePermission annotation [UseEnforcePermissionAnnotation]
|
||||
mContext.enforceCallingOrSelfPermission(
|
||||
^
|
||||
0 errors, 1 warnings
|
||||
"""
|
||||
)
|
||||
.expectFixDiffs(
|
||||
"""
|
||||
Fix for src/Foo.java line 8: Annotate with @EnforcePermission:
|
||||
@@ -6 +6
|
||||
+ @android.annotation.EnforcePermission(android.Manifest.permission.READ_CONTACTS)
|
||||
@@ -8 +9
|
||||
- mContext.enforceCallingOrSelfPermission(
|
||||
- "android.Manifest.permission.READ_CONTACTS", "foo");
|
||||
"""
|
||||
)
|
||||
}
|
||||
|
||||
fun testAllOf() {
|
||||
lint().files(
|
||||
java(
|
||||
"""
|
||||
import android.content.Context;
|
||||
import android.test.ITest;
|
||||
public class Foo {
|
||||
private Context mContext;
|
||||
private ITest itest = new ITest.Stub() {
|
||||
@Override
|
||||
public void test() throws android.os.RemoteException {
|
||||
mContext.enforceCallingOrSelfPermission(
|
||||
"android.Manifest.permission.READ_CONTACTS", "foo");
|
||||
mContext.enforceCallingOrSelfPermission(
|
||||
"android.Manifest.permission.WRITE_CONTACTS", "foo");
|
||||
}
|
||||
};
|
||||
}
|
||||
"""
|
||||
).indented(),
|
||||
*stubs
|
||||
)
|
||||
.run()
|
||||
.expect(
|
||||
"""
|
||||
src/Foo.java:10: Warning: ITest permission check can be converted to @EnforcePermission annotation [UseEnforcePermissionAnnotation]
|
||||
mContext.enforceCallingOrSelfPermission(
|
||||
^
|
||||
0 errors, 1 warnings
|
||||
"""
|
||||
)
|
||||
.expectFixDiffs(
|
||||
"""
|
||||
Fix for src/Foo.java line 10: Annotate with @EnforcePermission:
|
||||
@@ -6 +6
|
||||
+ @android.annotation.EnforcePermission(allOf={android.Manifest.permission.READ_CONTACTS, android.Manifest.permission.WRITE_CONTACTS})
|
||||
@@ -8 +9
|
||||
- mContext.enforceCallingOrSelfPermission(
|
||||
- "android.Manifest.permission.READ_CONTACTS", "foo");
|
||||
- mContext.enforceCallingOrSelfPermission(
|
||||
- "android.Manifest.permission.WRITE_CONTACTS", "foo");
|
||||
"""
|
||||
)
|
||||
}
|
||||
|
||||
fun testPrecedingExpressions() {
|
||||
lint().files(
|
||||
java(
|
||||
"""
|
||||
import android.os.Binder;
|
||||
import android.test.ITest;
|
||||
public class Foo extends ITest.Stub {
|
||||
private mContext Context;
|
||||
@Override
|
||||
public void test() throws android.os.RemoteException {
|
||||
long uid = Binder.getCallingUid();
|
||||
mContext.enforceCallingOrSelfPermission("android.Manifest.permission.READ_CONTACTS", "foo");
|
||||
}
|
||||
}
|
||||
"""
|
||||
).indented(),
|
||||
*stubs
|
||||
)
|
||||
.run()
|
||||
.expectClean()
|
||||
}
|
||||
|
||||
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 {
|
||||
public void enforceCallingOrSelfPermission(String permission, String message) {}
|
||||
}
|
||||
"""
|
||||
).indented()
|
||||
|
||||
private val binderStub: TestFile = java(
|
||||
"""
|
||||
package android.os;
|
||||
public class Binder {
|
||||
public static int getCallingUid() {}
|
||||
}
|
||||
"""
|
||||
).indented()
|
||||
|
||||
val stubs = arrayOf(aidlStub, contextStub, binderStub)
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user