diff options
Diffstat (limited to 'lint/libs/lint-checks/src/main/java/com/android/tools/lint/checks/UnsafeIntentLaunchDetector.kt')
| -rw-r--r-- | lint/libs/lint-checks/src/main/java/com/android/tools/lint/checks/UnsafeIntentLaunchDetector.kt | 275 |
1 files changed, 220 insertions, 55 deletions
diff --git a/lint/libs/lint-checks/src/main/java/com/android/tools/lint/checks/UnsafeIntentLaunchDetector.kt b/lint/libs/lint-checks/src/main/java/com/android/tools/lint/checks/UnsafeIntentLaunchDetector.kt index e43dce6aae..5bd51d4405 100644 --- a/lint/libs/lint-checks/src/main/java/com/android/tools/lint/checks/UnsafeIntentLaunchDetector.kt +++ b/lint/libs/lint-checks/src/main/java/com/android/tools/lint/checks/UnsafeIntentLaunchDetector.kt @@ -20,6 +20,9 @@ import com.android.SdkConstants.ANDROID_URI import com.android.SdkConstants.ATTR_EXPORTED import com.android.SdkConstants.ATTR_NAME import com.android.SdkConstants.ATTR_PERMISSION +import com.android.SdkConstants.TAG_ACTIVITY +import com.android.SdkConstants.TAG_RECEIVER +import com.android.SdkConstants.TAG_SERVICE import com.android.tools.lint.checks.BroadcastReceiverUtils.BROADCAST_RECEIVER_METHOD_NAMES import com.android.tools.lint.client.api.JavaEvaluator import com.android.tools.lint.client.api.TYPE_INT @@ -38,25 +41,37 @@ 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.UastLintUtils +import com.android.tools.lint.detector.api.UastLintUtils.Companion.findConstruction import com.android.tools.lint.detector.api.UastLintUtils.Companion.findLastAssignment import com.android.tools.lint.detector.api.XmlContext import com.android.tools.lint.detector.api.XmlScanner import com.android.tools.lint.detector.api.findSelector +import com.android.tools.lint.detector.api.isReturningContext import com.android.tools.lint.detector.api.isReturningLambdaResult import com.android.tools.lint.detector.api.isScopingThis +import com.android.utils.iterator +import com.android.utils.subtag import com.intellij.psi.PsiMethod +import com.intellij.psi.PsiSwitchLabelStatementBase import com.intellij.psi.PsiVariable import java.util.EnumSet +import org.jetbrains.uast.UBinaryExpression import org.jetbrains.uast.UCallExpression import org.jetbrains.uast.UClass import org.jetbrains.uast.UElement import org.jetbrains.uast.UExpression +import org.jetbrains.uast.UIfExpression import org.jetbrains.uast.ULambdaExpression import org.jetbrains.uast.UMethod import org.jetbrains.uast.UParameter +import org.jetbrains.uast.UQualifiedReferenceExpression +import org.jetbrains.uast.UReferenceExpression import org.jetbrains.uast.UReturnExpression import org.jetbrains.uast.USimpleNameReferenceExpression +import org.jetbrains.uast.USwitchClauseExpression +import org.jetbrains.uast.USwitchExpression import org.jetbrains.uast.UThisExpression +import org.jetbrains.uast.UastBinaryOperator import org.jetbrains.uast.getParentOfType import org.jetbrains.uast.getQualifiedName import org.jetbrains.uast.isNullLiteral @@ -88,24 +103,49 @@ class UnsafeIntentLaunchDetector : Detector(), SourceCodeScanner, XmlScanner { override fun getApplicableElements() = listOf( - SdkConstants.TAG_ACTIVITY, - SdkConstants.TAG_SERVICE, - SdkConstants.TAG_RECEIVER, + TAG_ACTIVITY, + TAG_SERVICE, + TAG_RECEIVER, ) override fun visitElement(context: XmlContext, element: Element) { - val exportedAttr = element.getAttributeNS(ANDROID_URI, ATTR_EXPORTED) + storeUnprotectedComponents(context, getProtectedComponent(context, element) ?: return) + } + + private fun isComponentExported( + context: Context, + root: Element, + incidentComponent: String? + ): Boolean { + val application = root.subtag(SdkConstants.TAG_APPLICATION) ?: return false + for (component in application) { + when (component.tagName) { + TAG_ACTIVITY, + TAG_RECEIVER, + TAG_SERVICE -> { + if (incidentComponent == getProtectedComponent(context, component)) return true + } + } + } + return false + } + + // Returns the fully qualified component name if the component is protected; otherwise, null + // The element passed in is guaranteed to be one of the activity, receiver or service tag. + private fun getProtectedComponent(context: Context, component: Element): String? { + val exportedAttr = component.getAttributeNS(ANDROID_URI, ATTR_EXPORTED) if ( "true" == exportedAttr || - exportedAttr.isEmpty() && element.getElementsByTagName("intent-filter").length > 0 + exportedAttr.isEmpty() && component.getElementsByTagName("intent-filter").length > 0 ) { - val permission = element.getAttributeNS(ANDROID_URI, ATTR_PERMISSION) + val permission = component.getAttributeNS(ANDROID_URI, ATTR_PERMISSION) if (!isProbablyProtectedBySignaturePermission(permission)) { - var componentName = element.getAttributeNS(ANDROID_URI, ATTR_NAME) + var componentName = component.getAttributeNS(ANDROID_URI, ATTR_NAME) if (componentName.startsWith(".")) componentName = context.project.`package` + componentName - storeUnprotectedComponents(context, componentName) + return componentName } } + return null } // Any permission that is not declared by the system as normal permission is considered as a @@ -144,10 +184,13 @@ class UnsafeIntentLaunchDetector : Detector(), SourceCodeScanner, XmlScanner { initial = setOf(intentParam ?: return), context = context, location = context.getLocation(intentParam.sourcePsi), + checkProtectedBroadcast = + UNSAFE_INTENT_AS_PARAMETER_METHODS[BROADCAST_RECEIVER_CLASS]?.contains(methodName) == + true ) method.accept(visitor) if (visitor.launched) { - storeIncidentsToPartialResults(context, visitor) + reportIncident(context, visitor) } } } @@ -175,18 +218,11 @@ class UnsafeIntentLaunchDetector : Detector(), SourceCodeScanner, XmlScanner { containingMethod?.accept(visitor) if (visitor.launched) { if (visitor.unprotectedReceiver) { - // The registered-at-runtime receiver case, report the issue immediately. - val message = - """ - This intent could be coming from an untrusted source. It is later launched by \ - an unprotected component. You could either make the component \ - protected; or sanitize this intent using \ - androidx.core.content.IntentSanitizer. - """ - .trimIndent() - context.report(Incident(ISSUE, visitor.location, message)) + // The anonymous component registered-at-runtime receiver case, report the issue + // immediately. + reportIssue(context, null, visitor.location) } else { - storeIncidentsToPartialResults(context, visitor) + reportIncident(context, visitor) } } } @@ -237,18 +273,19 @@ class UnsafeIntentLaunchDetector : Detector(), SourceCodeScanner, XmlScanner { val receiverArg = UastLintUtils.findArgument(call, method, BROADCAST_RECEIVER_CLASS) ?: return if (receiverArg.isNullLiteral()) return - if (!isRuntimeReceiverProtected(call, method)) { - val receiverVar = receiverArg.tryResolve() as? PsiVariable ?: return - val receiverAssignment = - findLastAssignment(receiverVar, call)?.skipParenthesizedExprDown() ?: return - val receiverConstructor = receiverAssignment.findSelector() as? UCallExpression + if (!isRuntimeReceiverProtected(call, method, context.evaluator)) { + val receiverConstructor = findConstruction(BROADCAST_RECEIVER_CLASS, receiverArg, call, true) val unprotectedReceiverClassName = receiverConstructor?.classReference.getQualifiedName() ?: return storeUnprotectedComponents(context, unprotectedReceiverClassName) } } - fun isRuntimeReceiverProtected(call: UCallExpression, method: PsiMethod): Boolean { + fun isRuntimeReceiverProtected( + call: UCallExpression, + method: PsiMethod, + javaEvaluator: JavaEvaluator + ): Boolean { // The parameter positions vary across the various registerReceiver*() methods, so rather // than hardcode them we simply look them up based on the parameter name and type. @@ -267,7 +304,7 @@ class UnsafeIntentLaunchDetector : Detector(), SourceCodeScanner, XmlScanner { BroadcastReceiverUtils.checkIsProtectedReceiverAndReturnUnprotectedActions( filterArg, call, - evaluator + javaEvaluator ) return isProtected @@ -281,18 +318,31 @@ class UnsafeIntentLaunchDetector : Detector(), SourceCodeScanner, XmlScanner { unprotectedComponents.put(unprotectedComponentName, true) } - private fun storeIncidentsToPartialResults(context: Context, visitor: IntentLaunchChecker) { - val lintMap = context.getPartialResults(ISSUE).map() - val incidents = lintMap.getMap(KEY_INCIDENTS) ?: map().also { lintMap.put(KEY_INCIDENTS, it) } - // key is not important. so the size of the map is used to make it unique. - incidents.put( - incidents.size.toString(), - map().apply { - put(KEY_LOCATION, visitor.location) - put(KEY_SECONDARY_LOCATION, visitor.location.secondary ?: return) - put(KEY_INCIDENT_CLASS, visitor.incidentClass ?: return) + private fun reportIncident(context: Context, visitor: IntentLaunchChecker) { + if (context.isGlobalAnalysis()) { + val incidentComponent = visitor.incidentClass + if ( + isComponentExported( + context, + context.mainProject.mergedManifest?.documentElement ?: return, + incidentComponent + ) + ) { + reportIssue(context, incidentComponent, visitor.location) } - ) + } else { + val lintMap = context.getPartialResults(ISSUE).map() + val incidents = lintMap.getMap(KEY_INCIDENTS) ?: map().also { lintMap.put(KEY_INCIDENTS, it) } + // key is not important. so the size of the map is used to make it unique. + incidents.put( + incidents.size.toString(), + map().apply { + put(KEY_LOCATION, visitor.location) + put(KEY_SECONDARY_LOCATION, visitor.location.secondary ?: return) + put(KEY_INCIDENT_CLASS, visitor.incidentClass ?: return) + } + ) + } } override fun afterCheckRootProject(context: Context) { @@ -310,19 +360,23 @@ class UnsafeIntentLaunchDetector : Detector(), SourceCodeScanner, XmlScanner { if (unprotectedComponents.containsKey(incidentComponent)) { val location = incidentMap.getLocation(KEY_LOCATION) ?: continue location.secondary = incidentMap.getLocation(KEY_SECONDARY_LOCATION) - val message = - """ - This intent could be coming from an untrusted source. It is later launched by \ - an unprotected component $incidentComponent. You could either make the component \ - $incidentComponent protected; or sanitize this intent using \ - androidx.core.content.IntentSanitizer. - """ - .trimIndent() - context.report(Incident(ISSUE, location, message)) + reportIssue(context, incidentComponent, location) } } } + private fun reportIssue(context: Context, incidentComponent: String?, location: Location) { + val component = if (incidentComponent.isNullOrBlank()) "" else " $incidentComponent" + val message = + """ + This intent could be coming from an untrusted source. It is later launched by \ + an unprotected component$component. You could either make the component$component \ + protected; or sanitize this intent using androidx.core.content.IntentSanitizer. + """ + .trimIndent() + context.report(Incident(ISSUE, location, message)) + } + private inner class IntentLaunchChecker( initial: Collection<UElement>, var context: JavaContext, @@ -331,7 +385,8 @@ class UnsafeIntentLaunchDetector : Detector(), SourceCodeScanner, XmlScanner { var launched: Boolean = false, var returned: Boolean = false, var unprotectedReceiver: Boolean = false, - var resolveCallDepth: Int = 0 + var resolveCallDepth: Int = 0, + var checkProtectedBroadcast: Boolean = false ) : DataFlowAnalyzer(initial) { override fun returnsSelf(call: UCallExpression): Boolean { @@ -374,9 +429,11 @@ class UnsafeIntentLaunchDetector : Detector(), SourceCodeScanner, XmlScanner { } } if (isIntentLaunchedBySystem(context.evaluator, call)) { - launched = true - location.secondary = context.getLocation(call) - location.secondary?.message = "The unsafe intent is launched here." + if (!checkProtectedBroadcast || !inProtectedBroadcastBranch(context, call, reference)) { + launched = true + location.secondary = context.getLocation(call) + location.secondary?.message = "The unsafe intent is launched here." + } } else { if (resolveCallDepth > MAX_CALL_DEPTH) return // escaped to another method call. check the method recursively. @@ -393,7 +450,7 @@ class UnsafeIntentLaunchDetector : Detector(), SourceCodeScanner, XmlScanner { ) containingMethod.accept(visitor) if (visitor.launched) { - storeIncidentsToPartialResults(context, visitor) + reportIncident(context, visitor) } else if (visitor.returned) { // if the visited method returns the passed-in unsafe Intent, add this call to track it. instances.add(call) @@ -401,6 +458,113 @@ class UnsafeIntentLaunchDetector : Detector(), SourceCodeScanner, XmlScanner { } } + /** Returns if the expression is evaluated to a protected broadcast action. */ + private fun isProtectedBroadcastAction(expression: UExpression?): Boolean { + return BroadcastReceiverUtils.isProtectedBroadcast( + ConstantEvaluator().allowFieldInitializers().evaluate(expression) as String + ) + } + + /** + * Check if the call is within a branch of code that is protected by a protected broadcast + * action. If could either be an if statement that checks if the action of the intent is equal + * to a protected action; or an equivalent of a switch case statement. + */ + private fun inProtectedBroadcastBranch( + context: JavaContext, + call: UCallExpression, + reference: UElement + ): Boolean { + return inProtectedBroadcastIfBranch(context, call, reference) || + inProtectedBroadcastSwitchCase(call, reference) + } + + private fun inProtectedBroadcastIfBranch( + context: JavaContext, + call: UCallExpression, + reference: UElement + ): Boolean { + var ifExp = call.getParentOfType<UIfExpression>() + while (ifExp != null) { + var op1: UExpression? = null + var op2: UExpression? = null + val condition = ifExp.condition + if (condition is UBinaryExpression && condition.operator === UastBinaryOperator.EQUALS) { + // handle kotlin == + op1 = condition.leftOperand + op2 = condition.rightOperand + } else if (condition is UQualifiedReferenceExpression) { + // handle java equals method. + val methodCall = condition.selector as? UCallExpression + if (methodCall?.methodName == "equals") { + op1 = condition.receiver + op2 = methodCall.valueArguments[0] + } + } + if ( + op1 != null && + op2 != null && + ((isIntentAction(op1, reference) && isProtectedBroadcastAction(op2)) || + (isIntentAction(op2, reference) && isProtectedBroadcastAction(op1))) + ) { + return context.getLocation(call) in context.getLocation(ifExp.thenExpression) + } + ifExp = ifExp.getParentOfType() + } + return false + } + + private fun inProtectedBroadcastSwitchCase( + call: UCallExpression, + reference: UElement + ): Boolean { + var switchExp = call.getParentOfType<USwitchExpression>() + while (switchExp != null) { + val subject = switchExp.expression as? UReferenceExpression + val caseExpression = call.getParentOfType<USwitchClauseExpression>() ?: return false + val caseValue = caseExpression.caseValues.firstOrNull() ?: return false + if ((caseValue.sourcePsi as? PsiSwitchLabelStatementBase)?.isDefaultCase == true) + return false + if (isIntentAction(subject, reference) && isProtectedBroadcastAction(caseValue)) return true + switchExp = switchExp.getParentOfType() + } + return false + } + + private fun isIntentAction(expression: UExpression?, intentRef: UElement): Boolean { + val actionAssignmentCall = findIntentActionAssignmentCall(expression) + return actionAssignmentCall?.receiver?.skipParenthesizedExprDown()?.tryResolve() === + intentRef.tryResolve() + } + + private fun findIntentActionAssignmentCall(expression: UExpression?): UCallExpression? { + val actionExpr = expression?.skipParenthesizedExprDown() + val resolved = actionExpr?.tryResolve() + + if (resolved is PsiVariable) { + val assignment = findLastAssignment(resolved, actionExpr) ?: return null + return findIntentActionAssignmentCall(assignment) + } + + if (actionExpr is UQualifiedReferenceExpression) { + val call = actionExpr.selector as? UCallExpression ?: return null + return if (isReturningContext(call)) { + // eg. intent.apply { setAction("abc") } --> use filter variable. + findIntentActionAssignmentCall(actionExpr.receiver) + } else { + // eg. intent.getAction("abc") --> use getAction("abc") UCallExpression. + findIntentActionAssignmentCall(call) + } + } + + val method = resolved as? PsiMethod ?: return null + return if ("getAction" == method.name) { + actionExpr as? UCallExpression + } else { + null + } + } + /** * Handles kotlin scoping function with "this" object reference, like run, with. If "this" is * tracked, get into the lambda function and keep track of "this". @@ -420,7 +584,7 @@ class UnsafeIntentLaunchDetector : Detector(), SourceCodeScanner, XmlScanner { ) lambda.body.accept(visitor) if (visitor.launched) { - storeIncidentsToPartialResults(context, visitor) + reportIncident(context, visitor) } } } @@ -488,7 +652,7 @@ class UnsafeIntentLaunchDetector : Detector(), SourceCodeScanner, XmlScanner { if (call.methodName in registerReceiverMethods) { val method = call.resolve() ?: return - if (!isRuntimeReceiverProtected(call, method)) { + if (!isRuntimeReceiverProtected(call, method, context.evaluator)) { result = true } } @@ -548,7 +712,8 @@ class UnsafeIntentLaunchDetector : Detector(), SourceCodeScanner, XmlScanner { private val IMPLEMENTATION = Implementation( UnsafeIntentLaunchDetector::class.java, - EnumSet.of(Scope.JAVA_FILE, Scope.MANIFEST) + EnumSet.of(Scope.JAVA_FILE, Scope.MANIFEST), + Scope.JAVA_FILE_SCOPE, ) /** Issue describing the problem and pointing to the detector implementation. */ |