diff --git a/java/ql/lib/semmle/code/java/frameworks/android/Intent.qll b/java/ql/lib/semmle/code/java/frameworks/android/Intent.qll index 296d6b39b2b..3d526ef47da 100644 --- a/java/ql/lib/semmle/code/java/frameworks/android/Intent.qll +++ b/java/ql/lib/semmle/code/java/frameworks/android/Intent.qll @@ -77,6 +77,61 @@ class IntentGetParcelableExtraMethod extends Method { } } +/** The class `android.os.BaseBundle`, or a class that extends it. */ +class AndroidBundle extends Class { + AndroidBundle() { this.getASupertype*().hasQualifiedName("android.os", "BaseBundle") } +} + +/** An `Intent` that explicitly sets a destination component. */ +class ExplicitIntent extends Expr { + ExplicitIntent() { + exists(MethodAccess ma, Method m | + ma.getMethod() = m and + m.getDeclaringType() instanceof TypeIntent and + m.hasName(["setPackage", "setClass", "setClassName", "setComponent"]) and + ma.getQualifier() = this + ) + or + exists(ConstructorCall cc, Argument classArg | + cc.getConstructedType() instanceof TypeIntent and + cc.getAnArgument() = classArg and + classArg.getType() instanceof TypeClass and + not exists(NullLiteral nullLiteral | DataFlow::localExprFlow(nullLiteral, classArg)) and + cc = this + ) + } +} + +/** + * A sanitizer for explicit intents. + * + * Use this when you want to work only with implicit intents + * in a `DataFlow` or `TaintTracking` configuration. + */ +class ExplicitIntentSanitizer extends DataFlow::Node { + ExplicitIntentSanitizer() { + exists(ExplicitIntent explIntent | DataFlow::localExprFlow(explIntent, this.asExpr())) + } +} + +private class BundleExtrasSyntheticField extends SyntheticField { + BundleExtrasSyntheticField() { this = "android.content.Intent.extras" } + + override RefType getType() { result instanceof AndroidBundle } +} + +/** + * Holds if extras may be implicitly read from the Intent `node`. + */ +predicate allowIntentExtrasImplicitRead(DataFlow::Node node, DataFlow::Content c) { + node.getType() instanceof TypeIntent and + ( + c instanceof DataFlow::MapValueContent + or + c.(DataFlow::SyntheticFieldContent).getType() instanceof AndroidBundle + ) +} + private class IntentBundleFlowSteps extends SummaryModelCsv { override predicate row(string row) { row = diff --git a/java/ql/lib/semmle/code/java/security/AndroidSensitiveCommunicationQuery.qll b/java/ql/lib/semmle/code/java/security/AndroidSensitiveCommunicationQuery.qll index a03f74d5044..2d5f5b0fd85 100644 --- a/java/ql/lib/semmle/code/java/security/AndroidSensitiveCommunicationQuery.qll +++ b/java/ql/lib/semmle/code/java/security/AndroidSensitiveCommunicationQuery.qll @@ -121,31 +121,6 @@ private predicate isStartActivityOrServiceSink(DataFlow::Node arg) { ) } -private predicate isCleanIntent(Expr intent) { - intent.getType() instanceof TypeIntent and - ( - exists(MethodAccess setRecieverMa | - setRecieverMa.getQualifier() = intent and - setRecieverMa.getMethod().hasName(["setPackage", "setClass", "setClassName", "setComponent"]) - ) - or - // Handle the cases where the PackageContext and Class are set at construction time - // Intent(Context packageContext, Class cls) - // Intent(String action, Uri uri, Context packageContext, Class cls) - exists(ConstructorCall cc | cc = intent | - cc.getConstructedType() instanceof TypeIntent and - cc.getNumArgument() > 1 and - ( - cc.getArgument(0).getType() instanceof TypeContext and - not maybeNullArg(cc.getArgument(1)) - or - cc.getArgument(2).getType() instanceof TypeContext and - not maybeNullArg(cc.getArgument(3)) - ) - ) - ) -} - /** * Taint configuration tracking flow from variables containing sensitive information to broadcast Intents. */ @@ -165,11 +140,7 @@ class SensitiveCommunicationConfig extends TaintTracking::Configuration { /** * Holds if broadcast doesn't specify receiving package name of the 3rd party app */ - override predicate isSanitizer(DataFlow::Node node) { - exists(DataFlow::Node intent | isCleanIntent(intent.asExpr()) | - DataFlow::localFlow(intent, node) - ) - } + override predicate isSanitizer(DataFlow::Node node) { node instanceof ExplicitIntentSanitizer } override predicate allowImplicitRead(DataFlow::Node node, DataFlow::Content c) { super.allowImplicitRead(node, c) diff --git a/java/ql/src/experimental/Security/CWE/CWE-200/AndroidFileIntentSource.qll b/java/ql/src/experimental/Security/CWE/CWE-200/AndroidFileIntentSource.qll index 22935997afe..15eefaaeb63 100644 --- a/java/ql/src/experimental/Security/CWE/CWE-200/AndroidFileIntentSource.qll +++ b/java/ql/src/experimental/Security/CWE/CWE-200/AndroidFileIntentSource.qll @@ -48,11 +48,7 @@ class GetContentIntentConfig extends TaintTracking2::Configuration { // Allow the wrapped intent created by Intent.getChooser to be consumed // by at the sink: isSink(node) and - ( - content.(DataFlow::SyntheticFieldContent).getField() = "android.content.Intent.extras" - or - content instanceof DataFlow::MapValueContent - ) + allowIntentExtrasImplicitRead(node, content) } }