From 3f4c9b920e9ea507273b89fe4e11b52b310d2e7f Mon Sep 17 00:00:00 2001 From: Vandolf Estrellado Date: Wed, 18 Jan 2023 08:36:30 -1000 Subject: [PATCH 1/3] Expand fix for potential Privilege Escalation via Content Provider (CVE-2018-9492) --- .../core/EmptySecureContentProvider.java | 56 +++++++++++++++++++ .../android/core/SentryInitProvider.java | 36 +----------- .../core/SentryPerformanceProvider.java | 39 +------------ .../util/ContentProviderSecurityChecker.java | 16 +++--- 4 files changed, 66 insertions(+), 81 deletions(-) create mode 100644 sentry-android-core/src/main/java/io/sentry/android/core/EmptySecureContentProvider.java diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/EmptySecureContentProvider.java b/sentry-android-core/src/main/java/io/sentry/android/core/EmptySecureContentProvider.java new file mode 100644 index 0000000000..ff8f2b4e98 --- /dev/null +++ b/sentry-android-core/src/main/java/io/sentry/android/core/EmptySecureContentProvider.java @@ -0,0 +1,56 @@ +package io.sentry.android.core; + +import android.content.ContentProvider; +import android.content.ContentValues; +import android.database.Cursor; +import android.net.Uri; +import io.sentry.android.core.internal.util.ContentProviderSecurityChecker; +import org.jetbrains.annotations.ApiStatus; +import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.Nullable; + +/** + * A ContentProvider that does NOT store or provide any data for read or write operations. + * + *

This does not allow for overriding the abstract query, insert, update, and delete operations + * of the {@link ContentProvider}. Additionally, those functions are secure. + */ +@ApiStatus.Internal +abstract class EmptySecureContentProvider extends ContentProvider { + + private final ContentProviderSecurityChecker securityChecker = + new ContentProviderSecurityChecker(); + + @Override + public final @Nullable Cursor query( + @NotNull Uri uri, + @Nullable String[] strings, + @Nullable String s, + @Nullable String[] strings1, + @Nullable String s1) { + securityChecker.checkPrivilegeEscalation(this); + return null; + } + + @Override + public final @Nullable Uri insert(@NotNull Uri uri, @Nullable ContentValues contentValues) { + securityChecker.checkPrivilegeEscalation(this); + return null; + } + + @Override + public final int delete(@NotNull Uri uri, @Nullable String s, @Nullable String[] strings) { + securityChecker.checkPrivilegeEscalation(this); + return 0; + } + + @Override + public final int update( + @NotNull Uri uri, + @Nullable ContentValues contentValues, + @Nullable String s, + @Nullable String[] strings) { + securityChecker.checkPrivilegeEscalation(this); + return 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 7c6fa096c1..9e0460f812 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 @@ -1,20 +1,16 @@ package io.sentry.android.core; -import android.content.ContentProvider; -import android.content.ContentValues; import android.content.Context; import android.content.pm.ProviderInfo; -import android.database.Cursor; 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; @ApiStatus.Internal -public final class SentryInitProvider extends ContentProvider { +public final class SentryInitProvider extends EmptySecureContentProvider { @Override public boolean onCreate() { @@ -45,38 +41,8 @@ public void attachInfo(@NotNull Context context, @NotNull ProviderInfo info) { super.attachInfo(context, info); } - @Override - public @Nullable Cursor query( - @NotNull Uri uri, - @Nullable String[] strings, - @Nullable String s, - @Nullable String[] strings1, - @Nullable String s1) { - new ContentProviderSecurityChecker().checkPrivilegeEscalation(this); - return null; - } - @Override public @Nullable String getType(@NotNull Uri uri) { return null; } - - @Override - public @Nullable Uri insert(@NotNull Uri uri, @Nullable ContentValues contentValues) { - return null; - } - - @Override - public int delete(@NotNull Uri uri, @Nullable String s, @Nullable String[] strings) { - return 0; - } - - @Override - public int update( - @NotNull Uri uri, - @Nullable ContentValues contentValues, - @Nullable String s, - @Nullable String[] strings) { - return 0; - } } 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 791a26cd89..2b82f5353f 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 @@ -2,16 +2,12 @@ import android.app.Activity; import android.app.Application; -import android.content.ContentProvider; -import android.content.ContentValues; import android.content.Context; import android.content.pm.ProviderInfo; -import android.database.Cursor; import android.net.Uri; 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; @@ -25,7 +21,7 @@ * AppComponentFactory but it depends on androidx.core.app.AppComponentFactory */ @ApiStatus.Internal -public final class SentryPerformanceProvider extends ContentProvider +public final class SentryPerformanceProvider extends EmptySecureContentProvider implements Application.ActivityLifecycleCallbacks { // static to rely on Class load @@ -71,45 +67,12 @@ public void attachInfo(Context context, ProviderInfo info) { super.attachInfo(context, info); } - @Nullable - @Override - public Cursor query( - @NotNull Uri uri, - @Nullable String[] projection, - @Nullable String selection, - @Nullable String[] selectionArgs, - @Nullable String sortOrder) { - new ContentProviderSecurityChecker().checkPrivilegeEscalation(this); - return null; - } - @Nullable @Override public String getType(@NotNull Uri uri) { return null; } - @Nullable - @Override - public Uri insert(@NotNull Uri uri, @Nullable ContentValues values) { - return null; - } - - @Override - public int delete( - @NotNull Uri uri, @Nullable String selection, @Nullable String[] selectionArgs) { - return 0; - } - - @Override - public int update( - @NotNull Uri uri, - @Nullable ContentValues values, - @Nullable String selection, - @Nullable String[] selectionArgs) { - return 0; - } - @TestOnly static void setAppStartTime(final long appStartMillisLong, final @NotNull Date appStartTimeDate) { appStartMillis = appStartMillisLong; 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 index 8420f68564..9aa21507f7 100644 --- 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 @@ -2,7 +2,6 @@ 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; @@ -30,20 +29,21 @@ public ContentProviderSecurityChecker(final @NotNull BuildInfoProvider buildInfo *

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

Call this function in the {@link ContentProvider}'s implementations of the abstract + * functions; query, insert, update, and delete. * - *

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 should be invoked regardless of whether there is data to read/write 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. + *

This blocks the attacker by only allowing the app itself (not other apps) to interact with + * the ContentProvider. If the ContentProvider needs to be able to interact with other trusted + * apps, then this function or class should be refactored to accommodate that. * *

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) { + public void checkPrivilegeEscalation(@NotNull ContentProvider contentProvider) { final int sdkVersion = buildInfoProvider.getSdkInfoVersion(); if (sdkVersion >= Build.VERSION_CODES.O && sdkVersion <= Build.VERSION_CODES.P) { From 57648df745ad1094412f8bda9b5df1a3f1d2d5d0 Mon Sep 17 00:00:00 2001 From: Vandolf Estrellado Date: Wed, 18 Jan 2023 10:10:56 -1000 Subject: [PATCH 2/3] Added unreleased notes to CHANGELOG --- CHANGELOG.md | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7d1bae48bb..f9eab70c9a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,11 @@ # Changelog +## Unreleased + +### Fixes + +- Expand guard against CVE-2018-9492 "Privilege Escalation via Content Provider" ([#2482](https://github.com/getsentry/sentry-java/pull/2482)) + ## 6.12.1 ### Fixes From a56578ce976b7270a4808dde83d33de22531be32 Mon Sep 17 00:00:00 2001 From: Vandolf Estrellado Date: Wed, 18 Jan 2023 10:21:19 -1000 Subject: [PATCH 3/3] Ran apiDump --- sentry-android-core/api/sentry-android-core.api | 12 ++---------- 1 file changed, 2 insertions(+), 10 deletions(-) diff --git a/sentry-android-core/api/sentry-android-core.api b/sentry-android-core/api/sentry-android-core.api index 908e8da030..64dd993f54 100644 --- a/sentry-android-core/api/sentry-android-core.api +++ b/sentry-android-core/api/sentry-android-core.api @@ -189,24 +189,18 @@ public final class io/sentry/android/core/SentryAndroidOptions : io/sentry/Sentr public fun setProfilingTracesIntervalMillis (I)V } -public final class io/sentry/android/core/SentryInitProvider : android/content/ContentProvider { +public final class io/sentry/android/core/SentryInitProvider { public fun ()V public fun attachInfo (Landroid/content/Context;Landroid/content/pm/ProviderInfo;)V - public fun delete (Landroid/net/Uri;Ljava/lang/String;[Ljava/lang/String;)I public fun getType (Landroid/net/Uri;)Ljava/lang/String; - public fun insert (Landroid/net/Uri;Landroid/content/ContentValues;)Landroid/net/Uri; public fun onCreate ()Z - public fun query (Landroid/net/Uri;[Ljava/lang/String;Ljava/lang/String;[Ljava/lang/String;Ljava/lang/String;)Landroid/database/Cursor; public fun shutdown ()V - public fun update (Landroid/net/Uri;Landroid/content/ContentValues;Ljava/lang/String;[Ljava/lang/String;)I } -public final class io/sentry/android/core/SentryPerformanceProvider : android/content/ContentProvider, android/app/Application$ActivityLifecycleCallbacks { +public final class io/sentry/android/core/SentryPerformanceProvider : android/app/Application$ActivityLifecycleCallbacks { public fun ()V public fun attachInfo (Landroid/content/Context;Landroid/content/pm/ProviderInfo;)V - public fun delete (Landroid/net/Uri;Ljava/lang/String;[Ljava/lang/String;)I public fun getType (Landroid/net/Uri;)Ljava/lang/String; - public fun insert (Landroid/net/Uri;Landroid/content/ContentValues;)Landroid/net/Uri; public fun onActivityCreated (Landroid/app/Activity;Landroid/os/Bundle;)V public fun onActivityDestroyed (Landroid/app/Activity;)V public fun onActivityPaused (Landroid/app/Activity;)V @@ -215,8 +209,6 @@ public final class io/sentry/android/core/SentryPerformanceProvider : android/co public fun onActivityStarted (Landroid/app/Activity;)V public fun onActivityStopped (Landroid/app/Activity;)V public fun onCreate ()Z - public fun query (Landroid/net/Uri;[Ljava/lang/String;Ljava/lang/String;[Ljava/lang/String;Ljava/lang/String;)Landroid/database/Cursor; - public fun update (Landroid/net/Uri;Landroid/content/ContentValues;Ljava/lang/String;[Ljava/lang/String;)I } public final class io/sentry/android/core/SystemEventsBreadcrumbsIntegration : io/sentry/Integration, java/io/Closeable {