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

Commit 236cff68 authored by Matías Hernández's avatar Matías Hernández
Browse files

Prevent accidental creation of PackagePreferences for non-existing packages 

This could be caused by (indirectly) invoking PreferencesHelper.getOrCreatePackagePreferences with an INVALID_UID (e.g. as the result of NMS passing along the result of getUidForPackageAndUser for a missing package). This was never intended -- API calls can result in the creation of PackagePreferences (since the structure is lazily initialized) but only for present packages. In most cases this was prevented by other checks (e.g. canNotifyAsPackage, which fails if missing) but specifically for the "fromPrivilegedListener" methods there was no similar enforcement.

Switch the PreferencesHelper usage so that:
* setters (e.g. create/updateChannel, groups) throw if supplied an invalid uid (this is flag-controlled).
* getters that should have PackagePreferences creation as a side effect (e.g. getChannel) early exit if supplied an invalid uid.
* most getters don't actually create PackagePreferences and just return default values (e.g. canShowBadge) if they don't exist yet.

(Also, unify constants for representing a missing package into Process.INVALID_UID, and fix several tests that were impropely set up).

Bug: 430250142
Test: atest NotificationManagerServiceTest PreferencesHelperTest
Flag: com.android.server.notification.preferences_throws_on_invalid_uid
Change-Id: I7d0717aed9fb26490f68801c9220833f2bed8e23
parent 8286994c

services/core/java/com/android/server/notification/NotificationManagerService.java

+27 −20
Original line number Diff line number Diff line
@@ -110,6 +110,7 @@ import static android.os.IServiceManager.DUMP_FLAG_PRIORITY_CRITICAL; 
import static android.os.IServiceManager.DUMP_FLAG_PRIORITY_NORMAL;

import static android.os.PowerWhitelistManager.REASON_NOTIFICATION_SERVICE;

import static android.os.PowerWhitelistManager.TEMPORARY_ALLOWLIST_TYPE_FOREGROUND_SERVICE_ALLOWED;

import static android.os.Process.INVALID_UID;

import static android.os.UserHandle.USER_ALL;

import static android.os.UserHandle.USER_NULL;

