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

Commit b5819114 authored by Matías Hernández's avatar Matías Hernández Committed by Nishith Khanna
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.
* 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).

DISABLE_TOPIC_PROTECTOR

Bug: 426207912
Test: atest NotificationManagerServiceTest PreferencesHelperTest
(cherry picked from commit 236cff68)
(cherry picked from commit 8b986fffe8efab8b9ccecabe8dffed3ef49d6834)
Cherrypick-From: https://googleplex-android-review.googlesource.com/q/commit:db461040cf3e0418927291579e697120ce6b9e2d
Merged-In: I39c280765ffe26588852ea606dac22ed841d791f
Change-Id: I39c280765ffe26588852ea606dac22ed841d791f
parent 2857bbf2
Loading
Loading
Loading
Loading
+27 −15
Original line number Diff line number Diff line
@@ -102,6 +102,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;
@@ -458,7 +459,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[] {
@@ -4567,7 +4567,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) {
@@ -4858,7 +4858,7 @@ public class NotificationManagerService extends SystemService {
                String callingPkg, String targetPkg, int userId) {
            if (canNotifyAsPackage(callingPkg, targetPkg, userId)
                || isCallingUidSystem()) {
                int targetUid = -1;
                int targetUid = INVALID_UID;
                try {
                    targetUid = mPackageManagerClient.getPackageUidAsUser(targetPkg, userId);
                } catch (NameNotFoundException e) {
@@ -6889,14 +6889,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
@@ -13779,15 +13777,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);
@@ -13857,8 +13862,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);
                    }
                }
            }
@@ -13898,10 +13908,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);
                    }
                }
            }
@@ -13914,7 +13926,7 @@ public class NotificationManagerService extends SystemService {
            } catch (RemoteException e) {
                Slog.e(TAG, "role manager has bad default " + pkg + " " + userId);
            }
            return -1;
            return INVALID_UID;
        }
    }
+94 −82

File changed.

Preview size limit exceeded, changes collapsed.

+53 −0
Original line number Diff line number Diff line
@@ -95,6 +95,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;
@@ -214,6 +215,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;
@@ -390,6 +392,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;
@@ -639,6 +642,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());
@@ -656,6 +660,9 @@ public class NotificationManagerServiceTest extends UiServiceTestCase {
        // MockPackageManager - default returns ApplicationInfo with matching calling UID
        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();
@@ -667,6 +674,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
@@ -5357,6 +5366,50 @@ public class NotificationManagerServiceTest extends UiServiceTestCase {
        verify(mPreferencesHelper, never()).getNotificationChannelGroups(anyString(), 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
    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 testHasCompanionDevice_failure() throws Exception {
        when(mCompanionMgr.getAssociations(anyString(), anyInt())).thenThrow(
+64 −66

File changed.

Preview size limit exceeded, changes collapsed.

+32 −1
Original line number Diff line number Diff line
@@ -25,11 +25,15 @@ import static junit.framework.Assert.assertFalse;
import static junit.framework.Assert.assertTrue;

import static org.mockito.ArgumentMatchers.anyBoolean;
import static org.mockito.Matchers.eq;
import static org.mockito.ArgumentMatchers.anyLong;
import static org.mockito.ArgumentMatchers.eq;
import static org.mockito.Mockito.doReturn;
import static org.mockito.Mockito.doThrow;
import static org.mockito.Mockito.any;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.times;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.verifyNoMoreInteractions;
import static org.mockito.Mockito.when;

import static java.util.Arrays.asList;
@@ -49,6 +53,7 @@ import android.content.pm.IPackageManager;
import android.content.pm.PackageManager;
import android.os.Looper;
import android.os.PowerManager;
import android.os.RemoteException;
import android.os.UserHandle;
import android.os.UserManager;
import android.permission.PermissionManager;
@@ -233,6 +238,32 @@ public class RoleObserverTest extends UiServiceTestCase {
        verify(mPreferencesHelper, times(1)).updateDefaultApps(0, null, emer0Pair);
    }

    @Test
    public void testInit_withInvalidPackages_invalidIgnored() throws Exception {
        UserHandle user = new UserHandle(0);
        when(mUm.getUserHandles(anyBoolean())).thenReturn(List.of(user));
        String invalidDialerPkg = "invalidDialer";
        String validEmergencyPkg = "emergency";
        doThrow(new RemoteException(new PackageManager.NameNotFoundException())).when(mPm)
                .getPackageUid(eq(invalidDialerPkg), anyLong(), eq(user.getIdentifier()));
        doReturn(40).when(mPm)
                .getPackageUid(eq(validEmergencyPkg), anyLong(), eq(user.getIdentifier()));
        when(mRoleManager.getRoleHoldersAsUser(ROLE_DIALER, user))
                .thenReturn(List.of(invalidDialerPkg));
        when(mRoleManager.getRoleHoldersAsUser(ROLE_EMERGENCY, user))
                .thenReturn(List.of(validEmergencyPkg));

        mRoleObserver.init();

        // Only valid packages were passed to PreferencesHelper
        ArraySet<Pair<String, Integer>> expectedEmergencyApps = new ArraySet<>();
        expectedEmergencyApps.add(new Pair<>(validEmergencyPkg, 40));
        ArraySet<Pair<String, Integer>> expectedDialerApps = new ArraySet<>();
        verify(mPreferencesHelper).updateDefaultApps(0, null, expectedEmergencyApps);
        verify(mPreferencesHelper).updateDefaultApps(0, null, expectedDialerApps);
        verifyNoMoreInteractions(mPreferencesHelper);
    }

    @Test
    public void testInit_forTrampolines() throws Exception {
        when(mPm.getPackageUid("com.browser", MATCH_ALL, 0)).thenReturn(30);