firebase / quickstart-android

Firebase Quickstart Samples for Android
https://firebase.google.com
Apache License 2.0
8.82k stars 7.32k forks source link

Ambiguity in Risk of Exporting FirebaseMessagingService-type Services Without Permissions #868

Open jleadford opened 5 years ago

jleadford commented 5 years ago

I have a general question.

This pull request: https://github.com/firebase/quickstart-android/pull/850

explicitly disables the export of the sample FirebaseMessaging Service, i.e.

<service android:name=".java.MyFirebaseMessagingService" android:exported="false">

because, and it says, "Set FirebaseMessagingServices to exported="false" to explicitly prevent other apps from being able to send messages to it."

This is a good practice, but other documentation (see e.g. https://stackoverflow.com/a/43991861) notes that runtime checks (in the FCM code) prevent other applications from calling into this Service arbitrarily.

Experimentally, this seems to be the case:

  1. Create some app that extends the FirebaseMessaging class, and notes the following block in its manifest

` <service android:name=".java.MyFirebaseMessagingService"">

` which was the default before the noted PR. 2. Set a breakpoint on the extended class's onMessageReceived method, in e.g. Android Studio 3. Run the application under Debug, foreground it, and issue the following ADB command (with suitably replaced options) to start the Service with an intent `adb shell am startservice -n com.test.android.app/com.foo.java.MyFirebaseMessagingService -a com.google.firebase.MESSAGING_EVENT` 4. Notice that your breakpoint is not hit, even though the Intent was sent successfully This was repeated with various intent extras, as well. And with runtime method hooking of onMessageReceived with https://www.frida.re/, to log if it is called. So, just wondering if someone could clear up this ambiguity-- is it actually a risk to export said service (i.e. com.google.firebase.messaging.FirebaseMessagingService and classes that extend it) without permissions (as the PR implies), or is it not (as the noted documentation states)?
google-oss-bot commented 5 years ago

This issue does not seem to follow the issue template. Make sure you provide all the required information.