diff --git a/java/ql/lib/change-notes/2022-09-23-android-service-sources.md b/java/ql/lib/change-notes/2022-09-23-android-service-sources.md new file mode 100644 index 00000000000..812ff07422d --- /dev/null +++ b/java/ql/lib/change-notes/2022-09-23-android-service-sources.md @@ -0,0 +1,4 @@ +--- +category: minorAnalysis +--- +* Added external flow sources for the intents received in exported Android services. diff --git a/java/ql/lib/semmle/code/java/dataflow/FlowSources.qll b/java/ql/lib/semmle/code/java/dataflow/FlowSources.qll index a6189337751..fcb4ac3b970 100644 --- a/java/ql/lib/semmle/code/java/dataflow/FlowSources.qll +++ b/java/ql/lib/semmle/code/java/dataflow/FlowSources.qll @@ -250,6 +250,12 @@ class AndroidIntentInput extends DataFlow::Node { this.asParameter() = m.getParameter(1) and receiverType = m.getDeclaringType() ) + or + exists(Method m, AndroidServiceIntentMethod sI | + m.overrides*(sI) and + this.asParameter() = m.getParameter(0) and + receiverType = m.getDeclaringType() + ) } } 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 51ee5088314..ad260c9545c 100644 --- a/java/ql/lib/semmle/code/java/frameworks/android/Intent.qll +++ b/java/ql/lib/semmle/code/java/frameworks/android/Intent.qll @@ -22,6 +22,13 @@ class TypeActivity extends Class { TypeActivity() { this.hasQualifiedName("android.app", "Activity") } } +/** + * The class `android.app.Service`. + */ +class TypeService extends Class { + TypeService() { this.hasQualifiedName("android.app", "Service") } +} + /** * The class `android.content.Context`. */ @@ -57,6 +64,17 @@ class AndroidReceiveIntentMethod extends Method { } } +/** + * The method `Service.onStart`, `onStartCommand`, + * `onBind`, `onRebind`, `onUnbind`, or `onTaskRemoved`. + */ +class AndroidServiceIntentMethod extends Method { + AndroidServiceIntentMethod() { + this.hasName(["onStart", "onStartCommand", "onBind", "onRebind", "onUnbind", "onTaskRemoved"]) and + this.getDeclaringType() instanceof TypeService + } +} + /** * The method `Context.startActivity` or `startActivities`. */ diff --git a/java/ql/test/experimental/query-tests/security/CWE-200/FileService.java b/java/ql/test/experimental/query-tests/security/CWE-200/FileService.java index 1e2895001e4..4641a975429 100644 --- a/java/ql/test/experimental/query-tests/security/CWE-200/FileService.java +++ b/java/ql/test/experimental/query-tests/security/CWE-200/FileService.java @@ -1,5 +1,5 @@ import java.io.FileOutputStream; - +import android.os.IBinder; import android.app.Service; import android.content.Intent; import android.net.Uri; @@ -52,13 +52,18 @@ public class FileService extends Service { @Override protected void onPostExecute(String result) { } - + @Override protected void onPreExecute() { } @Override protected void onProgressUpdate(Void... values) { - } + } + } + + @Override + public IBinder onBind(Intent intent) { + return null; } } diff --git a/java/ql/test/library-tests/dataflow/taintsources/AndroidManifest.xml b/java/ql/test/library-tests/dataflow/taintsources/AndroidManifest.xml index f9fe7eff10a..a9b8d4ad46b 100644 --- a/java/ql/test/library-tests/dataflow/taintsources/AndroidManifest.xml +++ b/java/ql/test/library-tests/dataflow/taintsources/AndroidManifest.xml @@ -18,7 +18,7 @@ - + @@ -28,6 +28,18 @@ + + + + + + + + + + + + diff --git a/java/ql/test/library-tests/dataflow/taintsources/IntentSources.java b/java/ql/test/library-tests/dataflow/taintsources/IntentSourcesActivity.java similarity index 85% rename from java/ql/test/library-tests/dataflow/taintsources/IntentSources.java rename to java/ql/test/library-tests/dataflow/taintsources/IntentSourcesActivity.java index 4753afb7405..3cd324a05db 100644 --- a/java/ql/test/library-tests/dataflow/taintsources/IntentSources.java +++ b/java/ql/test/library-tests/dataflow/taintsources/IntentSourcesActivity.java @@ -2,7 +2,7 @@ package com.example.myapp; import android.app.Activity; -public class IntentSources extends Activity { +public class IntentSourcesActivity extends Activity { private static void sink(Object o) {} @@ -26,14 +26,13 @@ public class IntentSources extends Activity { sink(trouble); // $hasRemoteTaintFlow } - } class OtherClass { private static void sink(Object o) {} - public void test(IntentSources is) throws java.io.IOException { + public void test(IntentSourcesActivity is) throws java.io.IOException { String trouble = is.getIntent().getStringExtra("key"); sink(trouble); // $hasRemoteTaintFlow } diff --git a/java/ql/test/library-tests/dataflow/taintsources/IntentSourcesReceiver.java b/java/ql/test/library-tests/dataflow/taintsources/IntentSourcesReceiver.java new file mode 100644 index 00000000000..b09ff0a449a --- /dev/null +++ b/java/ql/test/library-tests/dataflow/taintsources/IntentSourcesReceiver.java @@ -0,0 +1,22 @@ +package com.example.myapp; + +import android.content.BroadcastReceiver; +import android.content.Context; +import android.content.Intent; + +public class IntentSourcesReceiver extends BroadcastReceiver { + + private static void sink(Object o) {} + + @Override + public void onReceive(Context context, Intent intent) { + { + String trouble = intent.getStringExtra("data"); + sink(trouble); // $ hasRemoteTaintFlow + } + { + String trouble = intent.getExtras().getString("data"); + sink(trouble); // $ hasRemoteTaintFlow + } + } +} diff --git a/java/ql/test/library-tests/dataflow/taintsources/IntentSourcesService.java b/java/ql/test/library-tests/dataflow/taintsources/IntentSourcesService.java new file mode 100644 index 00000000000..d5f01bcd946 --- /dev/null +++ b/java/ql/test/library-tests/dataflow/taintsources/IntentSourcesService.java @@ -0,0 +1,87 @@ +package com.example.myapp; + +import android.app.Service; +import android.content.Context; +import android.content.Intent; +import android.os.IBinder; + +public class IntentSourcesService extends Service { + + private static void sink(Object o) {} + + @Override + public void onStart(Intent intent, int startId) { + { + String trouble = intent.getStringExtra("data"); + sink(trouble); // $ hasRemoteTaintFlow + } + { + String trouble = intent.getExtras().getString("data"); + sink(trouble); // $ hasRemoteTaintFlow + } + } + + @Override + public int onStartCommand(Intent intent, int flags, int startId) { + { + String trouble = intent.getStringExtra("data"); + sink(trouble); // $ hasRemoteTaintFlow + } + { + String trouble = intent.getExtras().getString("data"); + sink(trouble); // $ hasRemoteTaintFlow + } + return -1; + } + + @Override + public IBinder onBind(Intent intent) { + { + String trouble = intent.getStringExtra("data"); + sink(trouble); // $ hasRemoteTaintFlow + } + { + String trouble = intent.getExtras().getString("data"); + sink(trouble); // $ hasRemoteTaintFlow + } + return null; + } + + @Override + public boolean onUnbind(Intent intent) { + { + String trouble = intent.getStringExtra("data"); + sink(trouble); // $ hasRemoteTaintFlow + } + { + String trouble = intent.getExtras().getString("data"); + sink(trouble); // $ hasRemoteTaintFlow + } + return false; + } + + @Override + public void onRebind(Intent intent) { + { + String trouble = intent.getStringExtra("data"); + sink(trouble); // $ hasRemoteTaintFlow + } + { + String trouble = intent.getExtras().getString("data"); + sink(trouble); // $ hasRemoteTaintFlow + } + } + + @Override + public void onTaskRemoved(Intent intent) { + { + String trouble = intent.getStringExtra("data"); + sink(trouble); // $ hasRemoteTaintFlow + } + { + String trouble = intent.getExtras().getString("data"); + sink(trouble); // $ hasRemoteTaintFlow + } + } + +} diff --git a/java/ql/test/stubs/google-android-9.0.0/android/app/Service.java b/java/ql/test/stubs/google-android-9.0.0/android/app/Service.java index 370d7c5e4fe..8250b8917c2 100644 --- a/java/ql/test/stubs/google-android-9.0.0/android/app/Service.java +++ b/java/ql/test/stubs/google-android-9.0.0/android/app/Service.java @@ -17,6 +17,7 @@ package android.app; import android.content.Intent; import android.content.ContextWrapper; +import android.os.IBinder; /** * A Service is an application component representing either an application's desire @@ -28,7 +29,7 @@ import android.content.ContextWrapper; * can be started with * {@link android.content.Context#startService Context.startService()} and * {@link android.content.Context#bindService Context.bindService()}. - * + * *

Note that services, like other application objects, run in the main * thread of their hosting process. This means that, if your service is going * to do any CPU intensive (such as MP3 playback) or blocking (such as @@ -38,7 +39,7 @@ import android.content.ContextWrapper; * Threads. The {@link IntentService} class is available * as a standard implementation of Service that has its own thread where it * schedules its work to be done.

- * + * *

Topics covered here: *

    *
  1. What is a Service? @@ -57,10 +58,10 @@ import android.content.ContextWrapper; * * *

    What is a Service?

    - * + * *

    Most confusion about the Service class actually revolves around what * it is not:

    - * + * *
      *
    • A Service is not a separate process. The Service object itself * does not imply it is running in its own process; unless otherwise specified, @@ -68,9 +69,9 @@ import android.content.ContextWrapper; *
    • A Service is not a thread. It is not a means itself to do work off * of the main thread (to avoid Application Not Responding errors). *
    - * + * *

    Thus a Service itself is actually very simple, providing two main features:

    - * + * *
      *
    • A facility for the application to tell the system about * something it wants to be doing in the background (even when the user is not @@ -84,22 +85,22 @@ import android.content.ContextWrapper; * allows a long-standing connection to be made to the service in order to * interact with it. *
    - * + * *

    When a Service component is actually created, for either of these reasons, * all that the system actually does is instantiate the component * and call its {@link #onCreate} and any other appropriate callbacks on the * main thread. It is up to the Service to implement these with the appropriate * behavior, such as creating a secondary thread in which it does its work.

    - * + * *

    Note that because Service itself is so simple, you can make your * interaction with it as simple or complicated as you want: from treating it * as a local Java object that you make direct method calls on (as illustrated * by Local Service Sample), to providing * a full remoteable interface using AIDL.

    - * + * * *

    Service Lifecycle

    - * + * *

    There are two reasons that a service can be run by the system. If someone * calls {@link android.content.Context#startService Context.startService()} then the system will * retrieve the service (creating it and calling its {@link #onCreate} method @@ -112,7 +113,7 @@ import android.content.ContextWrapper; * will be stopped once Context.stopService() or stopSelf() is called; however, * services can use their {@link #stopSelf(int)} method to ensure the service is * not stopped until started intents have been processed. - * + * *

    For started services, there are two additional major modes of operation * they can decide to run in, depending on the value they return from * onStartCommand(): {@link #START_STICKY} is used for services that are @@ -120,7 +121,7 @@ import android.content.ContextWrapper; * or {@link #START_REDELIVER_INTENT} are used for services that should only * remain running while processing any commands sent to them. See the linked * documentation for more detail on the semantics. - * + * *

    Clients can also use {@link android.content.Context#bindService Context.bindService()} to * obtain a persistent connection to a service. This likewise creates the * service if it is not already running (calling {@link #onCreate} while @@ -132,7 +133,7 @@ import android.content.ContextWrapper; * service's IBinder). Usually the IBinder returned is for a complex * interface that has been written * in aidl. - * + * *

    A service can be both started and have connections bound to it. In such * a case, the system will keep the service running as long as either it is * started or there are one or more connections to it with the @@ -141,10 +142,10 @@ import android.content.ContextWrapper; * of these situations hold, the service's {@link #onDestroy} method is called * and the service is effectively terminated. All cleanup (stopping threads, * unregistering receivers) should be complete upon returning from onDestroy(). - * + * * *

    Permissions

    - * + * *

    Global access to a service can be enforced when it is declared in its * manifest's {@link android.R.styleable#AndroidManifestService <service>} * tag. By doing so, other applications will need to declare a corresponding @@ -168,13 +169,13 @@ import android.content.ContextWrapper; * permissions, by calling the * {@link #checkCallingPermission} * method before executing the implementation of that call. - * + * *

    See the Security and Permissions * document for more information on permissions and security in general. - * + * * *

    Process Lifecycle

    - * + * *

    The Android system will attempt to keep the process hosting a service * around as long as the service has been started or has clients bound to it. * When running low on memory and needing to kill existing processes, the @@ -211,7 +212,7 @@ import android.content.ContextWrapper; * the service to be killed under extreme memory pressure from the current * foreground application, but in practice this should not be a concern.) * - * + * *

    Note this means that most of the time your service is running, it may * be killed by the system if it is under heavy memory pressure. If this * happens, the system will later try to restart the service. An important @@ -220,67 +221,67 @@ import android.content.ContextWrapper; * may want to use {@link #START_FLAG_REDELIVERY} to have the system * re-deliver an Intent for you so that it does not get lost if your service * is killed while processing it. - * + * *

    Other application components running in the same process as the service * (such as an {@link android.app.Activity}) can, of course, increase the * importance of the overall * process beyond just the importance of the service itself. - * + * * *

    Local Service Sample

    - * + * *

    One of the most common uses of a Service is as a secondary component * running alongside other parts of an application, in the same process as * the rest of the components. All components of an .apk run in the same * process unless explicitly stated otherwise, so this is a typical situation. - * + * *

    When used in this way, by assuming the * components are in the same process, you can greatly simplify the interaction * between them: clients of the service can simply cast the IBinder they * receive from it to a concrete class published by the service. - * + * *

    An example of this use of a Service is shown here. First is the Service * itself, publishing a custom class when bound: - * + * * {@sample development/samples/ApiDemos/src/com/example/android/apis/app/LocalService.java * service} - * + * *

    With that done, one can now write client code that directly accesses the * running service, such as: - * + * * {@sample development/samples/ApiDemos/src/com/example/android/apis/app/LocalServiceActivities.java * bind} - * + * * *

    Remote Messenger Service Sample

    - * + * *

    If you need to be able to write a Service that can perform complicated * communication with clients in remote processes (beyond simply the use of * {@link Context#startService(Intent) Context.startService} to send * commands to it), then you can use the {@link android.os.Messenger} class * instead of writing full AIDL files. - * + * *

    An example of a Service that uses Messenger as its client interface * is shown here. First is the Service itself, publishing a Messenger to * an internal Handler when bound: - * + * * {@sample development/samples/ApiDemos/src/com/example/android/apis/app/MessengerService.java * service} - * + * *

    If we want to make this service run in a remote process (instead of the * standard one for its .apk), we can use android:process in its * manifest tag to specify one: - * + * * {@sample development/samples/ApiDemos/AndroidManifest.xml remote_service_declaration} - * + * *

    Note that the name "remote" chosen here is arbitrary, and you can use * other names if you want additional processes. The ':' prefix appends the * name to your package's standard process name. - * + * *

    With that done, clients can now bind to the service and send messages * to it. Note that this allows clients to register with it to receive * messages back as well: - * + * * {@sample development/samples/ApiDemos/src/com/example/android/apis/app/MessengerServiceActivities.java * bind} */ @@ -299,14 +300,14 @@ public abstract class Service extends ContextWrapper { } /** - * Called by the system every time a client explicitly starts the service by calling - * {@link android.content.Context#startService}, providing the arguments it supplied and a + * Called by the system every time a client explicitly starts the service by calling + * {@link android.content.Context#startService}, providing the arguments it supplied and a * unique integer token representing the start request. Do not call this method directly. - * + * *

    For backwards compatibility, the default implementation calls * {@link #onStart} and returns either {@link #START_STICKY} * or {@link #START_STICKY_COMPATIBILITY}. - * + * *

    Note that the system calls this on your * service's main thread. A service's main thread is the same * thread where UI operations take place for Activities running in the @@ -315,18 +316,18 @@ public abstract class Service extends ContextWrapper { * network calls, or heavy disk I/O, you should kick off a new * thread, or use {@link android.os.AsyncTask}.

    * - * @param intent The Intent supplied to {@link android.content.Context#startService}, + * @param intent The Intent supplied to {@link android.content.Context#startService}, * as given. This may be null if the service is being restarted after * its process has gone away, and it had previously returned anything * except {@link #START_STICKY_COMPATIBILITY}. * @param flags Additional data about this start request. - * @param startId A unique integer representing this specific request to + * @param startId A unique integer representing this specific request to * start. Use with {@link #stopSelfResult(int)}. - * + * * @return The return value indicates what semantics the system should * use for the service's current started state. It may be one of the * constants associated with the {@link #START_CONTINUATION_MASK} bits. - * + * * @see #stopSelfResult(int) */ public int onStartCommand(Intent intent, int flags, int startId) { @@ -341,4 +342,14 @@ public abstract class Service extends ContextWrapper { */ public void onDestroy() { } + + public abstract IBinder onBind(Intent intent); + + public void onRebind(Intent intent) {} + + public void onTaskRemoved(Intent rootIntent) {} + + public boolean onUnbind(Intent intent) { + return false; + } }