From decba39c09549af108b256980cdaf0cecfdeaacf Mon Sep 17 00:00:00 2001 From: Jami Cogswell Date: Mon, 19 Sep 2022 15:54:45 -0400 Subject: [PATCH 01/10] add service flow sources --- .../semmle/code/java/dataflow/FlowSources.qll | 6 +++++ .../code/java/frameworks/android/Intent.qll | 23 +++++++++++++++++++ 2 files changed, 29 insertions(+) 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..04cf7d742fd 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,22 @@ class AndroidReceiveIntentMethod extends Method { } } +/** + * A method of type Service that receives an Intent. + * Namely, `Service.onStart`, `onStartCommand`, `onBind`, + * `onRebind`, `onUnbind`, or `onTaskRemoved` + */ +class AndroidServiceIntentMethod extends Method { + AndroidServiceIntentMethod() { + ( + this.getName().matches("onStart%") or + this.getName().matches("on%ind") or + this.hasName("onTaskRemoved") + ) and + this.getDeclaringType() instanceof TypeService + } +} + /** * The method `Context.startActivity` or `startActivities`. */ From 367c31bf17db9bc0d8c9d80df7015fd27aee8c64 Mon Sep 17 00:00:00 2001 From: Jami Cogswell Date: Mon, 19 Sep 2022 16:08:46 -0400 Subject: [PATCH 02/10] add change note --- .../ql/lib/change-notes/2022-09-19-android-service-sources.md | 4 ++++ 1 file changed, 4 insertions(+) create mode 100644 java/ql/lib/change-notes/2022-09-19-android-service-sources.md diff --git a/java/ql/lib/change-notes/2022-09-19-android-service-sources.md b/java/ql/lib/change-notes/2022-09-19-android-service-sources.md new file mode 100644 index 00000000000..812ff07422d --- /dev/null +++ b/java/ql/lib/change-notes/2022-09-19-android-service-sources.md @@ -0,0 +1,4 @@ +--- +category: minorAnalysis +--- +* Added external flow sources for the intents received in exported Android services. From 7a96727c59354f891cabc0123c12ed9453680aef Mon Sep 17 00:00:00 2001 From: Jami Cogswell Date: Thu, 22 Sep 2022 11:03:17 -0400 Subject: [PATCH 03/10] add tests --- .../dataflow/taintsources/AndroidManifest.xml | 14 +- ...ources.java => IntentSourcesActivity.java} | 12 +- .../taintsources/IntentSourcesReceiver.java | 17 + .../taintsources/IntentSourcesService.java | 52 +++ .../android/app/Service.java | 392 +++++++++++------- 5 files changed, 328 insertions(+), 159 deletions(-) rename java/ql/test/library-tests/dataflow/taintsources/{IntentSources.java => IntentSourcesActivity.java} (72%) create mode 100644 java/ql/test/library-tests/dataflow/taintsources/IntentSourcesReceiver.java create mode 100644 java/ql/test/library-tests/dataflow/taintsources/IntentSourcesService.java 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 72% rename from java/ql/test/library-tests/dataflow/taintsources/IntentSources.java rename to java/ql/test/library-tests/dataflow/taintsources/IntentSourcesActivity.java index 4753afb7405..2709494f5a4 100644 --- a/java/ql/test/library-tests/dataflow/taintsources/IntentSources.java +++ b/java/ql/test/library-tests/dataflow/taintsources/IntentSourcesActivity.java @@ -1,10 +1,12 @@ package com.example.myapp; import android.app.Activity; +import android.content.Intent; -public class IntentSources extends Activity { +public class IntentSourcesActivity extends Activity { - private static void sink(Object o) {} + private static void sink(Object o) { + } public void test() throws java.io.IOException { @@ -26,14 +28,14 @@ public class IntentSources extends Activity { sink(trouble); // $hasRemoteTaintFlow } - } class OtherClass { - private static void sink(Object o) {} + 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..956486df8d8 --- /dev/null +++ b/java/ql/test/library-tests/dataflow/taintsources/IntentSourcesReceiver.java @@ -0,0 +1,17 @@ +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 + } +} 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..24dfc069991 --- /dev/null +++ b/java/ql/test/library-tests/dataflow/taintsources/IntentSourcesService.java @@ -0,0 +1,52 @@ +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 + } + + @Override + public int onStartCommand(Intent intent, int flags, int startId) { + String trouble = intent.getStringExtra("data"); + sink(trouble); // $ hasRemoteTaintFlow + return -1; + } + + @Override + public IBinder onBind(Intent intent) { + String trouble = intent.getStringExtra("data"); + sink(trouble); // $ hasRemoteTaintFlow + return null; + } + + @Override + public boolean onUnbind(Intent intent) { + String trouble = intent.getStringExtra("data"); + sink(trouble); // $ hasRemoteTaintFlow + return false; + } + + @Override + public void onRebind(Intent intent) { + String trouble = intent.getStringExtra("data"); + sink(trouble); // $ hasRemoteTaintFlow + } + + @Override + public void onTaskRemoved(Intent intent) { + String trouble = intent.getStringExtra("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..dd270248a52 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,276 +17,339 @@ 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 + * A Service is an application component representing either an application's + * desire * to perform a longer-running operation while not interacting with the user - * or to supply functionality for other applications to use. Each service + * or to supply functionality for other applications to use. Each service * class must have a corresponding * {@link android.R.styleable#AndroidManifestService <service>} - * declaration in its package's AndroidManifest.xml. Services + * declaration in its package's AndroidManifest.xml. Services * 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 + * + *

+ * 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 * networking) operations, it should spawn its own thread in which to do that - * work. More information on this can be found in - * Processes and - * Threads. The {@link IntentService} class is available + * work. More information on this can be found in + * Processes and + * 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: + * schedules its work to be done. + *

+ * + *

+ * Topics covered here: *

    *
  1. What is a Service? *
  2. Service Lifecycle *
  3. Permissions *
  4. Process Lifecycle *
  5. Local Service Sample - *
  6. Remote Messenger Service Sample + *
  7. Remote Messenger Service + * Sample *
* *
*

Developer Guides

- *

For a detailed discussion about how to create services, read the - * Services developer guide.

+ *

+ * For a detailed discussion about how to create services, read the + * Services developer + * guide. + *

*
* * *

What is a Service?

- * - *

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

- * + * + *

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

+ * *
    - *
  • A Service is not a separate process. The Service object itself + *
  • A Service is not a separate process. The Service object itself * does not imply it is running in its own process; unless otherwise specified, * it runs in the same process as the application it is part of. - *
  • A Service is not a thread. It is not a means itself to do work off + *
  • 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:

- * + * + *

+ * 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 - * directly interacting with the application). This corresponds to calls to + * directly interacting with the application). This corresponds to calls to * {@link android.content.Context#startService Context.startService()}, which * ask the system to schedule work for the service, to be run until the service * or someone else explicitly stop it. *
  • A facility for an application to expose some of its functionality to - * other applications. This corresponds to calls to + * other applications. This corresponds to calls to * {@link android.content.Context#bindService Context.bindService()}, which * 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, + * + *

+ * 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 + * 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.

- * + * 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 + * + *

+ * 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 * if needed) and then call its {@link #onStartCommand} method with the - * arguments supplied by the client. The service will at this point continue - * running until {@link android.content.Context#stopService Context.stopService()} or - * {@link #stopSelf()} is called. Note that multiple calls to - * Context.startService() do not nest (though they do result in multiple corresponding - * calls to onStartCommand()), so no matter how many times it is started a service + * arguments supplied by the client. The service will at this point continue + * running until {@link android.content.Context#stopService + * Context.stopService()} or + * {@link #stopSelf()} is called. Note that multiple calls to + * Context.startService() do not nest (though they do result in multiple + * corresponding + * calls to onStartCommand()), so no matter how many times it is started a + * service * 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 + * + *

+ * 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 * explicitly started and stopped as needed, while {@link #START_NOT_STICKY} * or {@link #START_REDELIVER_INTENT} are used for services that should only - * remain running while processing any commands sent to them. See the linked + * 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 + * + *

+ * 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 - * doing so), but does not call onStartCommand(). The client will receive the + * doing so), but does not call onStartCommand(). The client will receive the * {@link android.os.IBinder} object that the service returns from its * {@link #onBind} method, allowing the client to then make calls back - * to the service. The service will remain running as long as the connection + * to the service. The service will remain running as long as the connection * is established (whether or not the client retains a reference on the - * service's IBinder). Usually the IBinder returned is for a complex - * interface that has been written + * 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 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 * {@link android.content.Context#BIND_AUTO_CREATE Context.BIND_AUTO_CREATE} - * flag. Once neither + * flag. Once neither * of these situations hold, the service's {@link #onDestroy} method is called - * and the service is effectively terminated. All cleanup (stopping threads, + * 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 + * + *

+ * 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 - * {@link android.R.styleable#AndroidManifestUsesPermission <uses-permission>} + * tag. By doing so, other applications will need to declare a corresponding + * {@link android.R.styleable#AndroidManifestUsesPermission + * <uses-permission>} * element in their own manifest to be able to start, stop, or bind to * the service. * - *

As of {@link android.os.Build.VERSION_CODES#GINGERBREAD}, when using + *

+ * As of {@link android.os.Build.VERSION_CODES#GINGERBREAD}, when using * {@link Context#startService(Intent) Context.startService(Intent)}, you can * also set {@link Intent#FLAG_GRANT_READ_URI_PERMISSION - * Intent.FLAG_GRANT_READ_URI_PERMISSION} and/or {@link Intent#FLAG_GRANT_WRITE_URI_PERMISSION - * Intent.FLAG_GRANT_WRITE_URI_PERMISSION} on the Intent. This will grant the - * Service temporary access to the specific URIs in the Intent. Access will + * Intent.FLAG_GRANT_READ_URI_PERMISSION} and/or + * {@link Intent#FLAG_GRANT_WRITE_URI_PERMISSION + * Intent.FLAG_GRANT_WRITE_URI_PERMISSION} on the Intent. This will grant the + * Service temporary access to the specific URIs in the Intent. Access will * remain until the Service has called {@link #stopSelf(int)} for that start * command or a later one, or until the Service has been completely stopped. * This works for granting access to the other apps that have not requested * the permission protecting the Service, or even when the Service is not * exported at all. * - *

In addition, a service can protect individual IPC calls into it with + *

+ * In addition, a service can protect individual IPC calls into it with * permissions, by calling the * {@link #checkCallingPermission} * method before executing the implementation of that call. - * - *

See the Security and Permissions + * + *

+ * 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 + * + *

+ * 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 * priority of a process hosting the service will be the higher of the * following possibilities: * *

    - *

  • If the service is currently executing code in its + *

  • + *

    + * If the service is currently executing code in its * {@link #onCreate onCreate()}, {@link #onStartCommand onStartCommand()}, * or {@link #onDestroy onDestroy()} methods, then the hosting process will * be a foreground process to ensure this code can execute without * being killed. - *

  • If the service has been started, then its hosting process is considered + *

  • + *

    + * If the service has been started, then its hosting process is considered * to be less important than any processes that are currently visible to the - * user on-screen, but more important than any process not visible. Because + * user on-screen, but more important than any process not visible. Because * only a few processes are generally visible to the user, this means that - * the service should not be killed except in low memory conditions. However, since - * the user is not directly aware of a background service, in that state it is + * the service should not be killed except in low memory conditions. However, + * since + * the user is not directly aware of a background service, in that state it + * is * considered a valid candidate to kill, and you should be prepared for this to - * happen. In particular, long-running services will be increasingly likely to + * happen. In particular, long-running services will be increasingly likely to * kill and are guaranteed to be killed (and restarted if appropriate) if they * remain started long enough. - *

  • If there are clients bound to the service, then the service's hosting - * process is never less important than the most important client. That is, + *

  • + *

    + * If there are clients bound to the service, then the service's hosting + * process is never less important than the most important client. That is, * if one of its clients is visible to the user, then the service itself is - * considered to be visible. The way a client's importance impacts the service's + * considered to be visible. The way a client's importance impacts the service's * importance can be adjusted through {@link Context#BIND_ABOVE_CLIENT}, - * {@link Context#BIND_ALLOW_OOM_MANAGEMENT}, {@link Context#BIND_WAIVE_PRIORITY}, - * {@link Context#BIND_IMPORTANT}, and {@link Context#BIND_ADJUST_WITH_ACTIVITY}. - *

  • A started service can use the {@link #startForeground(int, Notification)} + * {@link Context#BIND_ALLOW_OOM_MANAGEMENT}, + * {@link Context#BIND_WAIVE_PRIORITY}, + * {@link Context#BIND_IMPORTANT}, and + * {@link Context#BIND_ADJUST_WITH_ACTIVITY}. + *

  • + *

    + * A started service can use the {@link #startForeground(int, Notification)} * API to put the service in a foreground state, where the system considers * it to be something the user is actively aware of and thus not a candidate - * for killing when low on memory. (It is still theoretically possible for + * for killing when low on memory. (It is still theoretically possible for * 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 - * consequence of this is that if you implement {@link #onStartCommand onStartCommand()} + * + *

+ * 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 + * consequence of this is that if you implement {@link #onStartCommand + * onStartCommand()} * to schedule work to be done asynchronously or in another thread, then you * 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 + * + *

+ * 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 + * + *

+ * 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 + * 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 + * + *

+ * 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 + * + *

+ * 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 + * 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} - * + * bind} + * * *

Remote Messenger Service Sample

- * - *

If you need to be able to write a Service that can perform complicated + * + *

+ * 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 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 + * 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 + * + * {@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 + * + *

+ * 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} + * bind} */ public abstract class Service extends ContextWrapper { /** - * Called by the system when the service is first created. Do not call this method directly. + * Called by the system when the service is first created. Do not call this + * method directly. */ public void onCreate() { } @@ -299,34 +362,43 @@ 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 - * unique integer token representing the start request. Do not call this method directly. - * - *

For backwards compatibility, the default implementation calls + * 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 - * same process. You should always avoid stalling the main - * thread's event loop. When doing long-running operations, - * 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}, - * 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 - * start. Use with {@link #stopSelfResult(int)}. - * + *

+ * 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 + * same process. You should always avoid stalling the main + * thread's event loop. When doing long-running operations, + * 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}, + * 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 + * 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. - * + * 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) { @@ -334,11 +406,25 @@ public abstract class Service extends ContextWrapper { } /** - * Called by the system to notify a Service that it is no longer used and is being removed. The + * Called by the system to notify a Service that it is no longer used and is + * being removed. The * service should clean up any resources it holds (threads, registered - * receivers, etc) at this point. Upon return, there will be no more calls - * in to this Service object and it is effectively dead. Do not call this method directly. + * receivers, etc) at this point. Upon return, there will be no more calls + * in to this Service object and it is effectively dead. Do not call this method + * directly. */ 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; + } } From 24b34cd32f8230e33e71bb52069d47c85eaf74f3 Mon Sep 17 00:00:00 2001 From: Jami Cogswell Date: Thu, 22 Sep 2022 11:44:03 -0400 Subject: [PATCH 04/10] add a few more tests, and some clean-up --- .../code/java/frameworks/android/Intent.qll | 5 +- .../taintsources/IntentSourcesActivity.java | 1 - .../taintsources/IntentSourcesReceiver.java | 10 +++- .../taintsources/IntentSourcesService.java | 60 +++++++++++++++---- 4 files changed, 58 insertions(+), 18 deletions(-) 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 04cf7d742fd..a51348d5c34 100644 --- a/java/ql/lib/semmle/code/java/frameworks/android/Intent.qll +++ b/java/ql/lib/semmle/code/java/frameworks/android/Intent.qll @@ -65,9 +65,8 @@ class AndroidReceiveIntentMethod extends Method { } /** - * A method of type Service that receives an Intent. - * Namely, `Service.onStart`, `onStartCommand`, `onBind`, - * `onRebind`, `onUnbind`, or `onTaskRemoved` + * The method `Service.onStart`, `onStartCommand`, + * `onBind`, `onRebind`, `onUnbind`, or `onTaskRemoved` */ class AndroidServiceIntentMethod extends Method { AndroidServiceIntentMethod() { diff --git a/java/ql/test/library-tests/dataflow/taintsources/IntentSourcesActivity.java b/java/ql/test/library-tests/dataflow/taintsources/IntentSourcesActivity.java index 2709494f5a4..2c065c16821 100644 --- a/java/ql/test/library-tests/dataflow/taintsources/IntentSourcesActivity.java +++ b/java/ql/test/library-tests/dataflow/taintsources/IntentSourcesActivity.java @@ -1,7 +1,6 @@ package com.example.myapp; import android.app.Activity; -import android.content.Intent; public class IntentSourcesActivity extends Activity { diff --git a/java/ql/test/library-tests/dataflow/taintsources/IntentSourcesReceiver.java b/java/ql/test/library-tests/dataflow/taintsources/IntentSourcesReceiver.java index 956486df8d8..e7ca2ac6d73 100644 --- a/java/ql/test/library-tests/dataflow/taintsources/IntentSourcesReceiver.java +++ b/java/ql/test/library-tests/dataflow/taintsources/IntentSourcesReceiver.java @@ -11,7 +11,13 @@ public class IntentSourcesReceiver extends BroadcastReceiver { @Override public void onReceive(Context context, Intent intent) { - String trouble = intent.getStringExtra("data"); - sink(trouble); // $ hasRemoteTaintFlow + { + 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 index 24dfc069991..0b7eabd3788 100644 --- a/java/ql/test/library-tests/dataflow/taintsources/IntentSourcesService.java +++ b/java/ql/test/library-tests/dataflow/taintsources/IntentSourcesService.java @@ -12,41 +12,77 @@ public class IntentSourcesService extends Service { @Override public void onStart(Intent intent, int startId) { - String trouble = intent.getStringExtra("data"); - sink(trouble); // $ hasRemoteTaintFlow + { + 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.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.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.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.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.getStringExtra("data"); + sink(trouble); // $ hasRemoteTaintFlow + } + { + String trouble = intent.getExtras().getString("data"); + sink(trouble); // $ hasRemoteTaintFlow + } } } From 7e13610d24d9861cbd9532e37cd5002e1c13b8fa Mon Sep 17 00:00:00 2001 From: Jami Cogswell Date: Thu, 22 Sep 2022 11:49:47 -0400 Subject: [PATCH 05/10] minor qldoc update --- java/ql/lib/semmle/code/java/frameworks/android/Intent.qll | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 a51348d5c34..97d16286c7b 100644 --- a/java/ql/lib/semmle/code/java/frameworks/android/Intent.qll +++ b/java/ql/lib/semmle/code/java/frameworks/android/Intent.qll @@ -66,7 +66,7 @@ class AndroidReceiveIntentMethod extends Method { /** * The method `Service.onStart`, `onStartCommand`, - * `onBind`, `onRebind`, `onUnbind`, or `onTaskRemoved` + * `onBind`, `onRebind`, `onUnbind`, or `onTaskRemoved`. */ class AndroidServiceIntentMethod extends Method { AndroidServiceIntentMethod() { From 65f3ae9829362d07e371462f50a1ca5044f65d91 Mon Sep 17 00:00:00 2001 From: Jami Cogswell Date: Fri, 23 Sep 2022 15:39:28 -0400 Subject: [PATCH 06/10] clean up files --- ... => 2022-09-23-android-service-sources.md} | 0 .../code/java/frameworks/android/Intent.qll | 6 +- .../taintsources/IntentSourcesActivity.java | 6 +- .../taintsources/IntentSourcesReceiver.java | 3 +- .../taintsources/IntentSourcesService.java | 3 +- .../android/app/Service.java | 300 +++++++----------- 6 files changed, 117 insertions(+), 201 deletions(-) rename java/ql/lib/change-notes/{2022-09-19-android-service-sources.md => 2022-09-23-android-service-sources.md} (100%) diff --git a/java/ql/lib/change-notes/2022-09-19-android-service-sources.md b/java/ql/lib/change-notes/2022-09-23-android-service-sources.md similarity index 100% rename from java/ql/lib/change-notes/2022-09-19-android-service-sources.md rename to java/ql/lib/change-notes/2022-09-23-android-service-sources.md 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 97d16286c7b..9d36b93e18c 100644 --- a/java/ql/lib/semmle/code/java/frameworks/android/Intent.qll +++ b/java/ql/lib/semmle/code/java/frameworks/android/Intent.qll @@ -70,11 +70,7 @@ class AndroidReceiveIntentMethod extends Method { */ class AndroidServiceIntentMethod extends Method { AndroidServiceIntentMethod() { - ( - this.getName().matches("onStart%") or - this.getName().matches("on%ind") or - this.hasName("onTaskRemoved") - ) and + this.getName().matches(["onStart%", "on%ind", "onTaskRemoved"]) and this.getDeclaringType() instanceof TypeService } } diff --git a/java/ql/test/library-tests/dataflow/taintsources/IntentSourcesActivity.java b/java/ql/test/library-tests/dataflow/taintsources/IntentSourcesActivity.java index 2c065c16821..3cd324a05db 100644 --- a/java/ql/test/library-tests/dataflow/taintsources/IntentSourcesActivity.java +++ b/java/ql/test/library-tests/dataflow/taintsources/IntentSourcesActivity.java @@ -4,8 +4,7 @@ import android.app.Activity; public class IntentSourcesActivity extends Activity { - private static void sink(Object o) { - } + private static void sink(Object o) {} public void test() throws java.io.IOException { @@ -31,8 +30,7 @@ public class IntentSourcesActivity extends Activity { class OtherClass { - private static void sink(Object o) { - } + private static void sink(Object o) {} public void test(IntentSourcesActivity is) throws java.io.IOException { String trouble = is.getIntent().getStringExtra("key"); diff --git a/java/ql/test/library-tests/dataflow/taintsources/IntentSourcesReceiver.java b/java/ql/test/library-tests/dataflow/taintsources/IntentSourcesReceiver.java index e7ca2ac6d73..b09ff0a449a 100644 --- a/java/ql/test/library-tests/dataflow/taintsources/IntentSourcesReceiver.java +++ b/java/ql/test/library-tests/dataflow/taintsources/IntentSourcesReceiver.java @@ -6,8 +6,7 @@ import android.content.Intent; public class IntentSourcesReceiver extends BroadcastReceiver { - private static void sink(Object o) { - } + private static void sink(Object o) {} @Override public void onReceive(Context context, Intent intent) { diff --git a/java/ql/test/library-tests/dataflow/taintsources/IntentSourcesService.java b/java/ql/test/library-tests/dataflow/taintsources/IntentSourcesService.java index 0b7eabd3788..d5f01bcd946 100644 --- a/java/ql/test/library-tests/dataflow/taintsources/IntentSourcesService.java +++ b/java/ql/test/library-tests/dataflow/taintsources/IntentSourcesService.java @@ -7,8 +7,7 @@ import android.os.IBinder; public class IntentSourcesService extends Service { - private static void sink(Object o) { - } + private static void sink(Object o) {} @Override public void onStart(Intent intent, int startId) { 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 dd270248a52..5cb4c85e20a 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,263 +17,211 @@ 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 + * A Service is an application component representing either an application's desire * to perform a longer-running operation while not interacting with the user - * or to supply functionality for other applications to use. Each service + * or to supply functionality for other applications to use. Each service * class must have a corresponding * {@link android.R.styleable#AndroidManifestService <service>} - * declaration in its package's AndroidManifest.xml. Services + * declaration in its package's AndroidManifest.xml. Services * 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 + *

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 * networking) operations, it should spawn its own thread in which to do that - * work. More information on this can be found in - * Processes and - * Threads. The {@link IntentService} class is available + * work. More information on this can be found in + * Processes and + * 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. - *

+ * schedules its work to be done.

* - *

- * Topics covered here: + *

Topics covered here: *

    *
  1. What is a Service? *
  2. Service Lifecycle *
  3. Permissions *
  4. Process Lifecycle *
  5. Local Service Sample - *
  6. Remote Messenger Service - * Sample + *
  7. Remote Messenger Service Sample *
* *
*

Developer Guides

- *

- * For a detailed discussion about how to create services, read the - * Services developer - * guide. - *

+ *

For a detailed discussion about how to create services, read the + * Services developer guide.

*
* * *

What is a Service?

* - *

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

+ *

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

* *
    - *
  • A Service is not a separate process. The Service object itself + *
  • A Service is not a separate process. The Service object itself * does not imply it is running in its own process; unless otherwise specified, * it runs in the same process as the application it is part of. - *
  • A Service is not a thread. It is not a means itself to do work off + *
  • 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: - *

+ *

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 - * directly interacting with the application). This corresponds to calls to + * directly interacting with the application). This corresponds to calls to * {@link android.content.Context#startService Context.startService()}, which * ask the system to schedule work for the service, to be run until the service * or someone else explicitly stop it. *
  • A facility for an application to expose some of its functionality to - * other applications. This corresponds to calls to + * other applications. This corresponds to calls to * {@link android.content.Context#bindService Context.bindService()}, which * 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, + *

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. - *

+ * 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 + *

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. - *

+ * 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 + *

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 * if needed) and then call its {@link #onStartCommand} method with the - * arguments supplied by the client. The service will at this point continue - * running until {@link android.content.Context#stopService - * Context.stopService()} or - * {@link #stopSelf()} is called. Note that multiple calls to - * Context.startService() do not nest (though they do result in multiple - * corresponding - * calls to onStartCommand()), so no matter how many times it is started a - * service + * arguments supplied by the client. The service will at this point continue + * running until {@link android.content.Context#stopService Context.stopService()} or + * {@link #stopSelf()} is called. Note that multiple calls to + * Context.startService() do not nest (though they do result in multiple corresponding + * calls to onStartCommand()), so no matter how many times it is started a service * 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 + *

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 * explicitly started and stopped as needed, while {@link #START_NOT_STICKY} * or {@link #START_REDELIVER_INTENT} are used for services that should only - * remain running while processing any commands sent to them. See the linked + * 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 + *

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 - * doing so), but does not call onStartCommand(). The client will receive the + * doing so), but does not call onStartCommand(). The client will receive the * {@link android.os.IBinder} object that the service returns from its * {@link #onBind} method, allowing the client to then make calls back - * to the service. The service will remain running as long as the connection + * to the service. The service will remain running as long as the connection * is established (whether or not the client retains a reference on the - * service's IBinder). Usually the IBinder returned is for a complex - * interface that has been - * written + * 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 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 * {@link android.content.Context#BIND_AUTO_CREATE Context.BIND_AUTO_CREATE} - * flag. Once neither + * flag. Once neither * of these situations hold, the service's {@link #onDestroy} method is called - * and the service is effectively terminated. All cleanup (stopping threads, + * 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 + *

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 - * {@link android.R.styleable#AndroidManifestUsesPermission - * <uses-permission>} + * tag. By doing so, other applications will need to declare a corresponding + * {@link android.R.styleable#AndroidManifestUsesPermission <uses-permission>} * element in their own manifest to be able to start, stop, or bind to * the service. * - *

- * As of {@link android.os.Build.VERSION_CODES#GINGERBREAD}, when using + *

As of {@link android.os.Build.VERSION_CODES#GINGERBREAD}, when using * {@link Context#startService(Intent) Context.startService(Intent)}, you can * also set {@link Intent#FLAG_GRANT_READ_URI_PERMISSION - * Intent.FLAG_GRANT_READ_URI_PERMISSION} and/or - * {@link Intent#FLAG_GRANT_WRITE_URI_PERMISSION - * Intent.FLAG_GRANT_WRITE_URI_PERMISSION} on the Intent. This will grant the - * Service temporary access to the specific URIs in the Intent. Access will + * Intent.FLAG_GRANT_READ_URI_PERMISSION} and/or {@link Intent#FLAG_GRANT_WRITE_URI_PERMISSION + * Intent.FLAG_GRANT_WRITE_URI_PERMISSION} on the Intent. This will grant the + * Service temporary access to the specific URIs in the Intent. Access will * remain until the Service has called {@link #stopSelf(int)} for that start * command or a later one, or until the Service has been completely stopped. * This works for granting access to the other apps that have not requested * the permission protecting the Service, or even when the Service is not * exported at all. * - *

- * In addition, a service can protect individual IPC calls into it with + *

In addition, a service can protect individual IPC calls into it with * permissions, by calling the * {@link #checkCallingPermission} * method before executing the implementation of that call. * - *

- * See the Security and - * Permissions + *

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 + *

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 * priority of a process hosting the service will be the higher of the * following possibilities: * *

    - *
  • - *

    - * If the service is currently executing code in its + *

  • If the service is currently executing code in its * {@link #onCreate onCreate()}, {@link #onStartCommand onStartCommand()}, * or {@link #onDestroy onDestroy()} methods, then the hosting process will * be a foreground process to ensure this code can execute without * being killed. - *

  • - *

    - * If the service has been started, then its hosting process is considered + *

  • If the service has been started, then its hosting process is considered * to be less important than any processes that are currently visible to the - * user on-screen, but more important than any process not visible. Because + * user on-screen, but more important than any process not visible. Because * only a few processes are generally visible to the user, this means that - * the service should not be killed except in low memory conditions. However, - * since - * the user is not directly aware of a background service, in that state it - * is + * the service should not be killed except in low memory conditions. However, since + * the user is not directly aware of a background service, in that state it is * considered a valid candidate to kill, and you should be prepared for this to - * happen. In particular, long-running services will be increasingly likely to + * happen. In particular, long-running services will be increasingly likely to * kill and are guaranteed to be killed (and restarted if appropriate) if they * remain started long enough. - *

  • - *

    - * If there are clients bound to the service, then the service's hosting - * process is never less important than the most important client. That is, + *

  • If there are clients bound to the service, then the service's hosting + * process is never less important than the most important client. That is, * if one of its clients is visible to the user, then the service itself is - * considered to be visible. The way a client's importance impacts the service's + * considered to be visible. The way a client's importance impacts the service's * importance can be adjusted through {@link Context#BIND_ABOVE_CLIENT}, - * {@link Context#BIND_ALLOW_OOM_MANAGEMENT}, - * {@link Context#BIND_WAIVE_PRIORITY}, - * {@link Context#BIND_IMPORTANT}, and - * {@link Context#BIND_ADJUST_WITH_ACTIVITY}. - *

  • - *

    - * A started service can use the {@link #startForeground(int, Notification)} + * {@link Context#BIND_ALLOW_OOM_MANAGEMENT}, {@link Context#BIND_WAIVE_PRIORITY}, + * {@link Context#BIND_IMPORTANT}, and {@link Context#BIND_ADJUST_WITH_ACTIVITY}. + *

  • A started service can use the {@link #startForeground(int, Notification)} * API to put the service in a foreground state, where the system considers * it to be something the user is actively aware of and thus not a candidate - * for killing when low on memory. (It is still theoretically possible for + * for killing when low on memory. (It is still theoretically possible for * 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 - * consequence of this is that if you implement {@link #onStartCommand - * onStartCommand()} + *

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 + * consequence of this is that if you implement {@link #onStartCommand onStartCommand()} * to schedule work to be done asynchronously or in another thread, then you * 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 + *

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. @@ -281,75 +229,64 @@ import android.os.IBinder; * *

Local Service Sample

* - *

- * One of the most common uses of a Service is as a secondary component + *

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 + * 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 + *

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 + *

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} + * service} * - *

- * With that done, one can now write client code that directly accesses the + *

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} + * bind} * * *

Remote Messenger Service Sample

* - *

- * If you need to be able to write a Service that can perform complicated + *

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 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} + * service} * - *

- * If we want to make this service run in a remote process (instead of the + *

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} + * {@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 + *

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 + *

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} + * bind} */ public abstract class Service extends ContextWrapper { /** - * Called by the system when the service is first created. Do not call this - * method directly. + * Called by the system when the service is first created. Do not call this method directly. */ public void onCreate() { } @@ -362,42 +299,33 @@ 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 - * unique integer token representing the start request. Do not call this method - * directly. + * 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 + *

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 + *

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 - * same process. You should always avoid stalling the main - * thread's event loop. When doing long-running operations, + * same process. You should always avoid stalling the main + * thread's event loop. When doing long-running operations, * network calls, or heavy disk I/O, you should kick off a new - * thread, or use {@link android.os.AsyncTask}. - *

+ * thread, or use {@link android.os.AsyncTask}.

* - * @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 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 - * start. Use with {@link #stopSelfResult(int)}. + * 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. + * 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) */ @@ -406,23 +334,19 @@ public abstract class Service extends ContextWrapper { } /** - * Called by the system to notify a Service that it is no longer used and is - * being removed. The + * Called by the system to notify a Service that it is no longer used and is being removed. The * service should clean up any resources it holds (threads, registered - * receivers, etc) at this point. Upon return, there will be no more calls - * in to this Service object and it is effectively dead. Do not call this method - * directly. + * receivers, etc) at this point. Upon return, there will be no more calls + * in to this Service object and it is effectively dead. Do not call this method directly. */ public void onDestroy() { } public abstract IBinder onBind(Intent intent); - public void onRebind(Intent intent) { - } + public void onRebind(Intent intent) {} - public void onTaskRemoved(Intent rootIntent) { - } + public void onTaskRemoved(Intent rootIntent) {} public boolean onUnbind(Intent intent) { return false; From 9acda05dbd567e0a7e51788238a58e6823115c3c Mon Sep 17 00:00:00 2001 From: Jami Cogswell Date: Fri, 23 Sep 2022 16:12:11 -0400 Subject: [PATCH 07/10] update Service stub --- java/ql/test/stubs/google-android-9.0.0/android/app/Service.java | 1 + 1 file changed, 1 insertion(+) 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 5cb4c85e20a..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 From 1e016575776b89fe6b79a852cbc6e3ca26612ee6 Mon Sep 17 00:00:00 2001 From: Jami Cogswell Date: Fri, 23 Sep 2022 18:57:35 -0400 Subject: [PATCH 08/10] add onBind to FileService to see if it fixes Java Language Tests failure --- .../query-tests/security/CWE-200/FileService.java | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) 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..b7c81c87bbb 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 @@ -5,6 +5,7 @@ import android.content.Intent; import android.net.Uri; import android.os.Bundle; import android.os.AsyncTask; +import android.os.IBinder; public class FileService extends Service { public static String KEY_LOCAL_FILE = "local_file"; @@ -27,6 +28,11 @@ public class FileService extends Service { return 2; } + @Override + public IBinder onBind(Intent intent) { + return null; + } + public class CopyAndUploadContentUrisTask extends AsyncTask { public Object[] makeParamsToExecute( String sourceUri @@ -52,13 +58,13 @@ public class FileService extends Service { @Override protected void onPostExecute(String result) { } - + @Override protected void onPreExecute() { } @Override protected void onProgressUpdate(Void... values) { - } + } } } From 9b4201f8803aa0721143650e58a6ae3d992ee8e3 Mon Sep 17 00:00:00 2001 From: Jami Cogswell Date: Fri, 23 Sep 2022 22:46:55 -0400 Subject: [PATCH 09/10] update FileService --- .../query-tests/security/CWE-200/FileService.java | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) 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 b7c81c87bbb..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,11 +1,10 @@ import java.io.FileOutputStream; - +import android.os.IBinder; import android.app.Service; import android.content.Intent; import android.net.Uri; import android.os.Bundle; import android.os.AsyncTask; -import android.os.IBinder; public class FileService extends Service { public static String KEY_LOCAL_FILE = "local_file"; @@ -28,11 +27,6 @@ public class FileService extends Service { return 2; } - @Override - public IBinder onBind(Intent intent) { - return null; - } - public class CopyAndUploadContentUrisTask extends AsyncTask { public Object[] makeParamsToExecute( String sourceUri @@ -67,4 +61,9 @@ public class FileService extends Service { protected void onProgressUpdate(Void... values) { } } + + @Override + public IBinder onBind(Intent intent) { + return null; + } } From 7e0c61de2c7216df5f04c141c9d24b81cb3a7b3d Mon Sep 17 00:00:00 2001 From: Jami Cogswell Date: Tue, 27 Sep 2022 10:45:52 -0400 Subject: [PATCH 10/10] switch to hasName --- java/ql/lib/semmle/code/java/frameworks/android/Intent.qll | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 9d36b93e18c..ad260c9545c 100644 --- a/java/ql/lib/semmle/code/java/frameworks/android/Intent.qll +++ b/java/ql/lib/semmle/code/java/frameworks/android/Intent.qll @@ -70,7 +70,7 @@ class AndroidReceiveIntentMethod extends Method { */ class AndroidServiceIntentMethod extends Method { AndroidServiceIntentMethod() { - this.getName().matches(["onStart%", "on%ind", "onTaskRemoved"]) and + this.hasName(["onStart", "onStartCommand", "onBind", "onRebind", "onUnbind", "onTaskRemoved"]) and this.getDeclaringType() instanceof TypeService } }