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

Commit f695eb04 authored by Pranav Madapurmath's avatar Pranav Madapurmath Committed by Automerger Merge Worker
Browse files

Resolve StatusHints image exploit across user. am: 9f721a67 

Original change: https://googleplex-android-review.googlesource.com/c/platform/packages/services/Telecomm/+/23438954



Change-Id: Ifc1273c4004b6e5aba440d0f5a9343436bcbd59a
Signed-off-by: default avatarAutomerger Merge Worker <android-build-automerger-merge-worker@system.gserviceaccount.com>
parents aaad0de3 9f721a67

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

+31 −0
Original line number Diff line number Diff line
@@ -23,6 +23,7 @@ import android.app.AppOpsManager; 
import android.content.ComponentName;

import android.content.Context;

import android.content.pm.PackageManager;

import android.graphics.drawable.Icon;

import android.net.Uri;

import android.os.Binder;

import android.os.Bundle;
@@ -83,10 +84,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);
@@ -482,6 +490,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) {
@@ -727,10 +743,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);
@@ -962,6 +985,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.provider.BlockedNumberContract;
@@ -54,12 +61,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;
@@ -194,7 +203,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());
@@ -223,7 +232,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());
@@ -254,7 +263,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());
@@ -692,13 +701,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
@@ -716,7 +725,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()
@@ -974,7 +983,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());
@@ -997,7 +1006,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());
@@ -1212,4 +1221,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
@@ -68,6 +68,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
@@ -102,6 +103,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;

        }
@@ -118,6 +124,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;

        }
@@ -134,6 +145,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 {
@@ -494,6 +511,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<>();
@@ -738,7 +756,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,
@@ -759,5 +777,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.