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

Commit 1582119d authored by Pranav Madapurmath's avatar Pranav Madapurmath Committed by Android (Google) Code Review
Browse files

Merge "Resolve StatusHints image exploit across user." into udc-dev

parents 75cc5fef 8e821dcb

src/com/android/server/telecom/ConnectionServiceWrapper.java

100644 → 100755
+31 −0
Original line number Diff line number Diff line
@@ -26,6 +26,7 @@ import android.content.pm.PackageManager; 
import android.location.Location;

import android.location.LocationManager;

import android.location.LocationRequest;

import android.graphics.drawable.Icon;

import android.net.Uri;

import android.os.Binder;

import android.os.Bundle;
@@ -100,10 +101,17 @@ public class ConnectionServiceWrapper extends ServiceBinder implements 
                ParcelableConnection connection, Session.Info sessionInfo) {

            Log.startSession(sessionInfo, LogUtils.Sessions.CSW_HANDLE_CREATE_CONNECTION_COMPLETE,

                    mPackageAbbreviation);

            UserHandle callingUserHandle = Binder.getCallingUserHandle();

            long token = Binder.clearCallingIdentity();

            try {

                synchronized (mLock) {

                    logIncoming("handleCreateConnectionComplete %s", callId);

                    // Check status hints image for cross user access

                    if (connection.getStatusHints() != null) {

                        Icon icon = connection.getStatusHints().getIcon();

                        connection.getStatusHints().setIcon(StatusHints.

                                validateAccountIconUserBoundary(icon, callingUserHandle));

                    }

                    ConnectionServiceWrapper.this

                            .handleCreateConnectionComplete(callId, request, connection);
@@ -504,6 +512,14 @@ public class ConnectionServiceWrapper extends ServiceBinder implements 
            Log.startSession(sessionInfo, LogUtils.Sessions.CSW_ADD_CONFERENCE_CALL,

                    mPackageAbbreviation);



            UserHandle callingUserHandle = Binder.getCallingUserHandle();

            // Check status hints image for cross user access

            if (parcelableConference.getStatusHints() != null) {

                Icon icon = parcelableConference.getStatusHints().getIcon();

                parcelableConference.getStatusHints().setIcon(StatusHints.

                        validateAccountIconUserBoundary(icon, callingUserHandle));

            }



            if (parcelableConference.getConnectElapsedTimeMillis() != 0

                    && mContext.checkCallingOrSelfPermission(MODIFY_PHONE_STATE)

                            != PackageManager.PERMISSION_GRANTED) {
@@ -769,10 +785,17 @@ public class ConnectionServiceWrapper extends ServiceBinder implements 
        public void setStatusHints(String callId, StatusHints statusHints,

                Session.Info sessionInfo) {

            Log.startSession(sessionInfo, "CSW.sSH", mPackageAbbreviation);

            UserHandle callingUserHandle = Binder.getCallingUserHandle();

            long token = Binder.clearCallingIdentity();

            try {

                synchronized (mLock) {

                    logIncoming("setStatusHints %s %s", callId, statusHints);

                    // Check status hints image for cross user access

                    if (statusHints != null) {

                        Icon icon = statusHints.getIcon();

                        statusHints.setIcon(StatusHints.validateAccountIconUserBoundary(

                                icon, callingUserHandle));

                    }

                    Call call = mCallIdMapper.getCall(callId);

                    if (call != null) {

                        call.setStatusHints(statusHints);
@@ -1007,6 +1030,14 @@ public class ConnectionServiceWrapper extends ServiceBinder implements 
                                    connection.getCallDirection(),

                                    connection.getCallerNumberVerificationStatus());

                        }



                        // Check status hints image for cross user access

                        if (connection.getStatusHints() != null) {

                            Icon icon = connection.getStatusHints().getIcon();

                            connection.getStatusHints().setIcon(StatusHints.

                                    validateAccountIconUserBoundary(icon, userHandle));

                        }



                        // Check to see if this Connection has already been added.

                        Call alreadyAddedConnection = mCallsManager

                                .getAlreadyAddedConnection(connectIdToCheck);

tests/src/com/android/server/telecom/tests/BasicCallTests.java

+158 −8
Original line number Diff line number Diff line
@@ -16,8 +16,11 @@ 


package com.android.server.telecom.tests;



import static com.android.server.telecom.tests.ConnectionServiceFixture.STATUS_HINTS_EXTRA;



import static org.junit.Assert.assertEquals;

import static org.junit.Assert.assertFalse;

import static org.junit.Assert.assertNotNull;

import static org.junit.Assert.assertNull;

import static org.junit.Assert.assertTrue;

import static org.mockito.ArgumentMatchers.nullable;
@@ -38,10 +41,14 @@ import android.content.Context; 
import android.content.IContentProvider;

import android.content.pm.PackageManager;

import android.media.AudioDeviceInfo;

import android.content.Intent;

import android.graphics.drawable.Icon;

import android.media.AudioManager;

import android.net.Uri;

import android.os.Binder;

import android.os.Bundle;

import android.os.Handler;

import android.os.Looper;

import android.os.Process;

import android.os.UserHandle;

import android.os.UserManager;
@@ -55,12 +62,14 @@ import android.telecom.Log; 
import android.telecom.ParcelableCall;

import android.telecom.PhoneAccount;

import android.telecom.PhoneAccountHandle;

import android.telecom.StatusHints;

import android.telecom.TelecomManager;

import android.telecom.VideoProfile;

import android.test.suitebuilder.annotation.LargeTest;

import android.test.suitebuilder.annotation.MediumTest;



import androidx.test.filters.FlakyTest;

import androidx.test.filters.SmallTest;



import com.android.internal.telecom.IInCallAdapter;

import android.telecom.CallerInfo;
@@ -196,7 +205,7 @@ public class BasicCallTests extends TelecomSystemTest { 
    @Test

    public void testTelecomManagerAcceptRingingVideoCall() throws Exception {

        IdPair ids = startIncomingPhoneCall("650-555-1212", mPhoneAccountA0.getAccountHandle(),

                VideoProfile.STATE_BIDIRECTIONAL, mConnectionServiceFixtureA);

                VideoProfile.STATE_BIDIRECTIONAL, mConnectionServiceFixtureA, null);



        assertEquals(Call.STATE_RINGING, mInCallServiceFixtureX.getCall(ids.mCallId).getState());

        assertEquals(Call.STATE_RINGING, mInCallServiceFixtureY.getCall(ids.mCallId).getState());
@@ -225,7 +234,7 @@ public class BasicCallTests extends TelecomSystemTest { 
    @Test

    public void testTelecomManagerAcceptRingingVideoCallAsAudio() throws Exception {

        IdPair ids = startIncomingPhoneCall("650-555-1212", mPhoneAccountA0.getAccountHandle(),

                VideoProfile.STATE_BIDIRECTIONAL, mConnectionServiceFixtureA);

                VideoProfile.STATE_BIDIRECTIONAL, mConnectionServiceFixtureA, null);



        assertEquals(Call.STATE_RINGING, mInCallServiceFixtureX.getCall(ids.mCallId).getState());

        assertEquals(Call.STATE_RINGING, mInCallServiceFixtureY.getCall(ids.mCallId).getState());
@@ -256,7 +265,7 @@ public class BasicCallTests extends TelecomSystemTest { 
    @Test

    public void testTelecomManagerAcceptRingingInvalidVideoState() throws Exception {

        IdPair ids = startIncomingPhoneCall("650-555-1212", mPhoneAccountA0.getAccountHandle(),

                VideoProfile.STATE_BIDIRECTIONAL, mConnectionServiceFixtureA);

                VideoProfile.STATE_BIDIRECTIONAL, mConnectionServiceFixtureA, null);



        assertEquals(Call.STATE_RINGING, mInCallServiceFixtureX.getCall(ids.mCallId).getState());

        assertEquals(Call.STATE_RINGING, mInCallServiceFixtureY.getCall(ids.mCallId).getState());
@@ -694,13 +703,13 @@ public class BasicCallTests extends TelecomSystemTest { 
    @MediumTest

    @Test

    public void testBasicConferenceCall() throws Exception {

        makeConferenceCall();

        makeConferenceCall(null, null);

    }



    @MediumTest

    @Test

    public void testAddCallToConference1() throws Exception {

        ParcelableCall conferenceCall = makeConferenceCall();

        ParcelableCall conferenceCall = makeConferenceCall(null, null);

        IdPair callId3 = startAndMakeActiveOutgoingCall("650-555-1214",

                mPhoneAccountA0.getAccountHandle(), mConnectionServiceFixtureA);

        // testAddCallToConference{1,2} differ in the order of arguments to InCallAdapter#conference
@@ -718,7 +727,7 @@ public class BasicCallTests extends TelecomSystemTest { 
    @MediumTest

    @Test

    public void testAddCallToConference2() throws Exception {

        ParcelableCall conferenceCall = makeConferenceCall();

        ParcelableCall conferenceCall = makeConferenceCall(null, null);

        IdPair callId3 = startAndMakeActiveOutgoingCall("650-555-1214",

                mPhoneAccountA0.getAccountHandle(), mConnectionServiceFixtureA);

        mInCallServiceFixtureX.getInCallAdapter()
@@ -976,7 +985,7 @@ public class BasicCallTests extends TelecomSystemTest { 
    public void testOutgoingCallSelectPhoneAccountVideo() throws Exception {

        startOutgoingPhoneCallPendingCreateConnection("650-555-1212",

                null, mConnectionServiceFixtureA,

                Process.myUserHandle(), VideoProfile.STATE_BIDIRECTIONAL);

                Process.myUserHandle(), VideoProfile.STATE_BIDIRECTIONAL, null);

        com.android.server.telecom.Call call = mTelecomSystem.getCallsManager().getCalls()

                .iterator().next();

        assert(call.isVideoCallingSupportedByPhoneAccount());
@@ -999,7 +1008,7 @@ public class BasicCallTests extends TelecomSystemTest { 
    public void testOutgoingCallSelectPhoneAccountNoVideo() throws Exception {

        startOutgoingPhoneCallPendingCreateConnection("650-555-1212",

                null, mConnectionServiceFixtureA,

                Process.myUserHandle(), VideoProfile.STATE_BIDIRECTIONAL);

                Process.myUserHandle(), VideoProfile.STATE_BIDIRECTIONAL, null);

        com.android.server.telecom.Call call = mTelecomSystem.getCallsManager().getCalls()

                .iterator().next();

        assert(call.isVideoCallingSupportedByPhoneAccount());
@@ -1214,4 +1223,145 @@ public class BasicCallTests extends TelecomSystemTest { 
        assertTrue(muteValues.get(0));

        assertFalse(muteValues.get(1));

    }



    /**

     * Verifies that StatusHints image is validated in ConnectionServiceWrapper#addConferenceCall

     * when the image doesn't belong to the calling user. Simulates a scenario where an app

     * could manipulate the contents of the bundle and send it via the binder to upload an image

     * from another user.

     *

     * @throws Exception

     */

    @SmallTest

    @Test

    public void testValidateStatusHintsImage_addConferenceCall() throws Exception {

        Intent callIntent1 = new Intent();

        // Stub intent for call2

        Intent callIntent2 = new Intent();

        Bundle callExtras1 = new Bundle();

        Icon icon = Icon.createWithContentUri("content://10@media/external/images/media/");

        // Load StatusHints extra into TelecomManager.EXTRA_OUTGOING_CALL_EXTRAS to be processed

        // as the call extras. This will be leveraged in ConnectionServiceFixture to set the

        // StatusHints for the given connection.

        StatusHints statusHints = new StatusHints(icon);

        assertNotNull(statusHints.getIcon());

        callExtras1.putParcelable(STATUS_HINTS_EXTRA, statusHints);

        callIntent1.putExtra(TelecomManager.EXTRA_OUTGOING_CALL_EXTRAS, callExtras1);



        // Start conference call to invoke ConnectionServiceWrapper#addConferenceCall.

        // Note that the calling user would be User 0.

        ParcelableCall conferenceCall = makeConferenceCall(callIntent1, callIntent2);



        // Ensure that StatusHints was set.

        assertNotNull(mInCallServiceFixtureX.getCall(mInCallServiceFixtureX.mLatestCallId)

                .getStatusHints());

        // Ensure that the StatusHints image icon was disregarded.

        assertNull(mInCallServiceFixtureX.getCall(mInCallServiceFixtureX.mLatestCallId)

                .getStatusHints().getIcon());

    }



    /**

     * Verifies that StatusHints image is validated in

     * ConnectionServiceWrapper#handleCreateConnectionComplete when the image doesn't belong to the

     * calling user. Simulates a scenario where an app could manipulate the contents of the

     * bundle and send it via the binder to upload an image from another user.

     *

     * @throws Exception

     */

    @SmallTest

    @Test

    public void testValidateStatusHintsImage_handleCreateConnectionComplete() throws Exception {

        Bundle extras = new Bundle();

        Icon icon = Icon.createWithContentUri("content://10@media/external/images/media/");

        // Load the bundle with the test extra in order to simulate an app directly invoking the

        // binder on ConnectionServiceWrapper#handleCreateConnectionComplete.

        StatusHints statusHints = new StatusHints(icon);

        assertNotNull(statusHints.getIcon());

        extras.putParcelable(STATUS_HINTS_EXTRA, statusHints);



        // Start incoming call with StatusHints extras

        // Note that the calling user in ConnectionServiceWrapper#handleCreateConnectionComplete

        // would be User 0.

        IdPair ids = startIncomingPhoneCallWithExtras("650-555-1212",

                mPhoneAccountA0.getAccountHandle(), mConnectionServiceFixtureA, extras);



        // Ensure that StatusHints was set.

        assertNotNull(mInCallServiceFixtureX.getCall(ids.mCallId).getStatusHints());

        // Ensure that the StatusHints image icon was disregarded.

        assertNull(mInCallServiceFixtureX.getCall(ids.mCallId).getStatusHints().getIcon());

    }



    /**

     * Verifies that StatusHints image is validated in ConnectionServiceWrapper#setStatusHints

     * when the image doesn't belong to the calling user. Simulates a scenario where an app

     * could manipulate the contents of the bundle and send it via the binder to upload an image

     * from another user.

     *

     * @throws Exception

     */

    @SmallTest

    @Test

    public void testValidateStatusHintsImage_setStatusHints() throws Exception {

        IdPair outgoing = startAndMakeActiveOutgoingCall("650-555-1214",

                mPhoneAccountA0.getAccountHandle(), mConnectionServiceFixtureA);



        // Modify existing connection with StatusHints image exploit

        Icon icon = Icon.createWithContentUri("content://10@media/external/images/media/");

        StatusHints statusHints = new StatusHints(icon);

        assertNotNull(statusHints.getIcon());

        ConnectionServiceFixture.ConnectionInfo connectionInfo = mConnectionServiceFixtureA

                .mConnectionById.get(outgoing.mConnectionId);

        connectionInfo.statusHints = statusHints;



        // Invoke ConnectionServiceWrapper#setStatusHints.

        // Note that the calling user would be User 0.

        mConnectionServiceFixtureA.sendSetStatusHints(outgoing.mConnectionId);

        waitForHandlerAction(mConnectionServiceFixtureA.mConnectionServiceDelegate.getHandler(),

                TEST_TIMEOUT);



        // Ensure that StatusHints was set.

        assertNotNull(mInCallServiceFixtureX.getCall(outgoing.mCallId).getStatusHints());

        // Ensure that the StatusHints image icon was disregarded.

        assertNull(mInCallServiceFixtureX.getCall(outgoing.mCallId)

                .getStatusHints().getIcon());

    }



    /**

     * Verifies that StatusHints image is validated in

     * ConnectionServiceWrapper#addExistingConnection when the image doesn't belong to the calling

     * user. Simulates a scenario where an app could manipulate the contents of the bundle and

     * send it via the binder to upload an image from another user.

     *

     * @throws Exception

     */

    @SmallTest

    @Test

    public void testValidateStatusHintsImage_addExistingConnection() throws Exception {

        IdPair outgoing = startAndMakeActiveOutgoingCall("650-555-1214",

                mPhoneAccountA0.getAccountHandle(), mConnectionServiceFixtureA);

        Connection existingConnection = mConnectionServiceFixtureA.mLatestConnection;



        // Modify existing connection with StatusHints image exploit

        Icon icon = Icon.createWithContentUri("content://10@media/external/images/media/");

        StatusHints modifiedStatusHints = new StatusHints(icon);

        assertNotNull(modifiedStatusHints.getIcon());

        ConnectionServiceFixture.ConnectionInfo connectionInfo = mConnectionServiceFixtureA

                .mConnectionById.get(outgoing.mConnectionId);

        connectionInfo.statusHints = modifiedStatusHints;



        // Invoke ConnectionServiceWrapper#addExistingConnection.

        // Note that the calling user would be User 0.

        mConnectionServiceFixtureA.sendAddExistingConnection(outgoing.mConnectionId);

        waitForHandlerAction(mConnectionServiceFixtureA.mConnectionServiceDelegate.getHandler(),

                TEST_TIMEOUT);



        // Ensure that StatusHints was set. Due to test setup, the ParcelableConnection object that

        // is passed into sendAddExistingConnection is instantiated on invocation. The call's

        // StatusHints are not updated at the time of completion, so instead, we can verify that

        // the ParcelableConnection object was modified.

        assertNotNull(mConnectionServiceFixtureA.mLatestParcelableConnection.getStatusHints());

        // Ensure that the StatusHints image icon was disregarded.

        assertNull(mConnectionServiceFixtureA.mLatestParcelableConnection

                .getStatusHints().getIcon());

    }

}

tests/src/com/android/server/telecom/tests/CallExtrasTest.java

+3 −3
Original line number Diff line number Diff line
@@ -370,7 +370,7 @@ public class CallExtrasTest extends TelecomSystemTest { 
    @LargeTest

    @Test

    public void testConferenceSetExtras() throws Exception {

        ParcelableCall call = makeConferenceCall();

        ParcelableCall call = makeConferenceCall(null, null);

        String conferenceId = call.getId();



        Conference conference = mConnectionServiceFixtureA.mLatestConference;
@@ -414,7 +414,7 @@ public class CallExtrasTest extends TelecomSystemTest { 
    @FlakyTest(bugId = 117751305)

    @Test

    public void testConferenceExtraOperations() throws Exception {

        ParcelableCall call = makeConferenceCall();

        ParcelableCall call = makeConferenceCall(null, null);

        String conferenceId = call.getId();

        Conference conference = mConnectionServiceFixtureA.mLatestConference;

        assertNotNull(conference);
@@ -450,7 +450,7 @@ public class CallExtrasTest extends TelecomSystemTest { 
    @LargeTest

    @Test

    public void testConferenceICS() throws Exception {

        ParcelableCall call = makeConferenceCall();

        ParcelableCall call = makeConferenceCall(null, null);

        String conferenceId = call.getId();

        Conference conference = mConnectionServiceFixtureA.mLatestConference;

tests/src/com/android/server/telecom/tests/ConnectionServiceFixture.java

+20 −1
Original line number Diff line number Diff line
@@ -69,6 +69,7 @@ public class ConnectionServiceFixture implements TestFixture<IConnectionService> 
    static int INVALID_VIDEO_STATE = -1;

    public CountDownLatch mExtrasLock = new CountDownLatch(1);

    static int NOT_SPECIFIED = 0;

    public static final String STATUS_HINTS_EXTRA = "updateStatusHints";



    /**

     * Implementation of ConnectionService that performs no-ops for tasks normally meant for
@@ -103,6 +104,11 @@ public class ConnectionServiceFixture implements TestFixture<IConnectionService> 
            if (mProperties != NOT_SPECIFIED) {

                fakeConnection.setConnectionProperties(mProperties);

            }

            // Testing for StatusHints image icon cross user access

            if (request.getExtras() != null) {

                fakeConnection.setStatusHints(

                        request.getExtras().getParcelable(STATUS_HINTS_EXTRA));

            }



            return fakeConnection;

        }
@@ -119,6 +125,11 @@ public class ConnectionServiceFixture implements TestFixture<IConnectionService> 
            if (mProperties != NOT_SPECIFIED) {

                fakeConnection.setConnectionProperties(mProperties);

            }

            // Testing for StatusHints image icon cross user access

            if (request.getExtras() != null) {

                fakeConnection.setStatusHints(

                        request.getExtras().getParcelable(STATUS_HINTS_EXTRA));

            }

            return fakeConnection;

        }
@@ -135,6 +146,12 @@ public class ConnectionServiceFixture implements TestFixture<IConnectionService> 
                Conference fakeConference = new FakeConference();

                fakeConference.addConnection(cxn1);

                fakeConference.addConnection(cxn2);

                if (cxn1.getStatusHints() != null || cxn2.getStatusHints() != null) {

                    // For testing purposes, pick one of the status hints that isn't null.

                    StatusHints statusHints = cxn1.getStatusHints() != null

                            ? cxn1.getStatusHints() : cxn2.getStatusHints();

                    fakeConference.setStatusHints(statusHints);

                }

                mLatestConference = fakeConference;

                addConference(fakeConference);

            } else {
@@ -507,6 +524,7 @@ public class ConnectionServiceFixture implements TestFixture<IConnectionService> 


    public String mLatestConnectionId;

    public Connection mLatestConnection;

    public ParcelableConnection mLatestParcelableConnection;

    public Conference mLatestConference;

    public final Set<IConnectionServiceAdapter> mConnectionServiceAdapters = new HashSet<>();

    public final Map<String, ConnectionInfo> mConnectionById = new HashMap<>();
@@ -751,7 +769,7 @@ public class ConnectionServiceFixture implements TestFixture<IConnectionService> 
    }



    private ParcelableConnection parcelable(ConnectionInfo c) {

        return new ParcelableConnection(

        mLatestParcelableConnection = new ParcelableConnection(

                c.request.getAccountHandle(),

                c.state,

                c.capabilities,
@@ -772,5 +790,6 @@ public class ConnectionServiceFixture implements TestFixture<IConnectionService> 
                c.conferenceableConnectionIds,

                c.extras,

                c.callerNumberVerificationStatus);

        return mLatestParcelableConnection;

    }

}

tests/src/com/android/server/telecom/tests/TelecomSystemTest.java

+47 −21

File changed.

Preview size limit exceeded, changes collapsed.