import static android.os.UserHandle.USER_SYSTEM;
@@ -503,7 +504,6 @@ public class NotificationManagerService extends SystemService { 
     */

    private static final int NOTIFICATION_RAPID_CLEAR_THRESHOLD_MS = 5000;













    static final int INVALID_UID = -1;















    static final String ROOT_PKG = "root";





























    static final String[] DEFAULT_ALLOWED_ADJUSTMENTS = new String[] {










@@ -5175,7 +5175,7 @@ public class NotificationManagerService extends SystemService {













                String conversationId) {















            if (canNotifyAsPackage(callingPkg, targetPkg, userId)















                    || isCallerSystemOrSystemUiOrShell()) {













                int targetUid = -1;













                int targetUid = INVALID_UID;















                try {















                    targetUid = mPackageManagerClient.getPackageUidAsUser(targetPkg, userId);















                } catch (NameNotFoundException e) {










@@ -5477,7 +5477,7 @@ public class NotificationManagerService extends SystemService {













                String targetPkg, @CannotBeSpecialUser @UserIdInt int userId) {















            if (canNotifyAsPackage(callingPkg, targetPkg, userId)















                || isCallingUidSystem()) {













                int targetUid = -1;













                int targetUid = INVALID_UID;















                try {















                    targetUid = mPackageManagerClient.getPackageUidAsUser(targetPkg, userId);















                } catch (NameNotFoundException e) {










@@ -7410,9 +7410,6 @@ public class NotificationManagerService extends SystemService {













                }















            }















            int uid = getUidForPackageAndUser(pkg, user);













            if (uid == INVALID_UID) {













                return null;













            }















            NotificationChannel parentChannel =















                    mPreferencesHelper.getNotificationChannel(pkg, uid, parentId, false);















            if (parentChannel == null) {










@@ -7571,14 +7568,12 @@ public class NotificationManagerService extends SystemService {













        }





























        private int getUidForPackageAndUser(String pkg, UserHandle user) throws RemoteException {













            int uid = INVALID_UID;















            final long identity = Binder.clearCallingIdentity();















            try {













                uid = mPackageManager.getPackageUid(pkg, 0, user.getIdentifier());













                return mPackageManager.getPackageUid(pkg, 0, user.getIdentifier());















            } finally {















                Binder.restoreCallingIdentity(identity);















            }













            return uid;















        }





























        @Override










@@ -9225,12 +9220,10 @@ public class NotificationManagerService extends SystemService {













                if (canPostPromoted) {















                    notification.flags |= FLAG_PROMOTED_ONGOING;















                }



























            }















        }















    }









































    /**















     * Whether a notification can be non-dismissible.















     * A notification should be dismissible, unless it's exempted for some reason.










@@ -15019,15 +15012,22 @@ public class NotificationManagerService extends SystemService {













            List<UserHandle> users = mUm.getUserHandles(/* excludeDying */ true);















            mNonBlockableDefaultApps = new ArrayMap<>();















            for (int i = 0; i < NON_BLOCKABLE_DEFAULT_ROLES.length; i++) {













                String role = NON_BLOCKABLE_DEFAULT_ROLES[i];















                final ArrayMap<Integer, ArraySet<String>> userToApprovedList = new ArrayMap<>();













                mNonBlockableDefaultApps.put(NON_BLOCKABLE_DEFAULT_ROLES[i], userToApprovedList);













                mNonBlockableDefaultApps.put(role, userToApprovedList);















                for (int j = 0; j < users.size(); j++) {













                    Integer userId = users.get(j).getIdentifier();













                    int userId = users.get(j).getIdentifier();















                    ArraySet<String> approvedForUserId = new ArraySet<>(mRm.getRoleHoldersAsUser(













                            NON_BLOCKABLE_DEFAULT_ROLES[i], UserHandle.of(userId)));













                            role, UserHandle.of(userId)));















                    ArraySet<Pair<String, Integer>> approvedAppUids = new ArraySet<>();















                    for (String pkg : approvedForUserId) {













                        approvedAppUids.add(new Pair(pkg, getUidForPackage(pkg, userId)));













                        int uid = getUidForPackage(pkg, userId);













                        if (uid != INVALID_UID) {













                            approvedAppUids.add(new Pair<>(pkg, uid));













                        } else {













                            Slog.e(TAG, "init: Invalid package for role " + role













                                    + " (user " + userId + "): " + pkg);













                        }















                    }















                    userToApprovedList.put(userId, approvedForUserId);















                    mPreferencesHelper.updateDefaultApps(userId, null, approvedAppUids);










@@ -15097,8 +15097,13 @@ public class NotificationManagerService extends SystemService {













            }















            for (String nowApproved : roleHolders) {















                if (!previouslyApproved.contains(nowApproved)) {













                    toAdd.add(new Pair(nowApproved,













                            getUidForPackage(nowApproved, user.getIdentifier())));













                    int uid = getUidForPackage(nowApproved, user.getIdentifier());













                    if (uid != INVALID_UID) {













                        toAdd.add(new Pair<>(nowApproved, uid));













                    } else {













                        Slog.e(TAG, "onRoleHoldersChanged: Invalid package for role " + roleName













                                + " (user " + user.getIdentifier() + "): " + nowApproved);













                    }















                }















            }
























@@ -15138,10 +15143,12 @@ public class NotificationManagerService extends SystemService {













                UserHandle user = users[i];















                for (String pkg : mRm.getRoleHoldersAsUser(RoleManager.ROLE_BROWSER, user)) {















                    int uid = getUidForPackage(pkg, user.getIdentifier());













                    if (uid != -1) {













                    if (uid != INVALID_UID) {















                        newUids.add(uid);















                    } else {













                        Slog.e(TAG, "Bad uid (-1) for browser package " + pkg);













                        Slog.e(TAG, "updateTrampoline: Invalid package for role "













                                + RoleManager.ROLE_BROWSER + " (user " + user.getIdentifier()













                                + "): " + pkg);















                    }















                }















            }









@@ -15154,7 +15161,7 @@ public class NotificationManagerService extends SystemService {













            } catch (RemoteException e) {















                Slog.e(TAG, "role manager has bad default " + pkg + " " + userId);















            }













            return -1;













            return INVALID_UID;















        }















    }







































services/core/java/com/android/server/notification/PreferencesHelper.java



+99
−88


























File changed.
Preview size limit exceeded, changes collapsed.
























services/core/java/com/android/server/notification/flags.aconfig



+10
−0




























Original line number
Diff line number
Diff line








@@ -227,3 +227,13 @@ flag {













  description: "Use random ids for new conversation channels, instead of concatenating parent id and conversation id"















  bug: "432250872"















}



























flag {













  name: "preferences_throws_on_invalid_uid"













  namespace: "systemui"













  description: "Throw IAE when calling getOrCreatePackagePreferences() with INVALID_UID."













  bug: "390441924"













  metadata {













    purpose: PURPOSE_BUGFIX













  }













}
























services/tests/uiservicestests/src/com/android/server/notification/NotificationManagerServiceTest.java



+53
−0




























Original line number
Diff line number
Diff line








@@ -102,6 +102,7 @@ import static android.os.Flags.FLAG_ALLOW_PRIVATE_PROFILE;













import static android.os.PowerManager.PARTIAL_WAKE_LOCK;















import static android.os.PowerWhitelistManager.REASON_NOTIFICATION_SERVICE;















import static android.os.PowerWhitelistManager.TEMPORARY_ALLOWLIST_TYPE_FOREGROUND_SERVICE_ALLOWED;













import static android.os.Process.INVALID_UID;















import static android.os.UserHandle.USER_SYSTEM;















import static android.os.UserManager.USER_TYPE_FULL_SECONDARY;















import static android.os.UserManager.USER_TYPE_FULL_SYSTEM;










@@ -230,6 +231,7 @@ import android.app.Person;













import android.app.RemoteInput;















import android.app.RemoteInputHistoryItem;















import android.app.StatsManager;













import android.app.WallpaperManager;















import android.app.ZenBypassingApp;















import android.app.admin.DevicePolicyManagerInternal;















import android.app.backup.BackupRestoreEventLogger;










@@ -414,6 +416,7 @@ public class NotificationManagerServiceTest extends UiServiceTestCase {













    private static final String TEST_CHANNEL_ID = "NotificationManagerServiceTestChannelId";















    private static final String TEST_PACKAGE = "The.name.is.Package.Test.Package";















    private static final String PKG_NO_CHANNELS = "com.example.no.channels";













    private static final String MISSING_PACKAGE = "MISSING!";















    private static final int TEST_TASK_ID = 1;















    private static final int UID_HEADLESS = 1_000_000;















    private static final int TOAST_DURATION = 2_000;










@@ -663,6 +666,7 @@ public class NotificationManagerServiceTest extends UiServiceTestCase {













        mContext.addMockSystemService(Context.USER_SERVICE, mUm);















        mContext.addMockSystemService(Context.ACCESSIBILITY_SERVICE,















                mock(AccessibilityManager.class));













        mContext.addMockSystemService(WallpaperManager.class, mock(WallpaperManager.class));





























        doNothing().when(mContext).sendBroadcast(any(), anyString());















        doNothing().when(mContext).sendBroadcastAsUser(any(), any());









@@ -684,6 +688,8 @@ public class NotificationManagerServiceTest extends UiServiceTestCase {













        mContext.setMockPackageManager(mPackageManagerClient);





























        when(mPackageManager.getPackageUid(eq(mPkg), anyLong(), eq(mUserId))).thenReturn(mUid);













        when(mPackageManager.getPackageUid(eq(MISSING_PACKAGE), anyLong(), anyInt()))













                .thenReturn(INVALID_UID);















        when(mPackageManager.getApplicationInfo(anyString(), anyLong(), anyInt()))















                .thenAnswer((Answer<ApplicationInfo>) invocation -> {















                    Object[] args = invocation.getArguments();









@@ -695,6 +701,8 @@ public class NotificationManagerServiceTest extends UiServiceTestCase {













                    return getApplicationInfo((String) args[0], mUid);















                });















        when(mPackageManagerClient.getPackageUidAsUser(any(), anyInt())).thenReturn(mUid);













        when(mPackageManagerClient.getPackageUidAsUser(eq(MISSING_PACKAGE), anyInt()))













                .thenThrow(new PackageManager.NameNotFoundException("Missing package!"));















        when(mPackageManagerInternal.isSameApp(anyString(), anyInt(), anyInt())).thenAnswer(















                (Answer<Boolean>) invocation -> {















                    // TODO: b/317957802 - This is overly broad and basically makes ANY










@@ -5729,6 +5737,51 @@ public class NotificationManagerServiceTest extends UiServiceTestCase {













                anyInt());















    }



























    @Test













    public void getNotificationChannelsFromPrivilegedListener_invalidPackage_returnsNull()













            throws Exception {













        when(mListeners.checkServiceTokenLocked(any())).thenReturn(mListener);













        when(mCompanionMgr.getAssociations(mPkg, mUserId))













                .thenReturn(singletonList(mock(AssociationInfo.class)));

























        ParceledListSlice<?> channels =













                mBinderService.getNotificationChannelsFromPrivilegedListener(













                        mock(INotificationListener.class), MISSING_PACKAGE, mUser);

























        assertThat(channels.getList()).isEmpty();













    }

























    @Test













    public void getNotificationChannelGroupsFromPrivilegedListener_invalidPackage_returnsEmpty()













            throws Exception {













        when(mListeners.checkServiceTokenLocked(any())).thenReturn(mListener);













        when(mCompanionMgr.getAssociations(mPkg, mUserId))













                .thenReturn(singletonList(mock(AssociationInfo.class)));

























        ParceledListSlice<?> groups =













                mBinderService.getNotificationChannelGroupsFromPrivilegedListener(













                        mock(INotificationListener.class), MISSING_PACKAGE, mUser);

























        assertThat(groups.getList()).isEmpty();













    }

























    @Test













    @EnableFlags(Flags.FLAG_PREFERENCES_THROWS_ON_INVALID_UID)













    public void updateNotificationChannelFromPrivilegedListener_invalidPackage_throws()













            throws Exception {













        when(mListeners.checkServiceTokenLocked(any())).thenReturn(mListener);













        when(mCompanionMgr.getAssociations(mPkg, mUserId))













                .thenReturn(singletonList(mock(AssociationInfo.class)));

























        Exception e = assertThrows(IllegalArgumentException.class,













                () -> mBinderService.updateNotificationChannelFromPrivilegedListener(













                        mock(INotificationListener.class), MISSING_PACKAGE, mUser,













                        mTestNotificationChannel));

























        assertThat(e).hasMessageThat().isEqualTo(













                "Valid uid required to get settings of " + MISSING_PACKAGE);













    }



























    @Test















    public void testGetPackagesWithChannels_blocked() throws Exception {















        // While we mostly rely on the PreferencesHelper implementation of channels, we filter in

































services/tests/uiservicestests/src/com/android/server/notification/PreferencesHelperTest.java



+76
−73


























File changed.
Preview size limit exceeded, changes collapsed.