diff --git a/CHANGELOG.md b/CHANGELOG.md index d85c9c48e9..7da03e5d7b 100644 --- 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 index f37f72b0da..7c6fa096c1 100644 --- 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 index 850464d20a..791a26cd89 100644 --- 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/sentry-android-core/src/main/java/io/sentry/android/core/internal/util/ContentProviderSecurityChecker.java b/sentry-android-core/src/main/java/io/sentry/android/core/internal/util/ContentProviderSecurityChecker.java new file mode 100644 index 0000000000..8420f68564 --- /dev/null +++ b/sentry-android-core/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). + * + *

Throws a SecurityException if the security check is breached. + * + *

See https://www.cvedetails.com/cve/CVE-2018-9492/ and + * https://github.com/getsentry/sentry-java/issues/2460 + * + *

Call this function in the {@link ContentProvider#query(Uri, String[], String, String[], + * String)} function. + * + *

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. + * + *

This blocks the attacker by only allowing the app itself (not other apps) to "query" the + * provider. + * + *

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/sentry-android-core/src/test/java/io/sentry/android/core/internal/util/ContentProviderSecurityCheckerTest.kt b/sentry-android-core/src/test/java/io/sentry/android/core/internal/util/ContentProviderSecurityCheckerTest.kt new file mode 100644 index 0000000000..58b0d7ebe6 --- /dev/null +++ b/sentry-android-core/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() + + 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() + + 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() + + 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.mockPackages(null) + + assertFailsWith { + fixture.getSut().checkPrivilegeEscalation(contentProvider) + } + } + + @Test + fun `When calling package does not match app package, security check exception is thrown`() { + val contentProvider = mock() + + contentProvider.mockPackages("{$APP_PACKAGE}.attacker") + + assertFailsWith { + fixture.getSut().checkPrivilegeEscalation(contentProvider) + } + } + + @Test + fun `When calling package matches app package, no security exception thrown`() { + val contentProvider = mock() + + contentProvider.mockPackages(APP_PACKAGE) + + fixture.getSut().checkPrivilegeEscalation(contentProvider) + + // No exception! + } +} + +private fun ContentProvider.mockPackages(callingPackage: String?) { + whenever(this.callingPackage).thenReturn(callingPackage) + + val context = mock() + whenever(this.context).thenReturn(context) + whenever(context.packageName).thenReturn(APP_PACKAGE) +} + +private const val APP_PACKAGE = "com.app"