diff --git a/CHANGELOG.md b/CHANGELOG.md index 9d11457338..44fb097390 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,12 @@ # Changelog +## TBD + +### Enhancements + +* Improve detection of rooted devices + [#1194](https://github.com/bugsnag/bugsnag-android/pull/1194) + ## 5.7.1 (2021-03-03) ### Bug fixes diff --git a/bugsnag-android-core/detekt-baseline.xml b/bugsnag-android-core/detekt-baseline.xml index bacb5d2566..2febaf5e7e 100644 --- a/bugsnag-android-core/detekt-baseline.xml +++ b/bugsnag-android-core/detekt-baseline.xml @@ -7,15 +7,13 @@ LongParameterList:AppWithState.kt$AppWithState$( config: ImmutableConfig, binaryArch: String?, id: String?, releaseStage: String?, version: String?, codeBundleId: String?, duration: Number?, durationInForeground: Number?, inForeground: Boolean? ) LongParameterList:Device.kt$Device$( buildInfo: DeviceBuildInfo, /** * The Application Binary Interface used */ var cpuAbi: Array<String>?, /** * Whether the device has been jailbroken */ var jailbroken: Boolean?, /** * A UUID generated by Bugsnag and used for the individual application on a device */ var id: String?, /** * The IETF language tag of the locale used */ var locale: String?, /** * The total number of bytes of memory on the device */ var totalMemory: Long?, /** * A collection of names and their versions of the primary languages, frameworks or * runtimes that the application is running on */ var runtimeVersions: MutableMap<String, Any>? ) LongParameterList:DeviceBuildInfo.kt$DeviceBuildInfo$( val manufacturer: String?, val model: String?, val osVersion: String?, val apiLevel: Int?, val osBuild: String?, val fingerprint: String?, val tags: String?, val brand: String?, val cpuAbis: Array<String>? ) - LongParameterList:DeviceDataCollector.kt$DeviceDataCollector$( private val connectivity: Connectivity, private val appContext: Context, private val resources: Resources?, private val deviceId: String?, private val buildInfo: DeviceBuildInfo, private val dataDirectory: File, private val logger: Logger ) + LongParameterList:DeviceDataCollector.kt$DeviceDataCollector$( private val connectivity: Connectivity, private val appContext: Context, private val resources: Resources?, private val deviceId: String?, private val buildInfo: DeviceBuildInfo, private val dataDirectory: File, private val rootDetector: RootDetector, private val logger: Logger ) LongParameterList:DeviceWithState.kt$DeviceWithState$( buildInfo: DeviceBuildInfo, jailbroken: Boolean?, id: String?, locale: String?, totalMemory: Long?, runtimeVersions: MutableMap<String, Any>, /** * The number of free bytes of storage available on the device */ var freeDisk: Long?, /** * The number of free bytes of memory available on the device */ var freeMemory: Long?, /** * The orientation of the device when the event occurred: either portrait or landscape */ var orientation: String?, /** * The timestamp on the device when the event occurred */ var time: Date? ) - LongParameterList:NativeStackframe.kt$NativeStackframe$( /** * The name of the method that was being executed */ var method: String?, /** * The location of the source file */ var file: String?, /** * The line number within the source file this stackframe refers to */ var lineNumber: Number?, /** * The address of the instruction where the event occurred. */ var frameAddress: Long?, /** * The address of the function where the event occurred. */ var symbolAddress: Long?, /** * The address of the library where the event occurred. */ var loadAddress: Long?, /** * The type of the error */ var type: ErrorType = ErrorType.C ) LongParameterList:ThreadState.kt$ThreadState$( exc: Throwable?, isUnhandled: Boolean, sendThreads: ThreadSendPolicy, projectPackages: Collection<String>, logger: Logger, currentThread: java.lang.Thread = java.lang.Thread.currentThread(), stackTraces: MutableMap<java.lang.Thread, Array<StackTraceElement>> = java.lang.Thread.getAllStackTraces() ) LongParameterList:ThreadState.kt$ThreadState$( stackTraces: MutableMap<java.lang.Thread, Array<StackTraceElement>>, currentThread: java.lang.Thread, exc: Throwable?, isUnhandled: Boolean, projectPackages: Collection<String>, logger: Logger ) MagicNumber:DefaultDelivery.kt$DefaultDelivery$299 MagicNumber:DefaultDelivery.kt$DefaultDelivery$429 MagicNumber:DefaultDelivery.kt$DefaultDelivery$499 - MatchingDeclarationName:Breadcrumb.kt$BreadcrumbInternal : Streamable ProtectedMemberInFinalClass:ConfigInternal.kt$ConfigInternal$protected val plugins = mutableSetOf<Plugin>() ProtectedMemberInFinalClass:EventInternal.kt$EventInternal$protected fun isAnr(event: Event): Boolean ProtectedMemberInFinalClass:EventInternal.kt$EventInternal$protected fun shouldDiscardClass(): Boolean diff --git a/bugsnag-android-core/src/main/java/com/bugsnag/android/Client.java b/bugsnag-android-core/src/main/java/com/bugsnag/android/Client.java index 2a61cf5124..9060b98d02 100644 --- a/bugsnag-android-core/src/main/java/com/bugsnag/android/Client.java +++ b/bugsnag-android-core/src/main/java/com/bugsnag/android/Client.java @@ -163,8 +163,8 @@ public Unit invoke(Boolean hasConnection, String networkState) { DeviceBuildInfo info = DeviceBuildInfo.Companion.defaultInfo(); Resources resources = appContext.getResources(); - deviceDataCollector = new DeviceDataCollector(connectivity, appContext, - resources, deviceId, info, Environment.getDataDirectory(), logger); + deviceDataCollector = new DeviceDataCollector(connectivity, appContext, resources, + deviceId, info, Environment.getDataDirectory(), new RootDetector(), logger); if (appContext instanceof Application) { Application application = (Application) appContext; diff --git a/bugsnag-android-core/src/main/java/com/bugsnag/android/DeviceDataCollector.kt b/bugsnag-android-core/src/main/java/com/bugsnag/android/DeviceDataCollector.kt index fdd2b003d5..ea93d7b678 100644 --- a/bugsnag-android-core/src/main/java/com/bugsnag/android/DeviceDataCollector.kt +++ b/bugsnag-android-core/src/main/java/com/bugsnag/android/DeviceDataCollector.kt @@ -23,6 +23,7 @@ internal class DeviceDataCollector( private val deviceId: String?, private val buildInfo: DeviceBuildInfo, private val dataDirectory: File, + private val rootDetector: RootDetector, private val logger: Logger ) { @@ -84,6 +85,9 @@ internal class DeviceDataCollector( * Check if the current Android device is rooted */ private fun isRooted(): Boolean { + if (rootDetector.isRooted()) { + return true + } val tags = buildInfo.tags if (tags != null && tags.contains("test-keys")) { return true diff --git a/bugsnag-android-core/src/main/java/com/bugsnag/android/RootDetector.kt b/bugsnag-android-core/src/main/java/com/bugsnag/android/RootDetector.kt new file mode 100644 index 0000000000..d488b9ff95 --- /dev/null +++ b/bugsnag-android-core/src/main/java/com/bugsnag/android/RootDetector.kt @@ -0,0 +1,41 @@ +package com.bugsnag.android + +import androidx.annotation.VisibleForTesting +import java.io.BufferedReader +import java.io.IOException + +/** + * Attempts to detect whether the device is rooted. Root detection errs on the side of false + * negatives rather than false positives. + * + * This class will only give a reasonable indication that a device has been rooted - as it's + * possible to manipulate Java return values & native library loading, it will always be possible + * for a determined application to defeat these root checks. + */ +internal class RootDetector { + + fun isRooted() = checkSuExists() + + /** + * Checks whether the su binary exists by running `which su`. A non-empty result + * indicates that the binary is present, which is a good indicator that the device + * may have been rooted. + */ + fun checkSuExists(): Boolean = checkSuExists(ProcessBuilder()) + + @VisibleForTesting + internal fun checkSuExists(processBuilder: ProcessBuilder): Boolean { + processBuilder.command(listOf("which", "su")) + + var process: Process? = null + return try { + process = processBuilder.start() + val output = process.inputStream.bufferedReader().use(BufferedReader::readText) + output.isNotBlank() + } catch (ignored: IOException) { + false + } finally { + process?.destroy() + } + } +} diff --git a/bugsnag-android-core/src/test/java/com/bugsnag/android/DeviceDataCollectorSerializationTest.kt b/bugsnag-android-core/src/test/java/com/bugsnag/android/DeviceDataCollectorSerializationTest.kt index e2bee24b65..6e7df04595 100644 --- a/bugsnag-android-core/src/test/java/com/bugsnag/android/DeviceDataCollectorSerializationTest.kt +++ b/bugsnag-android-core/src/test/java/com/bugsnag/android/DeviceDataCollectorSerializationTest.kt @@ -27,6 +27,7 @@ internal class DeviceDataCollectorSerializationTest { val res = mock(Resources::class.java) val conf = mock(Configuration::class.java) val connectivity = mock(Connectivity::class.java) + val rootDetector = mock(RootDetector::class.java) val prefs = mock(SharedPreferences::class.java) val editor = mock(SharedPreferences.Editor::class.java) @@ -46,6 +47,7 @@ internal class DeviceDataCollectorSerializationTest { metrics.densityDpi = 120 `when`(res.displayMetrics).thenReturn(metrics) `when`(connectivity.retrieveNetworkAccessState()).thenReturn("unknown") + `when`(rootDetector.isRooted()).thenReturn(false) // construct devicedata object val deviceData = DeviceDataCollector( @@ -55,6 +57,7 @@ internal class DeviceDataCollectorSerializationTest { "123", buildInfo, File(""), + rootDetector, NoopLogger ) diff --git a/bugsnag-android-core/src/test/java/com/bugsnag/android/DeviceMetadataSerializationTest.kt b/bugsnag-android-core/src/test/java/com/bugsnag/android/DeviceMetadataSerializationTest.kt index e2f71acf2b..8f525a4d7f 100644 --- a/bugsnag-android-core/src/test/java/com/bugsnag/android/DeviceMetadataSerializationTest.kt +++ b/bugsnag-android-core/src/test/java/com/bugsnag/android/DeviceMetadataSerializationTest.kt @@ -55,6 +55,7 @@ internal class DeviceMetadataSerializationTest { "123", buildInfo, File(""), + RootDetector(), NoopLogger ) diff --git a/bugsnag-android-core/src/test/java/com/bugsnag/android/RootDetectorTest.kt b/bugsnag-android-core/src/test/java/com/bugsnag/android/RootDetectorTest.kt new file mode 100644 index 0000000000..dd1d8ec920 --- /dev/null +++ b/bugsnag-android-core/src/test/java/com/bugsnag/android/RootDetectorTest.kt @@ -0,0 +1,62 @@ +package com.bugsnag.android + +import org.junit.Assert.assertFalse +import org.junit.Assert.assertTrue +import org.junit.Before +import org.junit.Test +import org.junit.runner.RunWith +import org.mockito.Mock +import org.mockito.Mockito.`when` +import org.mockito.Mockito.times +import org.mockito.Mockito.verify +import org.mockito.junit.MockitoJUnitRunner +import java.io.ByteArrayInputStream +import java.io.IOException + +@RunWith(MockitoJUnitRunner::class) +class RootDetectorTest { + + private val rootDetector = RootDetector() + + @Mock + lateinit var processBuilder: ProcessBuilder + + @Mock + lateinit var process: Process + + @Before + fun setUp() { + `when`(processBuilder.start()).thenReturn(process) + } + + /** + * IOExceptions thrown when starting the process are handled appropriately + */ + @Test + fun checkSuProcessStartException() { + `when`(processBuilder.start()).thenThrow(IOException()) + assertFalse(rootDetector.checkSuExists(processBuilder)) + } + + /** + * The method returns false if 'which su' returns an empty string + */ + @Test + fun checkSuNotFound() { + val emptyStream = ByteArrayInputStream("".toByteArray()) + `when`(process.inputStream).thenReturn(emptyStream) + assertFalse(rootDetector.checkSuExists(processBuilder)) + verify(processBuilder, times(1)).command(listOf("which", "su")) + verify(process, times(1)).destroy() + } + + /** + * The method returns true if 'which su' returns a non-empty string + */ + @Test + fun checkSuFound() { + val resultStream = ByteArrayInputStream("/system/bin/su".toByteArray()) + `when`(process.inputStream).thenReturn(resultStream) + assertTrue(rootDetector.checkSuExists(processBuilder)) + } +}