diff --git a/firebase-crashlytics/CHANGELOG.md b/firebase-crashlytics/CHANGELOG.md index 2c75af37754..3c60a8cd760 100644 --- a/firebase-crashlytics/CHANGELOG.md +++ b/firebase-crashlytics/CHANGELOG.md @@ -1,5 +1,7 @@ # Unreleased +- [changed] Improved rooted device detection to align with OWASP MASTG-KNOW-0027 recommendations [#8099] + # 20.0.6 - [fixed] Fixed race condition that caused logs from background threads to not be attached to diff --git a/firebase-crashlytics/src/androidTest/java/com/google/firebase/crashlytics/internal/common/CommonUtilsTest.java b/firebase-crashlytics/src/androidTest/java/com/google/firebase/crashlytics/internal/common/CommonUtilsTest.java index ce0e71d1049..d9b1aeb45b6 100644 --- a/firebase-crashlytics/src/androidTest/java/com/google/firebase/crashlytics/internal/common/CommonUtilsTest.java +++ b/firebase-crashlytics/src/androidTest/java/com/google/firebase/crashlytics/internal/common/CommonUtilsTest.java @@ -151,9 +151,11 @@ public void testGetTotalRamInBytes() { @Test public void testIsRooted() { + final Context mockContext = mock(Context.class); + when(mockContext.getPackageManager()).thenReturn(mock(PackageManager.class)); // No good way to test the alternate case, // just want to ensure we can complete the call without an exception here. - final boolean isRooted = CommonUtils.isRooted(); + final boolean isRooted = CommonUtils.isRooted(mockContext); Log.d(Logger.TAG, "isRooted: " + isRooted + " isEmulator: " + CommonUtils.isEmulator()); // We don't care about the actual result of isRooted, just that we didn't cause an exception @@ -176,7 +178,10 @@ private boolean isBitSet(int data, int mask) { @Test public void testGetDeviceState() { - final int state = CommonUtils.getDeviceState(); + final Context mockContext = mock(Context.class); + when(mockContext.getPackageManager()).thenReturn(mock(PackageManager.class)); + + final int state = CommonUtils.getDeviceState(mockContext); Log.d(Logger.TAG, "testGetDeviceState: state=" + state); if (CommonUtils.isEmulator()) { @@ -191,7 +196,7 @@ public void testGetDeviceState() { assertFalse(isBitSet(state, CommonUtils.DEVICE_STATE_DEBUGGERATTACHED)); } - if (CommonUtils.isRooted()) { + if (CommonUtils.isRooted(mockContext)) { assertTrue(isBitSet(state, CommonUtils.DEVICE_STATE_JAILBROKEN)); } else { assertFalse(isBitSet(state, CommonUtils.DEVICE_STATE_JAILBROKEN)); diff --git a/firebase-crashlytics/src/main/java/com/google/firebase/crashlytics/internal/common/CommonUtils.java b/firebase-crashlytics/src/main/java/com/google/firebase/crashlytics/internal/common/CommonUtils.java index d76ccbc2133..db11436c2e9 100644 --- a/firebase-crashlytics/src/main/java/com/google/firebase/crashlytics/internal/common/CommonUtils.java +++ b/firebase-crashlytics/src/main/java/com/google/firebase/crashlytics/internal/common/CommonUtils.java @@ -318,34 +318,97 @@ public static boolean isEmulator() { || Build.HARDWARE.contains(RANCHU); } - public static boolean isRooted() { + /** + * Utility method intended for root status validation within a local scope. + *

+ * NOTE: Root detection is complex; compromised devices may spoof results + * to bypass these basic security checks. + * For high-security requirements, integrate solutions like Google Play Integrity API. + *

