Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit b8a3ba91 authored by Lokesh Kumar Goel's avatar Lokesh Kumar Goel
Browse files

Revert "Cleanup flag for notification sound Uri permission check"

This reverts commit a6e1ff31.

Reason for revert: CVE-2025-22420 A-337775777 is being removed from ASB#2025-04 due to an possible app compatibility regression. Partners are recommended to remove this fix and should not reference it in any public documentation. An updated fix will be required as part of a future Android Security Bulletin.

Pixel Impact: Internal testing found WhatsApp java crashes with this fix. Detected on Dogfood builds. We recommend Pixel removal if possible. Crash report here b/390020418

Change-Id: I18ab82279e67ad0b7b8de509b23e1b7607b1dbbe
parent da3c41b2
Loading
Loading
Loading
Loading
+63 −3
Original line number Diff line number Diff line
@@ -37,7 +37,10 @@ import android.app.KeyguardManager;
import android.app.Notification;
import android.app.NotificationChannel;
import android.app.Person;
import android.content.ContentProvider;
import android.content.ContentResolver;
import android.content.Context;
import android.content.Intent;
import android.content.pm.PackageManager;
import android.content.pm.PackageManagerInternal;
import android.content.pm.ShortcutInfo;
@@ -46,6 +49,7 @@ import android.media.AudioAttributes;
import android.media.AudioSystem;
import android.metrics.LogMaker;
import android.net.Uri;
import android.os.Binder;
import android.os.Build;
import android.os.Bundle;
import android.os.IBinder;
@@ -1509,14 +1513,23 @@ public final class NotificationRecord {

            final Notification notification = getNotification();
            notification.visitUris((uri) -> {
                if (com.android.server.notification.Flags.notificationVerifyChannelSoundUri()) {
                    visitGrantableUri(uri, false, false);
                } else {
                    oldVisitGrantableUri(uri, false, false);
                }
            });

            if (notification.getChannelId() != null) {
                NotificationChannel channel = getChannel();
                if (channel != null) {
                    if (com.android.server.notification.Flags.notificationVerifyChannelSoundUri()) {
                        visitGrantableUri(channel.getSound(), (channel.getUserLockedFields()
                                & NotificationChannel.USER_LOCKED_SOUND) != 0, true);
                    } else {
                        oldVisitGrantableUri(channel.getSound(), (channel.getUserLockedFields()
                                & NotificationChannel.USER_LOCKED_SOUND) != 0, true);
                    }
                }
            }
        } finally {
@@ -1524,6 +1537,53 @@ public final class NotificationRecord {
        }
    }

    /**
     * Note the presence of a {@link Uri} that should have permission granted to
     * whoever will be rendering it.
     * <p>
     * If the enqueuing app has the ability to grant access, it will be added to
     * {@link #mGrantableUris}. Otherwise, this will either log or throw
     * {@link SecurityException} depending on target SDK of enqueuing app.
     */
    private void oldVisitGrantableUri(Uri uri, boolean userOverriddenUri, boolean isSound) {
        if (uri == null || !ContentResolver.SCHEME_CONTENT.equals(uri.getScheme())) return;

        if (mGrantableUris != null && mGrantableUris.contains(uri)) {
            return; // already verified this URI
        }

        final int sourceUid = getSbn().getUid();
        final long ident = Binder.clearCallingIdentity();
        try {
            // This will throw a SecurityException if the caller can't grant.
            mUgmInternal.checkGrantUriPermission(sourceUid, null,
                    ContentProvider.getUriWithoutUserId(uri),
                    Intent.FLAG_GRANT_READ_URI_PERMISSION,
                    ContentProvider.getUserIdFromUri(uri, UserHandle.getUserId(sourceUid)));

            if (mGrantableUris == null) {
                mGrantableUris = new ArraySet<>();
            }
            mGrantableUris.add(uri);
        } catch (SecurityException e) {
            if (!userOverriddenUri) {
                if (isSound) {
                    mSound = Settings.System.DEFAULT_NOTIFICATION_URI;
                    Log.w(TAG, "Replacing " + uri + " from " + sourceUid + ": " + e.getMessage());
                } else {
                    if (mTargetSdkVersion >= Build.VERSION_CODES.P) {
                        throw e;
                    } else {
                        Log.w(TAG,
                                "Ignoring " + uri + " from " + sourceUid + ": " + e.getMessage());
                    }
                }
            }
        } finally {
            Binder.restoreCallingIdentity(ident);
        }
    }

    /**
     * Note the presence of a {@link Uri} that should have permission granted to
     * whoever will be rendering it.
+3 −1
Original line number Diff line number Diff line
@@ -1198,7 +1198,9 @@ public class PreferencesHelper implements RankingConfig {
                // Verify that the app has permission to read the sound Uri
                // Only check for new channels, as regular apps can only set sound
                // before creating. See: {@link NotificationChannel#setSound}
                if (Flags.notificationVerifyChannelSoundUri()) {
                    PermissionHelper.grantUriPermission(mUgmInternal, channel.getSound(), uid);
                }

                channel.setImportanceLockedByCriticalDeviceFunction(
                        r.defaultAppLockedImportance || r.fixedImportance);
+10 −0
Original line number Diff line number Diff line
@@ -164,6 +164,16 @@ flag {
  bug: "358524009"
}

flag {
  name: "notification_verify_channel_sound_uri"
  namespace: "systemui"
  description: "Verify Uri permission for sound when creating a notification channel"
  bug: "337775777"
  metadata {
    purpose: PURPOSE_BUGFIX
  }
}

flag {
  name: "notification_vibration_in_sound_uri_for_channel"
  namespace: "systemui"
+4 −0
Original line number Diff line number Diff line
@@ -66,6 +66,7 @@ import static com.android.internal.config.sysui.SystemUiSystemPropertiesFlags.No
import static com.android.internal.util.FrameworkStatsLog.PACKAGE_NOTIFICATION_PREFERENCES__FSI_STATE__DENIED;
import static com.android.internal.util.FrameworkStatsLog.PACKAGE_NOTIFICATION_PREFERENCES__FSI_STATE__GRANTED;
import static com.android.internal.util.FrameworkStatsLog.PACKAGE_NOTIFICATION_PREFERENCES__FSI_STATE__NOT_REQUESTED;
import static com.android.server.notification.Flags.FLAG_NOTIFICATION_VERIFY_CHANNEL_SOUND_URI;
import static com.android.server.notification.Flags.FLAG_PERSIST_INCOMPLETE_RESTORE_DATA;
import static com.android.server.notification.NotificationChannelLogger.NotificationChannelEvent.NOTIFICATION_CHANNEL_UPDATED_BY_USER;
import static com.android.server.notification.PreferencesHelper.DEFAULT_BUBBLE_PREFERENCE;
@@ -3263,6 +3264,7 @@ public class PreferencesHelperTest extends UiServiceTestCase {
    }

    @Test
    @EnableFlags(FLAG_NOTIFICATION_VERIFY_CHANNEL_SOUND_URI)
    public void testCreateChannel_noSoundUriPermission_contentSchemeVerified() {
        final Uri sound = Uri.parse(SCHEME_CONTENT + "://media/test/sound/uri");

@@ -3282,6 +3284,7 @@ public class PreferencesHelperTest extends UiServiceTestCase {
    }

    @Test
    @EnableFlags(FLAG_NOTIFICATION_VERIFY_CHANNEL_SOUND_URI)
    public void testCreateChannel_noSoundUriPermission_fileSchemaIgnored() {
        final Uri sound = Uri.parse(SCHEME_FILE + "://path/sound");

@@ -3300,6 +3303,7 @@ public class PreferencesHelperTest extends UiServiceTestCase {
    }

    @Test
    @EnableFlags(FLAG_NOTIFICATION_VERIFY_CHANNEL_SOUND_URI)
    public void testCreateChannel_noSoundUriPermission_resourceSchemaIgnored() {
        final Uri sound = Uri.parse(SCHEME_ANDROID_RESOURCE + "://resId/sound");