diff --git a/java/ql/lib/change-notes/2022-09-22-android-deeplink-flow-steps.md b/java/ql/lib/change-notes/2022-09-22-android-deeplink-flow-steps.md new file mode 100644 index 00000000000..1ed229b1e05 --- /dev/null +++ b/java/ql/lib/change-notes/2022-09-22-android-deeplink-flow-steps.md @@ -0,0 +1,5 @@ +--- +category: minorAnalysis +--- +* Added data flow steps for tainted Android intents that are sent to services and receivers. +* Improved the data flow step for tainted Android intents that are sent to activities so that more cases are covered. diff --git a/java/ql/lib/change-notes/2022-09-22-android-deprecate-contextstartactivitymethod.md b/java/ql/lib/change-notes/2022-09-22-android-deprecate-contextstartactivitymethod.md new file mode 100644 index 00000000000..3500322afad --- /dev/null +++ b/java/ql/lib/change-notes/2022-09-22-android-deprecate-contextstartactivitymethod.md @@ -0,0 +1,4 @@ +--- +category: deprecated +--- +* Deprecated `ContextStartActivityMethod`. Use `StartActivityMethod` instead. 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 ad260c9545c..be38b83e5a7 100644 --- a/java/ql/lib/semmle/code/java/frameworks/android/Intent.qll +++ b/java/ql/lib/semmle/code/java/frameworks/android/Intent.qll @@ -3,9 +3,7 @@ private import semmle.code.java.dataflow.DataFlow private import semmle.code.java.dataflow.ExternalFlow private import semmle.code.java.dataflow.FlowSteps -/** - * The class `android.content.Intent`. - */ +/** The class `android.content.Intent`. */ class TypeIntent extends Class { TypeIntent() { this.hasQualifiedName("android.content", "Intent") } } @@ -15,23 +13,17 @@ class TypeComponentName extends Class { TypeComponentName() { this.hasQualifiedName("android.content", "ComponentName") } } -/** - * The class `android.app.Activity`. - */ +/** The class `android.app.Activity`. */ class TypeActivity extends Class { TypeActivity() { this.hasQualifiedName("android.app", "Activity") } } -/** - * The class `android.app.Service`. - */ +/** The class `android.app.Service`. */ class TypeService extends Class { TypeService() { this.hasQualifiedName("android.app", "Service") } } -/** - * The class `android.content.Context`. - */ +/** The class `android.content.Context`. */ class TypeContext extends RefType { // Not inlining this makes it more likely to be used as a sentinel, // which is useful when running Android queries on non-Android projects. @@ -39,25 +31,19 @@ class TypeContext extends RefType { TypeContext() { this.hasQualifiedName("android.content", "Context") } } -/** - * The class `android.content.BroadcastReceiver`. - */ +/** The class `android.content.BroadcastReceiver`. */ class TypeBroadcastReceiver extends Class { TypeBroadcastReceiver() { this.hasQualifiedName("android.content", "BroadcastReceiver") } } -/** - * The method `Activity.getIntent` - */ +/** The method `Activity.getIntent` */ class AndroidGetIntentMethod extends Method { AndroidGetIntentMethod() { this.hasName("getIntent") and this.getDeclaringType() instanceof TypeActivity } } -/** - * The method `BroadcastReceiver.onReceive`. - */ +/** The method `BroadcastReceiver.onReceive`. */ class AndroidReceiveIntentMethod extends Method { AndroidReceiveIntentMethod() { this.hasName("onReceive") and this.getDeclaringType() instanceof TypeBroadcastReceiver @@ -77,8 +63,10 @@ class AndroidServiceIntentMethod extends Method { /** * The method `Context.startActivity` or `startActivities`. + * + * DEPRECATED: Use `StartActivityMethod` instead. */ -class ContextStartActivityMethod extends Method { +deprecated class ContextStartActivityMethod extends Method { ContextStartActivityMethod() { (this.hasName("startActivity") or this.hasName("startActivities")) and this.getDeclaringType() instanceof TypeContext @@ -86,16 +74,57 @@ class ContextStartActivityMethod extends Method { } /** - * Specifies that if an `Intent` is tainted, then so are its synthetic fields. + * The method `Context.startActivity`, `Context.startActivities`, + * `Activity.startActivity`,`Activity.startActivities`, + * `Activity.startActivityForResult`, `Activity.startActivityIfNeeded`, + * `Activity.startNextMatchingActivity`, `Activity.startActivityFromChild`, + * or `Activity.startActivityFromFragment`. */ +class StartActivityMethod extends Method { + StartActivityMethod() { + this.getName().matches("start%Activit%") and + ( + this.getDeclaringType() instanceof TypeContext or + this.getDeclaringType() instanceof TypeActivity + ) + } +} + +/** + * The method `Context.sendBroadcast`, `sendBroadcastAsUser`, + * `sendOrderedBroadcast`, `sendOrderedBroadcastAsUser`, + * `sendStickyBroadcast`, `sendStickyBroadcastAsUser`, + * `sendStickyOrderedBroadcast`, `sendStickyOrderedBroadcastAsUser`, + * or `sendBroadcastWithMultiplePermissions`. + */ +class SendBroadcastMethod extends Method { + SendBroadcastMethod() { + this.getName().matches("send%Broadcast%") and + this.getDeclaringType() instanceof TypeContext + } +} + +/** + * The method `Context.startService`, `startForegroundService`, + * `bindIsolatedService`, `bindService`, or `bindServiceAsUser`. + */ +class StartServiceMethod extends Method { + StartServiceMethod() { + this.hasName([ + "startService", "startForegroundService", "bindIsolatedService", "bindService", + "bindServiceAsUser" + ]) and + this.getDeclaringType() instanceof TypeContext + } +} + +/** Specifies that if an `Intent` is tainted, then so are its synthetic fields. */ private class IntentFieldsInheritTaint extends DataFlow::SyntheticFieldContent, TaintInheritingContent { IntentFieldsInheritTaint() { this.getField().matches("android.content.Intent.%") } } -/** - * The method `Intent.getParcelableExtra`. - */ +/** The method `Intent.getParcelableExtra`. */ class IntentGetParcelableExtraMethod extends Method { IntentGetParcelableExtraMethod() { this.hasName("getParcelableExtra") and @@ -157,9 +186,7 @@ private class BundleExtrasSyntheticField extends SyntheticField { override RefType getType() { result instanceof AndroidBundle } } -/** - * Holds if extras may be implicitly read from the Intent `node`. - */ +/** Holds if extras may be implicitly read from the Intent `node`. */ predicate allowIntentExtrasImplicitRead(DataFlow::Node node, DataFlow::Content c) { node.getType() instanceof TypeIntent and ( @@ -194,25 +221,90 @@ class GrantWriteUriPermissionFlag extends GrantUriPermissionFlag { GrantWriteUriPermissionFlag() { this.hasName("FLAG_GRANT_WRITE_URI_PERMISSION") } } +/** An instantiation of `android.content.Intent`. */ +private class NewIntent extends ClassInstanceExpr { + NewIntent() { this.getConstructedType() instanceof TypeIntent } + + /** Gets the `Class` argument of this call. */ + Argument getClassArg() { + result.getType() instanceof TypeClass and + result = this.getAnArgument() + } +} + +/** A call to a method that starts an Android component. */ +private class StartComponentMethodAccess extends MethodAccess { + StartComponentMethodAccess() { + this.getMethod().overrides*(any(StartActivityMethod m)) or + this.getMethod().overrides*(any(StartServiceMethod m)) or + this.getMethod().overrides*(any(SendBroadcastMethod m)) + } + + /** Gets the intent argument of this call. */ + Argument getIntentArg() { + result.getType() instanceof TypeIntent and + result = this.getAnArgument() + } + + /** Holds if this targets a component of type `targetType`. */ + predicate targetsComponentType(RefType targetType) { + exists(NewIntent newIntent | + DataFlow::localExprFlow(newIntent, this.getIntentArg()) and + newIntent.getClassArg().getType().(ParameterizedType).getATypeArgument() = targetType + ) + } +} + /** - * A value-preserving step from the Intent argument of a `startActivity` call to - * a `getIntent` call in the Activity the Intent pointed to in its constructor. + * A value-preserving step from the intent argument of a `startActivity` call to + * a `getIntent` call in the activity the intent targeted in its constructor. */ private class StartActivityIntentStep extends AdditionalValueStep { override predicate step(DataFlow::Node n1, DataFlow::Node n2) { - exists(MethodAccess startActivity, MethodAccess getIntent, ClassInstanceExpr newIntent | - startActivity.getMethod().overrides*(any(ContextStartActivityMethod m)) and + exists(StartComponentMethodAccess startActivity, MethodAccess getIntent | + startActivity.getMethod().overrides*(any(StartActivityMethod m)) and getIntent.getMethod().overrides*(any(AndroidGetIntentMethod m)) and - newIntent.getConstructedType() instanceof TypeIntent and - DataFlow::localExprFlow(newIntent, startActivity.getArgument(0)) and - newIntent.getArgument(1).getType().(ParameterizedType).getATypeArgument() = - getIntent.getReceiverType() and - n1.asExpr() = startActivity.getArgument(0) and + startActivity.targetsComponentType(getIntent.getReceiverType()) and + n1.asExpr() = startActivity.getIntentArg() and n2.asExpr() = getIntent ) } } +/** + * A value-preserving step from the intent argument of a `sendBroadcast` call to + * the intent parameter in the `onReceive` method of the receiver the + * intent targeted in its constructor. + */ +private class SendBroadcastReceiverIntentStep extends AdditionalValueStep { + override predicate step(DataFlow::Node n1, DataFlow::Node n2) { + exists(StartComponentMethodAccess sendBroadcast, Method onReceive | + sendBroadcast.getMethod().overrides*(any(SendBroadcastMethod m)) and + onReceive.overrides*(any(AndroidReceiveIntentMethod m)) and + sendBroadcast.targetsComponentType(onReceive.getDeclaringType()) and + n1.asExpr() = sendBroadcast.getIntentArg() and + n2.asParameter() = onReceive.getParameter(1) + ) + } +} + +/** + * A value-preserving step from the intent argument of a `startService` call to + * the intent parameter in an `AndroidServiceIntentMethod` of the service the + * intent targeted in its constructor. + */ +private class StartServiceIntentStep extends AdditionalValueStep { + override predicate step(DataFlow::Node n1, DataFlow::Node n2) { + exists(StartComponentMethodAccess startService, Method serviceIntent | + startService.getMethod().overrides*(any(StartServiceMethod m)) and + serviceIntent.overrides*(any(AndroidServiceIntentMethod m)) and + startService.targetsComponentType(serviceIntent.getDeclaringType()) and + n1.asExpr() = startService.getIntentArg() and + n2.asParameter() = serviceIntent.getParameter(0) + ) + } +} + private class IntentBundleFlowSteps extends SummaryModelCsv { override predicate row(string row) { row = diff --git a/java/ql/test/library-tests/frameworks/android/intent/AndroidManifest.xml b/java/ql/test/library-tests/frameworks/android/intent/AndroidManifest.xml index 0be6a0ae8f8..0b7cc21200b 100644 --- a/java/ql/test/library-tests/frameworks/android/intent/AndroidManifest.xml +++ b/java/ql/test/library-tests/frameworks/android/intent/AndroidManifest.xml @@ -18,5 +18,30 @@ android:exported="false"> + + + + + + + + + + + + + + + diff --git a/java/ql/test/library-tests/frameworks/android/intent/TestStartActivityToGetIntent.java b/java/ql/test/library-tests/frameworks/android/intent/TestStartActivityToGetIntent.java index 3d497aac93d..35884a23a58 100644 --- a/java/ql/test/library-tests/frameworks/android/intent/TestStartActivityToGetIntent.java +++ b/java/ql/test/library-tests/frameworks/android/intent/TestStartActivityToGetIntent.java @@ -4,22 +4,90 @@ import android.content.Intent; public class TestStartActivityToGetIntent { - static Object source() { + static Object source(String kind) { return null; } static void sink(Object sink) {} - public void test(Context ctx) { - Intent intent = new Intent(null, SomeActivity.class); - intent.putExtra("data", (String) source()); - ctx.startActivity(intent); + public void test(Context ctx, Activity act) { + + // test all methods that start an activity + { + Intent intent = new Intent(null, SomeActivity.class); + intent.putExtra("data", (String) source("ctx-start")); + ctx.startActivity(intent); + } + { + Intent intent = new Intent(null, SomeActivity.class); + intent.putExtra("data", (String) source("ctx-start-acts")); + Intent[] intents = new Intent[] {intent}; + ctx.startActivities(intents); + } + { + Intent intent = new Intent(null, SomeActivity.class); + intent.putExtra("data", (String) source("act-start")); + act.startActivity(intent); + } + { + Intent intent = new Intent(null, SomeActivity.class); + intent.putExtra("data", (String) source("act-start-acts")); + Intent[] intents = new Intent[] {intent}; + act.startActivities(intents); + } + { + Intent intent = new Intent(null, SomeActivity.class); + intent.putExtra("data", (String) source("start-for-result")); + act.startActivityForResult(intent, 0); + } + { + Intent intent = new Intent(null, SomeActivity.class); + intent.putExtra("data", (String) source("start-if-needed")); + act.startActivityIfNeeded(intent, 0); + } + { + Intent intent = new Intent(null, SomeActivity.class); + intent.putExtra("data", (String) source("start-matching")); + act.startNextMatchingActivity(intent); + } + { + Intent intent = new Intent(null, SomeActivity.class); + intent.putExtra("data", (String) source("start-from-child")); + act.startActivityFromChild(null, intent, 0); + } + { + Intent intent = new Intent(null, SomeActivity.class); + intent.putExtra("data", (String) source("start-from-frag")); + act.startActivityFromFragment(null, intent, 0); + } + + // test 4-arg Intent constructor + { + Intent intent = new Intent(null, null, null, SomeActivity.class); + intent.putExtra("data", (String) source("4-arg")); + ctx.startActivity(intent); + } + + // safe test + { + Intent intent = new Intent(null, SafeActivity.class); + intent.putExtra("data", "safe"); + ctx.startActivity(intent); + } } static class SomeActivity extends Activity { public void test() { - sink(getIntent().getStringExtra("data")); // $ hasValueFlow + sink(getIntent().getStringExtra("data")); // $ hasValueFlow=ctx-start hasValueFlow=act-start hasValueFlow=start-for-result hasValueFlow=start-if-needed hasValueFlow=start-matching hasValueFlow=start-from-child hasValueFlow=start-from-frag hasValueFlow=4-arg MISSING: hasValueFlow=ctx-start-acts hasValueFlow=act-start-acts + } + + } + + static class SafeActivity extends Activity { + + public void test() { + sink(getIntent().getStringExtra("data")); // Safe } } } diff --git a/java/ql/test/library-tests/frameworks/android/intent/TestStartBroadcastReceiverToIntent.java b/java/ql/test/library-tests/frameworks/android/intent/TestStartBroadcastReceiverToIntent.java new file mode 100644 index 00000000000..f317b3ed2e8 --- /dev/null +++ b/java/ql/test/library-tests/frameworks/android/intent/TestStartBroadcastReceiverToIntent.java @@ -0,0 +1,93 @@ +import android.content.BroadcastReceiver; +import android.content.Context; +import android.content.Intent; + +public class TestStartBroadcastReceiverToIntent { + + static Object source(String kind) { + return null; + } + + static void sink(Object sink) {} + + public void test(Context ctx) { + + // test all methods that send a broadcast + { + Intent intent = new Intent(null, SomeBroadcastReceiver.class); + intent.putExtra("data", (String) source("send")); + ctx.sendBroadcast(intent); + } + { + Intent intent = new Intent(null, SomeBroadcastReceiver.class); + intent.putExtra("data", (String) source("send-as-user")); + ctx.sendBroadcastAsUser(intent, null); + } + { + Intent intent = new Intent(null, SomeBroadcastReceiver.class); + intent.putExtra("data", (String) source("send-with-perm")); + ctx.sendBroadcastWithMultiplePermissions(intent, null); + } + { + Intent intent = new Intent(null, SomeBroadcastReceiver.class); + intent.putExtra("data", (String) source("send-ordered")); + ctx.sendOrderedBroadcast(intent, null); + } + { + Intent intent = new Intent(null, SomeBroadcastReceiver.class); + intent.putExtra("data", (String) source("send-ordered-as-user")); + ctx.sendOrderedBroadcastAsUser(intent, null, null, null, null, 0, null, null); + } + { + Intent intent = new Intent(null, SomeBroadcastReceiver.class); + intent.putExtra("data", (String) source("send-sticky")); + ctx.sendStickyBroadcast(intent); + } + { + Intent intent = new Intent(null, SomeBroadcastReceiver.class); + intent.putExtra("data", (String) source("send-sticky-as-user")); + ctx.sendStickyBroadcastAsUser(intent, null); + } + { + Intent intent = new Intent(null, SomeBroadcastReceiver.class); + intent.putExtra("data", (String) source("send-sticky-ordered")); + ctx.sendStickyOrderedBroadcast(intent, null, null, 0, null, null); + } + { + Intent intent = new Intent(null, SomeBroadcastReceiver.class); + intent.putExtra("data", (String) source("send-sticky-ordered-as-user")); + ctx.sendStickyOrderedBroadcastAsUser(intent, null, null, null, 0, null, null); + } + + // test 4-arg Intent constructor + { + Intent intent = new Intent(null, null, null, SomeBroadcastReceiver.class); + intent.putExtra("data", (String) source("4-arg")); + ctx.sendBroadcast(intent); + } + + // safe test + { + Intent intent = new Intent(null, SafeBroadcastReceiver.class); + intent.putExtra("data", "safe"); + ctx.sendBroadcast(intent); + } + } + + static class SomeBroadcastReceiver extends BroadcastReceiver { + + // test method that receives an Intent as a parameter + @Override + public void onReceive(Context context, Intent intent) { + sink(intent.getStringExtra("data")); // $ hasValueFlow=send hasValueFlow=send-as-user hasValueFlow=send-with-perm hasValueFlow=send-ordered hasValueFlow=send-ordered-as-user hasValueFlow=send-sticky hasValueFlow=send-sticky-as-user hasValueFlow=send-sticky-ordered hasValueFlow=send-sticky-ordered-as-user hasValueFlow=4-arg + } + } + + static class SafeBroadcastReceiver extends BroadcastReceiver { + + @Override + public void onReceive(Context context, Intent intent) { + sink(intent.getStringExtra("data")); // Safe + } + } +} diff --git a/java/ql/test/library-tests/frameworks/android/intent/TestStartServiceToIntent.java b/java/ql/test/library-tests/frameworks/android/intent/TestStartServiceToIntent.java new file mode 100644 index 00000000000..98404eed1b7 --- /dev/null +++ b/java/ql/test/library-tests/frameworks/android/intent/TestStartServiceToIntent.java @@ -0,0 +1,130 @@ +import android.app.Service; +import android.os.IBinder; +import android.content.Context; +import android.content.Intent; + +public class TestStartServiceToIntent { + + static Object source(String kind) { + return null; + } + + static void sink(Object sink) {} + + public void test(Context ctx) { + + // test all methods that start a service + { + Intent intent = new Intent(null, SomeService.class); + intent.putExtra("data", (String) source("bind")); + ctx.bindService(intent, null, 0); + } + { + Intent intent = new Intent(null, SomeService.class); + intent.putExtra("data", (String) source("bind-as-user")); + ctx.bindServiceAsUser(intent, null, 0, null); + } + { + Intent intent = new Intent(null, SomeService.class); + intent.putExtra("data", (String) source("bind-isolated")); + ctx.bindIsolatedService(intent, 0, null, null, null); + } + { + Intent intent = new Intent(null, SomeService.class); + intent.putExtra("data", (String) source("start")); + ctx.startService(intent); + } + { + Intent intent = new Intent(null, SomeService.class); + intent.putExtra("data", (String) source("start-foreground")); + ctx.startForegroundService(intent); + } + + // test 4-arg Intent constructor + { + Intent intent = new Intent(null, null, null, SomeService.class); + intent.putExtra("data", (String) source("4-arg")); + ctx.startService(intent); + } + + // safe test + { + Intent intent = new Intent(null, SafeService.class); + intent.putExtra("data", "safe"); + ctx.startService(intent); + } + } + + static class SomeService extends Service { + + // test methods that receive an Intent as a parameter + @Override + public void onStart(Intent intent, int startId) { + sink(intent.getStringExtra("data")); // $ hasValueFlow=bind hasValueFlow=bind-as-user hasValueFlow=bind-isolated hasValueFlow=start hasValueFlow=start-foreground hasValueFlow=4-arg + } + + @Override + public int onStartCommand(Intent intent, int flags, int startId) { + sink(intent.getStringExtra("data")); // $ hasValueFlow=bind hasValueFlow=bind-as-user hasValueFlow=bind-isolated hasValueFlow=start hasValueFlow=start-foreground hasValueFlow=4-arg + return -1; + } + + @Override + public IBinder onBind(Intent intent) { + sink(intent.getStringExtra("data")); // $ hasValueFlow=bind hasValueFlow=bind-as-user hasValueFlow=bind-isolated hasValueFlow=start hasValueFlow=start-foreground hasValueFlow=4-arg + return null; + } + + @Override + public boolean onUnbind(Intent intent) { + sink(intent.getStringExtra("data")); // $ hasValueFlow=bind hasValueFlow=bind-as-user hasValueFlow=bind-isolated hasValueFlow=start hasValueFlow=start-foreground hasValueFlow=4-arg + return false; + } + + @Override + public void onRebind(Intent intent) { + sink(intent.getStringExtra("data")); // $ hasValueFlow=bind hasValueFlow=bind-as-user hasValueFlow=bind-isolated hasValueFlow=start hasValueFlow=start-foreground hasValueFlow=4-arg + } + + @Override + public void onTaskRemoved(Intent intent) { + sink(intent.getStringExtra("data")); // $ hasValueFlow=bind hasValueFlow=bind-as-user hasValueFlow=bind-isolated hasValueFlow=start hasValueFlow=start-foreground hasValueFlow=4-arg + } + } + + static class SafeService extends Service { + + @Override + public void onStart(Intent intent, int startId) { + sink(intent.getStringExtra("data")); // Safe + } + + @Override + public int onStartCommand(Intent intent, int flags, int startId) { + sink(intent.getStringExtra("data")); // Safe + return -1; + } + + @Override + public IBinder onBind(Intent intent) { + sink(intent.getStringExtra("data")); // Safe + return null; + } + + @Override + public boolean onUnbind(Intent intent) { + sink(intent.getStringExtra("data")); // Safe + return false; + } + + @Override + public void onRebind(Intent intent) { + sink(intent.getStringExtra("data")); // Safe + } + + @Override + public void onTaskRemoved(Intent intent) { + sink(intent.getStringExtra("data")); // Safe + } + } +}