+ * @return true if any rule is met. + */ + public static boolean isRooted(Context context) { // No reliable way to determine if an android phone is rooted, since a rooted phone could // always disguise itself as non-rooted. Some common approaches can be found on SO: - // http://stackoverflow.com/questions/1101380/determine-if-running-on-a-rooted-device + // http://stackoverflow.com/questions/1101380/determine-if-running-on-a-rooted-device // // http://stackoverflow.com/questions/3576989/how-can-you-detect-if-the-device-is-rooted-in-the-app // // http://stackoverflow.com/questions/7727021/how-can-androids-copy-protection-check-if-the-device-is-rooted + + // Validate custom ROMs. final boolean isEmulator = isEmulator(); final String buildTags = Build.TAGS; if (!isEmulator && buildTags != null && buildTags.contains("test-keys")) { return true; } - // Superuser.apk would only exist on a rooted device: - File file = new File("/system/app/Superuser.apk"); - if (file.exists()) { - return true; + // Check for common Root-Related files and binaries. + String[] paths = { + "/system/app/Superuser.apk", + "/sbin/su", + "/system/bin/su", + "/system/xbin/su", + "/data/local/xbin/su", + "/data/local/bin/su", + "/system/sd/xbin/su", + "/system/bin/failsafe/su", + "/data/local/su", + "/su/bin/su", + "/su/xbin/su", + "/su/bin/daemonsu", + "/system/xbin/daemonsu", + "/system/etc/init.d/99SuperSUDaemon", + "/dev/com.koushikdutta.superuser.daemon/", + "/system/xbin/busybox", + "/data/magisk.img", + "/sbin/.core/img/magisk.img", + "/system/lib/libmagisk.so" + }; + for (String path : paths) { + if (new File(path).exists()) { + return true; + } } - // su is only available on a rooted device (or the emulator) - // The user could rename or move to a non-standard location, but in that case they - // probably don't want us to know they're root and they can pretty much subvert - // any check anyway. - file = new File("/system/xbin/su"); - if (!isEmulator && file.exists()) { - return true; + // Check if 'su' Executable is in the PATH. + String pathVar = System.getenv("PATH"); + if (pathVar != null) { + for (String pathDir : pathVar.split(":")) { + if (new File(pathDir, "su").exists()) { + return true; + } + } } + + // Check for Installed Root Manager packages. + // NOTE: For Android 11+, this requires declared in the app's AndroidManifest.xml. + // See https://developer.android.com/training/package-visibility for more details. + if (context != null) { + String[] knownRootPackages = { + "com.noshufou.android.su", + "com.thirdparty.superuser", + "eu.chainfire.supersu", + "com.koushikdutta.superuser", + "com.zachspong.repodroid", + "com.ramdroid.repodroid", + "com.topjohnwu.magisk" + }; + PackageManager pm = context.getPackageManager(); + for (String pkg : knownRootPackages) { + try { + pm.getPackageInfo(pkg, 0); + return true; + } catch (PackageManager.NameNotFoundException e) { + // Package is not installed or not visible. + Logger.getLogger() + .d( + "Root check failed due to missing package or limited visibility. Further details: " + + e.getMessage()); + } + } + } + return false; } @@ -361,13 +424,13 @@ public static boolean isDebuggerAttached() { public static final int DEVICE_STATE_VENDORINTERNAL = 1 << 4; public static final int DEVICE_STATE_COMPROMISEDLIBRARIES = 1 << 5; - public static int getDeviceState() { + public static int getDeviceState(Context context) { int deviceState = 0; if (CommonUtils.isEmulator()) { deviceState |= DEVICE_STATE_ISSIMULATOR; } - if (CommonUtils.isRooted()) { + if (CommonUtils.isRooted(context)) { deviceState |= DEVICE_STATE_JAILBROKEN; } diff --git a/firebase-crashlytics/src/main/java/com/google/firebase/crashlytics/internal/common/CrashlyticsController.java b/firebase-crashlytics/src/main/java/com/google/firebase/crashlytics/internal/common/CrashlyticsController.java index b464819e18c..50d80f6661f 100644 --- a/firebase-crashlytics/src/main/java/com/google/firebase/crashlytics/internal/common/CrashlyticsController.java +++ b/firebase-crashlytics/src/main/java/com/google/firebase/crashlytics/internal/common/CrashlyticsController.java @@ -542,7 +542,7 @@ private void doOpenSession(String sessionIdentifier, Boolean isOnDemand) { String.format(Locale.US, GENERATOR_FORMAT, CrashlyticsCore.getVersion()); StaticSessionData.AppData appData = createAppData(idManager, this.appData); - StaticSessionData.OsData osData = createOsData(); + StaticSessionData.OsData osData = createOsData(context); StaticSessionData.DeviceData deviceData = createDeviceData(context); nativeComponent.prepareNativeSession( @@ -765,9 +765,9 @@ private static StaticSessionData.AppData createAppData(IdManager idManager, AppD appData.developmentPlatformProvider); } - private static StaticSessionData.OsData createOsData() { + private static StaticSessionData.OsData createOsData(Context context) { return StaticSessionData.OsData.create( - VERSION.RELEASE, VERSION.CODENAME, CommonUtils.isRooted()); + VERSION.RELEASE, VERSION.CODENAME, CommonUtils.isRooted(context)); } private static StaticSessionData.DeviceData createDeviceData(Context context) { @@ -781,7 +781,7 @@ private static StaticSessionData.DeviceData createDeviceData(Context context) { CommonUtils.calculateTotalRamInBytes(context), diskSpace, CommonUtils.isEmulator(), - CommonUtils.getDeviceState(), + CommonUtils.getDeviceState(context), Build.MANUFACTURER, Build.PRODUCT); } diff --git a/firebase-crashlytics/src/main/java/com/google/firebase/crashlytics/internal/common/CrashlyticsReportDataCapture.java b/firebase-crashlytics/src/main/java/com/google/firebase/crashlytics/internal/common/CrashlyticsReportDataCapture.java index 8fbb3107f63..300dcb7245a 100644 --- a/firebase-crashlytics/src/main/java/com/google/firebase/crashlytics/internal/common/CrashlyticsReportDataCapture.java +++ b/firebase-crashlytics/src/main/java/com/google/firebase/crashlytics/internal/common/CrashlyticsReportDataCapture.java @@ -208,7 +208,7 @@ private CrashlyticsReport.Session.OperatingSystem populateSessionOperatingSystem .setPlatform(SESSION_ANDROID_PLATFORM) .setVersion(VERSION.RELEASE) .setBuildVersion(VERSION.CODENAME) - .setJailbroken(CommonUtils.isRooted()) + .setJailbroken(CommonUtils.isRooted(context)) .build(); } @@ -219,7 +219,7 @@ private CrashlyticsReport.Session.Device populateSessionDeviceData() { final long totalRam = CommonUtils.calculateTotalRamInBytes(context); final long diskSpace = (long) statFs.getBlockCount() * (long) statFs.getBlockSize(); final boolean isEmulator = CommonUtils.isEmulator(); - final int state = CommonUtils.getDeviceState(); + final int state = CommonUtils.getDeviceState(context); final String manufacturer = Build.MANUFACTURER; final String modelClass = Build.PRODUCT;