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

Commit 3bddadd3 authored by Jeremy Joslin's avatar Jeremy Joslin
Browse files

Fix logic errors when checking the caller.

A few methods were incorrectly ORing instead of ANDing the frontline
security check. Fixed and added tests.

Test: runtest frameworks-services -c com.android.server.NetworkScoreServiceTest
Change-Id: I20531ad5a8df6fe9b9e2225198be25a2a070b603
parent b94427d2
Loading
Loading
Loading
Loading
+3 −3
Original line number Diff line number Diff line
@@ -667,7 +667,7 @@ public class NetworkScoreService extends INetworkScoreService.Stub {
    @Override
    public boolean setActiveScorer(String packageName) {
        // Only the system can set the active scorer
        if (!isCallerSystemProcess(getCallingUid()) || !callerCanRequestScores()) {
        if (!isCallerSystemProcess(getCallingUid()) && !callerCanRequestScores()) {
            throw new SecurityException(
                    "Caller is neither the system process nor a score requester.");
        }
@@ -736,7 +736,7 @@ public class NetworkScoreService extends INetworkScoreService.Stub {
    @Override
    public List<NetworkScorerAppData> getAllValidScorers() {
        // Only the system can access this data.
        if (!isCallerSystemProcess(getCallingUid()) || !callerCanRequestScores()) {
        if (!isCallerSystemProcess(getCallingUid()) && !callerCanRequestScores()) {
            throw new SecurityException(
                    "Caller is neither the system process nor a score requester.");
        }
@@ -747,7 +747,7 @@ public class NetworkScoreService extends INetworkScoreService.Stub {
    @Override
    public void disableScoring() {
        // Only the active scorer or the system should be allowed to disable scoring.
        if (!isCallerActiveScorer(getCallingUid()) || !callerCanRequestScores()) {
        if (!isCallerActiveScorer(getCallingUid()) && !callerCanRequestScores()) {
            throw new SecurityException(
                    "Caller is neither the active scorer nor the scorer manager.");
        }
+38 −0
Original line number Diff line number Diff line
@@ -562,6 +562,14 @@ public class NetworkScoreServiceTest {
        }
    }

    @Test
    public void testSetActiveScorer_requestNetworkScoresPermission() {
        when(mContext.checkCallingOrSelfPermission(permission.REQUEST_NETWORK_SCORES))
                .thenReturn(PackageManager.PERMISSION_GRANTED);

        mNetworkScoreService.setActiveScorer(null);
    }

    @Test
    public void testDisableScoring_notActiveScorer_noRequestNetworkScoresPermission() {
        bindToScorer(false /*callerIsScorer*/);
@@ -576,6 +584,36 @@ public class NetworkScoreServiceTest {
        }
    }

    @Test
    public void testDisableScoring_activeScorer_noRequestNetworkScoresPermission() {
        bindToScorer(true /*callerIsScorer*/);
        when(mContext.checkCallingOrSelfPermission(permission.REQUEST_NETWORK_SCORES))
                .thenReturn(PackageManager.PERMISSION_DENIED);

        mNetworkScoreService.disableScoring();
    }

    @Test
    public void testGetAllValidScorer_noRequestNetworkScoresPermission() {
        when(mContext.checkCallingOrSelfPermission(permission.REQUEST_NETWORK_SCORES))
                .thenReturn(PackageManager.PERMISSION_DENIED);

        try {
            mNetworkScoreService.getAllValidScorers();
            fail("SecurityException expected");
        } catch (SecurityException e) {
            // expected
        }
    }

    @Test
    public void testGetAllValidScorer_requestNetworkScoresPermission() {
        when(mContext.checkCallingOrSelfPermission(permission.REQUEST_NETWORK_SCORES))
                .thenReturn(PackageManager.PERMISSION_GRANTED);

        mNetworkScoreService.getAllValidScorers();
    }

    @Test
    public void testRegisterNetworkScoreCache_noRequestNetworkScoresPermission() {
        doThrow(new SecurityException()).when(mContext).enforceCallingOrSelfPermission(