Fix potential Privilege Escalation via Content Provider (CVE-2018-9492)…

… (#2466) Co-authored-by: Markus Hintersteiner <[email protected]> close undefined
getsentry · Jan 16, 2023 · 2374884 · 2374884
1 parent 068ace3
commit 2374884
Show file tree

Hide file tree

Showing 5 changed files with 154 additions and 0 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -14,6 +14,7 @@
 - Use a single TransactionPerfomanceCollector ([#2464](https://github.com/getsentry/sentry-java/pull/2464))
 - Don't override sdk name with Timber ([#2450](https://github.com/getsentry/sentry-java/pull/2450))
 - Set transactionNameSource to CUSTOM when setting transaction name ([#2405](https://github.com/getsentry/sentry-java/pull/2405))
+- Guard against CVE-2018-9492 "Privilege Escalation via Content Provider" ([#2466](https://github.com/getsentry/sentry-java/pull/2466))
 
 ## 6.11.0
 

diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/SentryInitProvider.java b/sentry-android-core/src/main/java/io/sentry/android/core/SentryInitProvider.java
@@ -8,6 +8,7 @@
 import android.net.Uri;
 import io.sentry.Sentry;
 import io.sentry.SentryLevel;
+import io.sentry.android.core.internal.util.ContentProviderSecurityChecker;
 import org.jetbrains.annotations.ApiStatus;
 import org.jetbrains.annotations.NotNull;
 import org.jetbrains.annotations.Nullable;
@@ -51,6 +52,7 @@ public void attachInfo(@NotNull Context context, @NotNull ProviderInfo info) {
       @Nullable String s,
       @Nullable String[] strings1,
       @Nullable String s1) {
+    new ContentProviderSecurityChecker().checkPrivilegeEscalation(this);
     return null;
   }
 

diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/SentryPerformanceProvider.java b/sentry-android-core/src/main/java/io/sentry/android/core/SentryPerformanceProvider.java
@@ -11,6 +11,7 @@
 import android.os.Bundle;
 import android.os.SystemClock;
 import io.sentry.DateUtils;
+import io.sentry.android.core.internal.util.ContentProviderSecurityChecker;
 import java.util.Date;
 import org.jetbrains.annotations.ApiStatus;
 import org.jetbrains.annotations.NotNull;
@@ -78,6 +79,7 @@ public Cursor query(
       @Nullable String selection,
       @Nullable String[] selectionArgs,
       @Nullable String sortOrder) {
+    new ContentProviderSecurityChecker().checkPrivilegeEscalation(this);
     return null;
   }
 

diff --git a/...re/src/main/java/io/sentry/android/core/internal/util/ContentProviderSecurityChecker.java b/...re/src/main/java/io/sentry/android/core/internal/util/ContentProviderSecurityChecker.java
@@ -0,0 +1,58 @@
+package io.sentry.android.core.internal.util;
+
+import android.annotation.SuppressLint;
+import android.content.ContentProvider;
+import android.net.Uri;
+import android.os.Build;
+import io.sentry.NoOpLogger;
+import io.sentry.android.core.BuildInfoProvider;
+import org.jetbrains.annotations.ApiStatus;
+import org.jetbrains.annotations.NotNull;
+
+@ApiStatus.Internal
+public final class ContentProviderSecurityChecker {
+
+  private final @NotNull BuildInfoProvider buildInfoProvider;
+
+  public ContentProviderSecurityChecker() {
+    this(new BuildInfoProvider(NoOpLogger.getInstance()));
+  }
+
+  public ContentProviderSecurityChecker(final @NotNull BuildInfoProvider buildInfoProvider) {
+    this.buildInfoProvider = buildInfoProvider;
+  }
+
+  /**
+   * Protects against "Privilege Escalation via Content Provider" (CVE-2018-9492).
+   *
+   * <p>Throws a SecurityException if the security check is breached.
+   *
+   * <p>See https://www.cvedetails.com/cve/CVE-2018-9492/ and
+   * https://github.com/getsentry/sentry-java/issues/2460
+   *
+   * <p>Call this function in the {@link ContentProvider#query(Uri, String[], String, String[],
+   * String)} function.
+   *
+   * <p>This should be invoked regardless of whether there is data to query or not. The attack is
+   * not contained to the specific provider but rather the entire system.
+   *
+   * <p>This blocks the attacker by only allowing the app itself (not other apps) to "query" the
+   * provider.
+   *
+   * <p>The vulnerability is specific to un-patched versions of Android 8 and 9 (API 26 to 28).
+   * Therefore, this security check is limited to those versions to mitigate risk of regression.
+   */
+  @SuppressLint("NewApi")
+  public void checkPrivilegeEscalation(ContentProvider contentProvider) {
+    final int sdkVersion = buildInfoProvider.getSdkInfoVersion();
+    if (sdkVersion >= Build.VERSION_CODES.O && sdkVersion <= Build.VERSION_CODES.P) {
+
+      String callingPackage = contentProvider.getCallingPackage();
+      String appPackage = contentProvider.getContext().getPackageName();
+      if (callingPackage != null && callingPackage.equals(appPackage)) {
+        return;
+      }
+      throw new SecurityException("Provider does not allow for granting of Uri permissions");
+    }
+  }
+}
diff --git a/.../src/test/java/io/sentry/android/core/internal/util/ContentProviderSecurityCheckerTest.kt b/.../src/test/java/io/sentry/android/core/internal/util/ContentProviderSecurityCheckerTest.kt
@@ -0,0 +1,91 @@
+package io.sentry.android.core.internal.util
+
+import android.content.ContentProvider
+import android.content.Context
+import android.os.Build
+import io.sentry.android.core.BuildInfoProvider
+import org.mockito.kotlin.mock
+import org.mockito.kotlin.verifyNoInteractions
+import org.mockito.kotlin.whenever
+import kotlin.test.Test
+import kotlin.test.assertFailsWith
+
+class ContentProviderSecurityCheckerTest {
+
+    private class Fixture {
+        val buildInfoProvider = mock<BuildInfoProvider>()
+
+        fun getSut(
+            sdkVersion: Int = Build.VERSION_CODES.O
+        ): ContentProviderSecurityChecker {
+            whenever(buildInfoProvider.sdkInfoVersion).thenReturn(sdkVersion)
+
+            return ContentProviderSecurityChecker(
+                buildInfoProvider
+            )
+        }
+    }
+
+    private val fixture = Fixture()
+
+    @Test
+    fun `When sdk version is less than vulnerable versions, security check is not performed`() {
+        val contentProvider = mock<ContentProvider>()
+
+        fixture.getSut(Build.VERSION_CODES.N_MR1).checkPrivilegeEscalation(contentProvider)
+
+        verifyNoInteractions(contentProvider)
+    }
+
+    @Test
+    fun `When sdk version is greater than vulnerable versions, security check is not performed`() {
+        val contentProvider = mock<ContentProvider>()
+
+        fixture.getSut(Build.VERSION_CODES.Q).checkPrivilegeEscalation(contentProvider)
+
+        verifyNoInteractions(contentProvider)
+    }
+
+    @Test
+    fun `When calling package is null, security check exception is thrown`() {
+        val contentProvider = mock<ContentProvider>()
+
+        contentProvider.mockPackages(null)
+
+        assertFailsWith<SecurityException> {
+            fixture.getSut().checkPrivilegeEscalation(contentProvider)
+        }
+    }
+
+    @Test
+    fun `When calling package does not match app package, security check exception is thrown`() {
+        val contentProvider = mock<ContentProvider>()
+
+        contentProvider.mockPackages("{$APP_PACKAGE}.attacker")
+
+        assertFailsWith<SecurityException> {
+            fixture.getSut().checkPrivilegeEscalation(contentProvider)
+        }
+    }
+
+    @Test
+    fun `When calling package matches app package, no security exception thrown`() {
+        val contentProvider = mock<ContentProvider>()
+
+        contentProvider.mockPackages(APP_PACKAGE)
+
+        fixture.getSut().checkPrivilegeEscalation(contentProvider)
+
+        // No exception!
+    }
+}
+
+private fun ContentProvider.mockPackages(callingPackage: String?) {
+    whenever(this.callingPackage).thenReturn(callingPackage)
+
+    val context = mock<Context>()
+    whenever(this.context).thenReturn(context)
+    whenever(context.packageName).thenReturn(APP_PACKAGE)
+}
+
+private const val APP_PACKAGE = "com.app"