Fix potential SecurityException thrown by ConnectivityManager on Android 11 (#2653)

markushi · web-flow · commit 003076de37cb · 2023-04-17T07:37:51.000+02:00
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -13,6 +13,7 @@
 - Fix timestamp intervals of PerformanceCollectionData in profiles ([#2648](https://github.com/getsentry/sentry-java/pull/2648))
 - Fix timestamps of PerformanceCollectionData in profiles ([#2632](https://github.com/getsentry/sentry-java/pull/2632))
 - Fix missing propagateMinConstraints flag for SentryTraced ([#2637](https://github.com/getsentry/sentry-java/pull/2637))
+- Fix potential SecurityException thrown by ConnectivityManager on Android 11 ([#2653](https://github.com/getsentry/sentry-java/pull/2653))
 
 ### Dependencies
 - Bump Kotlin compile version from v1.6.10 to 1.8.0 ([#2563](https://github.com/getsentry/sentry-java/pull/2563))
diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/internal/util/ConnectivityChecker.java b/sentry-android-core/src/main/java/io/sentry/android/core/internal/util/ConnectivityChecker.java
@@ -14,6 +14,11 @@
 import org.jetbrains.annotations.NotNull;
 import org.jetbrains.annotations.Nullable;
 
+/**
+ * Note: ConnectivityManager sometimes throws SecurityExceptions on Android 11. Hence all relevant
+ * calls are guarded with try/catch. see https://issuetracker.google.com/issues/175055271 for more
+ * details
+ */
 @ApiStatus.Internal
 public final class ConnectivityChecker {
 
@@ -62,13 +67,18 @@ private ConnectivityChecker() {}
       logger.log(SentryLevel.INFO, "No permission (ACCESS_NETWORK_STATE) to check network status.");
       return Status.NO_PERMISSION;
     }
-    final android.net.NetworkInfo activeNetworkInfo = connectivityManager.getActiveNetworkInfo();
 
-    if (activeNetworkInfo == null) {
-      logger.log(SentryLevel.INFO, "NetworkInfo is null, there's no active network.");
-      return Status.NOT_CONNECTED;
+    try {
+      final android.net.NetworkInfo activeNetworkInfo = connectivityManager.getActiveNetworkInfo();
+      if (activeNetworkInfo == null) {
+        logger.log(SentryLevel.INFO, "NetworkInfo is null, there's no active network.");
+        return Status.NOT_CONNECTED;
+      }
+      return activeNetworkInfo.isConnected() ? Status.CONNECTED : Status.NOT_CONNECTED;
+    } catch (Throwable t) {
+      logger.log(SentryLevel.ERROR, "Could not retrieve Connection Status", t);
+      return Status.UNKNOWN;
     }
-    return activeNetworkInfo.isConnected() ? Status.CONNECTED : Status.NOT_CONNECTED;
   }
 
   /**
@@ -93,79 +103,86 @@ private ConnectivityChecker() {}
       return null;
     }
 
-    boolean ethernet = false;
-    boolean wifi = false;
-    boolean cellular = false;
+    try {
+      boolean ethernet = false;
+      boolean wifi = false;
+      boolean cellular = false;
 
-    if (buildInfoProvider.getSdkInfoVersion() >= Build.VERSION_CODES.M) {
-      final Network activeNetwork = connectivityManager.getActiveNetwork();
-      if (activeNetwork == null) {
-        logger.log(SentryLevel.INFO, "Network is null and cannot check network status");
-        return null;
-      }
-      final NetworkCapabilities networkCapabilities =
-          connectivityManager.getNetworkCapabilities(activeNetwork);
-      if (networkCapabilities == null) {
-        logger.log(SentryLevel.INFO, "NetworkCapabilities is null and cannot check network type");
-        return null;
-      }
-      if (networkCapabilities.hasTransport(NetworkCapabilities.TRANSPORT_ETHERNET)) {
-        ethernet = true;
-      }
-      if (networkCapabilities.hasTransport(NetworkCapabilities.TRANSPORT_WIFI)) {
-        wifi = true;
-      }
-      if (networkCapabilities.hasTransport(NetworkCapabilities.TRANSPORT_CELLULAR)) {
-        cellular = true;
-      }
-    } else {
-      // ideally using connectivityManager.getAllNetworks(), but its >= Android L only
+      if (buildInfoProvider.getSdkInfoVersion() >= Build.VERSION_CODES.M) {
 
-      // for some reason linting didn't allow a single @SuppressWarnings("deprecation") on method
-      // signature
-      @SuppressWarnings("deprecation")
-      final android.net.NetworkInfo activeNetworkInfo = connectivityManager.getActiveNetworkInfo();
+        final Network activeNetwork = connectivityManager.getActiveNetwork();
+        if (activeNetwork == null) {
+          logger.log(SentryLevel.INFO, "Network is null and cannot check network status");
+          return null;
+        }
+        final NetworkCapabilities networkCapabilities =
+            connectivityManager.getNetworkCapabilities(activeNetwork);
+        if (networkCapabilities == null) {
+          logger.log(SentryLevel.INFO, "NetworkCapabilities is null and cannot check network type");
+          return null;
+        }
+        if (networkCapabilities.hasTransport(NetworkCapabilities.TRANSPORT_ETHERNET)) {
+          ethernet = true;
+        }
+        if (networkCapabilities.hasTransport(NetworkCapabilities.TRANSPORT_WIFI)) {
+          wifi = true;
+        }
+        if (networkCapabilities.hasTransport(NetworkCapabilities.TRANSPORT_CELLULAR)) {
+          cellular = true;
+        }
+      } else {
+        // ideally using connectivityManager.getAllNetworks(), but its >= Android L only
 
-      if (activeNetworkInfo == null) {
-        logger.log(SentryLevel.INFO, "NetworkInfo is null, there's no active network.");
-        return null;
-      }
+        // for some reason linting didn't allow a single @SuppressWarnings("deprecation") on method
+        // signature
+        @SuppressWarnings("deprecation")
+        final android.net.NetworkInfo activeNetworkInfo =
+            connectivityManager.getActiveNetworkInfo();
 
-      @SuppressWarnings("deprecation")
-      final int type = activeNetworkInfo.getType();
+        if (activeNetworkInfo == null) {
+          logger.log(SentryLevel.INFO, "NetworkInfo is null, there's no active network.");
+          return null;
+        }
 
-      @SuppressWarnings("deprecation")
-      final int TYPE_ETHERNET = ConnectivityManager.TYPE_ETHERNET;
+        @SuppressWarnings("deprecation")
+        final int type = activeNetworkInfo.getType();
 
-      @SuppressWarnings("deprecation")
-      final int TYPE_WIFI = ConnectivityManager.TYPE_WIFI;
+        @SuppressWarnings("deprecation")
+        final int TYPE_ETHERNET = ConnectivityManager.TYPE_ETHERNET;
 
-      @SuppressWarnings("deprecation")
-      final int TYPE_MOBILE = ConnectivityManager.TYPE_MOBILE;
+        @SuppressWarnings("deprecation")
+        final int TYPE_WIFI = ConnectivityManager.TYPE_WIFI;
 
-      switch (type) {
-        case TYPE_ETHERNET:
-          ethernet = true;
-          break;
-        case TYPE_WIFI:
-          wifi = true;
-          break;
-        case TYPE_MOBILE:
-          cellular = true;
-          break;
+        @SuppressWarnings("deprecation")
+        final int TYPE_MOBILE = ConnectivityManager.TYPE_MOBILE;
+
+        switch (type) {
+          case TYPE_ETHERNET:
+            ethernet = true;
+            break;
+          case TYPE_WIFI:
+            wifi = true;
+            break;
+          case TYPE_MOBILE:
+            cellular = true;
+            break;
+        }
       }
-    }
 
-    // TODO: change the protocol to be a list of transports as a device may have the capability of
-    // multiple
-    if (ethernet) {
-      return "ethernet";
-    }
-    if (wifi) {
-      return "wifi";
-    }
-    if (cellular) {
-      return "cellular";
+      // TODO: change the protocol to be a list of transports as a device may have the capability of
+      // multiple
+      if (ethernet) {
+        return "ethernet";
+      }
+      if (wifi) {
+        return "wifi";
+      }
+      if (cellular) {
+        return "cellular";
+      }
+
+    } catch (Throwable exception) {
+      logger.log(SentryLevel.ERROR, "Failed to retrieve network info", exception);
     }
 
     return null;
@@ -228,7 +245,12 @@ public static boolean registerNetworkCallback(
       logger.log(SentryLevel.INFO, "No permission (ACCESS_NETWORK_STATE) to check network status.");
       return false;
     }
-    connectivityManager.registerDefaultNetworkCallback(networkCallback);
+    try {
+      connectivityManager.registerDefaultNetworkCallback(networkCallback);
+    } catch (Throwable t) {
+      logger.log(SentryLevel.ERROR, "registerDefaultNetworkCallback failed", t);
+      return false;
+    }
     return true;
   }
 
@@ -245,6 +267,10 @@ public static void unregisterNetworkCallback(
     if (connectivityManager == null) {
       return;
     }
-    connectivityManager.unregisterNetworkCallback(networkCallback);
+    try {
+      connectivityManager.unregisterNetworkCallback(networkCallback);
+    } catch (Throwable t) {
+      logger.log(SentryLevel.ERROR, "unregisterNetworkCallback failed", t);
+    }
   }
 }
diff --git a/sentry-android-core/src/test/java/io/sentry/android/core/ConnectivityCheckerTest.kt b/sentry-android-core/src/test/java/io/sentry/android/core/ConnectivityCheckerTest.kt
@@ -59,7 +59,10 @@ class ConnectivityCheckerTest {
     @Test
     fun `When network is active and connected with permission, return CONNECTED for isConnected`() {
         whenever(networkInfo.isConnected).thenReturn(true)
-        assertEquals(ConnectivityChecker.Status.CONNECTED, ConnectivityChecker.getConnectionStatus(contextMock, mock()))
+        assertEquals(
+            ConnectivityChecker.Status.CONNECTED,
+            ConnectivityChecker.getConnectionStatus(contextMock, mock())
+        )
     }
 
     @Test
@@ -149,30 +152,42 @@ class ConnectivityCheckerTest {
     fun `When network capabilities has TRANSPORT_ETHERNET, return ethernet`() {
         whenever(networkCapabilities.hasTransport(eq(TRANSPORT_ETHERNET))).thenReturn(true)
 
-        assertEquals("ethernet", ConnectivityChecker.getConnectionType(contextMock, mock(), buildInfo))
+        assertEquals(
+            "ethernet",
+            ConnectivityChecker.getConnectionType(contextMock, mock(), buildInfo)
+        )
     }
 
     @Test
     fun `When network is TYPE_ETHERNET, return ethernet`() {
         whenever(buildInfo.sdkInfoVersion).thenReturn(Build.VERSION_CODES.ICE_CREAM_SANDWICH)
         whenever(networkInfo.type).thenReturn(TYPE_ETHERNET)
 
-        assertEquals("ethernet", ConnectivityChecker.getConnectionType(contextMock, mock(), buildInfo))
+        assertEquals(
+            "ethernet",
+            ConnectivityChecker.getConnectionType(contextMock, mock(), buildInfo)
+        )
     }
 
     @Test
     fun `When network capabilities has TRANSPORT_CELLULAR, return cellular`() {
         whenever(networkCapabilities.hasTransport(eq(TRANSPORT_CELLULAR))).thenReturn(true)
 
-        assertEquals("cellular", ConnectivityChecker.getConnectionType(contextMock, mock(), buildInfo))
+        assertEquals(
+            "cellular",
+            ConnectivityChecker.getConnectionType(contextMock, mock(), buildInfo)
+        )
     }
 
     @Test
     fun `When network is TYPE_MOBILE, return cellular`() {
         whenever(buildInfo.sdkInfoVersion).thenReturn(Build.VERSION_CODES.ICE_CREAM_SANDWICH)
         whenever(networkInfo.type).thenReturn(TYPE_MOBILE)
 
-        assertEquals("cellular", ConnectivityChecker.getConnectionType(contextMock, mock(), buildInfo))
+        assertEquals(
+            "cellular",
+            ConnectivityChecker.getConnectionType(contextMock, mock(), buildInfo)
+        )
     }
 
     @Test
@@ -181,7 +196,8 @@ class ConnectivityCheckerTest {
         whenever(buildInfo.sdkInfoVersion).thenReturn(Build.VERSION_CODES.N)
         whenever(contextMock.getSystemService(any())).thenReturn(connectivityManager)
         whenever(contextMock.checkPermission(any(), any(), any())).thenReturn(PERMISSION_DENIED)
-        val registered = ConnectivityChecker.registerNetworkCallback(contextMock, mock(), buildInfo, mock())
+        val registered =
+            ConnectivityChecker.registerNetworkCallback(contextMock, mock(), buildInfo, mock())
 
         assertFalse(registered)
         verify(connectivityManager, never()).registerDefaultNetworkCallback(any())
@@ -190,7 +206,8 @@ class ConnectivityCheckerTest {
     @Test
     fun `When sdkInfoVersion is not min N, do not register any NetworkCallback`() {
         whenever(contextMock.getSystemService(any())).thenReturn(connectivityManager)
-        val registered = ConnectivityChecker.registerNetworkCallback(contextMock, mock(), buildInfo, mock())
+        val registered =
+            ConnectivityChecker.registerNetworkCallback(contextMock, mock(), buildInfo, mock())
 
         assertFalse(registered)
         verify(connectivityManager, never()).registerDefaultNetworkCallback(any())
@@ -201,7 +218,8 @@ class ConnectivityCheckerTest {
         val buildInfo = mock<BuildInfoProvider>()
         whenever(buildInfo.sdkInfoVersion).thenReturn(Build.VERSION_CODES.N)
         whenever(contextMock.getSystemService(any())).thenReturn(connectivityManager)
-        val registered = ConnectivityChecker.registerNetworkCallback(contextMock, mock(), buildInfo, mock())
+        val registered =
+            ConnectivityChecker.registerNetworkCallback(contextMock, mock(), buildInfo, mock())
 
         assertTrue(registered)
         verify(connectivityManager).registerDefaultNetworkCallback(any())
@@ -224,4 +242,51 @@ class ConnectivityCheckerTest {
 
         verify(connectivityManager).unregisterNetworkCallback(any<NetworkCallback>())
     }
+
+    @Test
+    fun `When connectivityManager getActiveNetwork throws an exception, getConnectionType returns null`() {
+        whenever(buildInfo.sdkInfoVersion).thenReturn(Build.VERSION_CODES.S)
+        whenever(connectivityManager.activeNetwork).thenThrow(SecurityException("Android OS Bug"))
+
+        assertNull(ConnectivityChecker.getConnectionType(contextMock, mock(), buildInfo))
+    }
+
+    @Test
+    fun `When connectivityManager getActiveNetworkInfo throws an exception, getConnectionType returns null`() {
+        whenever(buildInfo.sdkInfoVersion).thenReturn(Build.VERSION_CODES.KITKAT)
+        whenever(connectivityManager.activeNetworkInfo).thenThrow(SecurityException("Android OS Bug"))
+
+        assertNull(ConnectivityChecker.getConnectionType(contextMock, mock(), buildInfo))
+        assertEquals(ConnectivityChecker.Status.UNKNOWN, ConnectivityChecker.getConnectionStatus(contextMock, mock()))
+    }
+
+    @Test
+    fun `When connectivityManager registerDefaultCallback throws an exception, false is returned`() {
+        whenever(connectivityManager.registerDefaultNetworkCallback(any())).thenThrow(
+            SecurityException("Android OS Bug")
+        )
+        assertFalse(
+            ConnectivityChecker.registerNetworkCallback(
+                contextMock,
+                mock(),
+                buildInfo,
+                mock()
+            )
+        )
+    }
+
+    @Test
+    fun `When connectivityManager unregisterDefaultCallback throws an exception, it gets swallowed`() {
+        whenever(connectivityManager.registerDefaultNetworkCallback(any())).thenThrow(
+            SecurityException("Android OS Bug")
+        )
+
+        var failed = false
+        try {
+            ConnectivityChecker.unregisterNetworkCallback(contextMock, mock(), buildInfo, mock())
+        } catch (t: Throwable) {
+            failed = true
+        }
+        assertFalse(failed)
+    }
 }
