Android issue: Sensitive data being saved on xml file

[Eris-Borovci]

Currently, the react-native-firebase library stores Firebase Cloud Messaging (FCM) notification data in the SharedPreferences file named io.invertase.firebase.xml in plain text. This poses a security risk, as sensitive notification details and metadata can be easily accessed on a compromised device. There is no configuration option to disable this storage or to use encrypted storage (e.g., EncryptedSharedPreferences) by default.

Steps to reproduce:

1. Trigger a background FCM notification.
2. Observe that the notification data is stored in io.invertase.firebase.xml in plain text.
3. Verify that no configuration option exists to encrypt or disable this storage.

We either need a configuration flag or secure defaults option that either:

• Prevents storing notification data in plain text altogether, or
• Uses EncryptedSharedPreferences to ensure that the data is encrypted at rest.

I was able to create a patch that converts encrypts the data saved on xml,

diff --git a/node_modules/@react-native-firebase/app/android/build.gradle b/node_modules/@react-native-firebase/app/android/build.gradle
index 98da166..0cf4288 100644
--- a/node_modules/@react-native-firebase/app/android/build.gradle
+++ b/node_modules/@react-native-firebase/app/android/build.gradle
@@ -103,6 +103,7 @@ dependencies {
   implementation platform("com.google.firebase:firebase-bom:${ReactNative.ext.getVersion("firebase", "bom")}")
   implementation "com.google.firebase:firebase-common"
   implementation "com.google.android.gms:play-services-auth:${ReactNative.ext.getVersion("play", "play-services-auth")}"
+  implementation 'androidx.security:security-crypto:1.1.0-alpha03'
 }
 
 ReactNative.shared.applyPackageVersion()
diff --git a/node_modules/@react-native-firebase/app/android/src/main/java/io/invertase/firebase/common/UniversalFirebasePreferences.java b/node_modules/@react-native-firebase/app/android/src/main/java/io/invertase/firebase/common/UniversalFirebasePreferences.java
index 2c7d0ca..c9652f6 100644
--- a/node_modules/@react-native-firebase/app/android/src/main/java/io/invertase/firebase/common/UniversalFirebasePreferences.java
+++ b/node_modules/@react-native-firebase/app/android/src/main/java/io/invertase/firebase/common/UniversalFirebasePreferences.java
@@ -17,9 +17,14 @@ package io.invertase.firebase.common;
  *
  */
 
-import android.content.Context;
 import android.content.SharedPreferences;
+
 import io.invertase.firebase.app.ReactNativeFirebaseApp;
+import androidx.security.crypto.EncryptedSharedPreferences;
+import androidx.security.crypto.MasterKey;
+
+import java.io.IOException;
+import java.security.GeneralSecurityException;
 
 public class UniversalFirebasePreferences {
   private static final String PREFERENCES_FILE = "io.invertase.firebase";
@@ -79,11 +84,23 @@ public class UniversalFirebasePreferences {
   }
 
   private SharedPreferences getPreferences() {
-    if (preferences == null) {
-      preferences =
-          ReactNativeFirebaseApp.getApplicationContext()
-              .getSharedPreferences(PREFERENCES_FILE, Context.MODE_PRIVATE);
+    try {
+      if (preferences == null) {
+
+        var context =  ReactNativeFirebaseApp.getApplicationContext();
+        var masterKey = new MasterKey.Builder(context).setKeyScheme(MasterKey.KeyScheme.AES256_GCM).build();
+
+        preferences = EncryptedSharedPreferences.create(
+          context,
+          PREFERENCES_FILE,
+          masterKey,
+          EncryptedSharedPreferences.PrefKeyEncryptionScheme.AES256_SIV,
+          EncryptedSharedPreferences.PrefValueEncryptionScheme.AES256_GCM
+        );
+      }
+      return preferences;
+    } catch (IOException | GeneralSecurityException e) {
+      throw new RuntimeException(e);
     }
-    return preferences;
   }
 }

[mikehardy]

Interesting - I'd be okay with an encrypted-by-default stance but I have some questions:

• why depend on an alpha library? implementation 'androidx.security:security-crypto:1.1.0-alpha03'
• what happens with existing data the first time you roll out a version with this patch?
• why choose AES256 vs something/anything else?

[Eris-Borovci]

• Android Studio suggested me the alpha version by default
• Yes i am trying to implement a type of migration for the old data
• The AES256 encryption is virtually uncrackable using any brute-force method, I did consider AES128 because its quite faster but because of my highly sensitive data coming through notifications I decided to choose safety

[mikehardy]

In my experience migrations are incredibly hard to get correct, despite the best efforts of the most skilled programmers. There can be device-specific failures at critical moments that just cannot be handled well. Really hard to get correct.

Fortunately, a different design of the migration could be:

• implement encrypted storage for preference files with a new prefs file key
• use encrypted storage for all new data
• when attempting to fetch data, fetch from encrypted first, if it fails, attempt fetch from old unencrypted
• if data is deleted from prefs, delete from encrypted and unencrypted

I believe this is secure for all new data, allows retrieval of old data, but will converge on "only encrypted data" even for upgrading clients.

What do you think? Any variation in the details would be fine, but this avoids migration by instead sort of doing a facade over the read/delete operations to paper over the dual-implementation

[Eris-Borovci]

First I wanna ask you about the importance of io.invertase.firebase.xml?

Why do we need the notifications data that we have clicked or even deleted to still exist there?

[mikehardy]

If you don't save the payload, it can be lost in certain common usage scenarios

• feat(messaging,android): save notification payload for messages that arrive in the foreground #3896