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

Commit c45bd7f2 authored by Carlos Valdivia's avatar Carlos Valdivia Committed by Paul Lawrence
Browse files

Prevent authenticators from using Settings to launch arbitrary activities. 

Various authenticator results such as getAuthToken and addAccount might
result in an Intent returned to the AccountManager caller. A malicious
authenticator could exploit the fact that the Settings are a system app,
lead the user to launch add account for their account type and thus get
Settings to use the intent to start some arbitrary third parties Activity.

The fix is to make sure that the UID of the app associated with Activity
to be launched by the supplied intent and the Authenticators UID share
the same signature.  This means that an authenticator implementer can only
exploit apps they control.

Bug: 7699048
Change-Id: I34330454c341e6a8422ca1ed3b390466a0feedce
(cherry picked from commit 5bab9daf)
parent 95023066

services/java/com/android/server/accounts/AccountManagerService.java

+55 −1
Original line number Diff line number Diff line
@@ -47,6 +47,7 @@ import android.content.pm.PackageManager; 
import android.content.pm.PackageManager.NameNotFoundException;

import android.content.pm.RegisteredServicesCache;

import android.content.pm.RegisteredServicesCacheListener;

import android.content.pm.ResolveInfo;

import android.content.pm.UserInfo;

import android.database.Cursor;

import android.database.DatabaseUtils;
@@ -583,15 +584,18 @@ public class AccountManagerService 
        try {

            new Session(fromAccounts, null, account.type, false,

                    false /* stripAuthTokenFromResult */) {

                @Override

                protected String toDebugString(long now) {

                    return super.toDebugString(now) + ", getAccountCredentialsForClone"

                            + ", " + account.type;

                }



                @Override

                public void run() throws RemoteException {

                    mAuthenticator.getAccountCredentialsForCloning(this, account);

                }



                @Override

                public void onResult(Bundle result) {

                    if (result != null) {

                        if (result.getBoolean(AccountManager.KEY_BOOLEAN_RESULT, false)) {
@@ -616,11 +620,13 @@ public class AccountManagerService 
        try {

            new Session(targetUser, null, account.type, false,

                    false /* stripAuthTokenFromResult */) {

                @Override

                protected String toDebugString(long now) {

                    return super.toDebugString(now) + ", getAccountCredentialsForClone"

                            + ", " + account.type;

                }



                @Override

                public void run() throws RemoteException {

                    // Confirm that the owner's account still exists before this step.

                    UserAccounts owner = getUserAccounts(UserHandle.USER_OWNER);
@@ -635,6 +641,7 @@ public class AccountManagerService 
                    }

                }



                @Override

                public void onResult(Bundle result) {

                    if (result != null) {

                        if (result.getBoolean(AccountManager.KEY_BOOLEAN_RESULT, false)) {
@@ -649,6 +656,7 @@ public class AccountManagerService 
                    }

                }



                @Override

                public void onError(int errorCode, String errorMessage) {

                    super.onError(errorCode,  errorMessage);

                    // TODO: Show error notification to user
@@ -777,6 +785,7 @@ public class AccountManagerService 
            mAccount = account;

        }



        @Override

        public void run() throws RemoteException {

            try {

                mAuthenticator.hasFeatures(this, mAccount, mFeatures);
@@ -785,6 +794,7 @@ public class AccountManagerService 
            }

        }



        @Override

        public void onResult(Bundle result) {

            IAccountManagerResponse response = getResponseAndClose();

            if (response != null) {
@@ -810,6 +820,7 @@ public class AccountManagerService 
            }

        }



        @Override

        protected String toDebugString(long now) {

            return super.toDebugString(now) + ", hasFeatures"

                    + ", " + mAccount
@@ -866,15 +877,18 @@ public class AccountManagerService 
            mAccount = account;

        }



        @Override

        protected String toDebugString(long now) {

            return super.toDebugString(now) + ", removeAccount"

                    + ", account " + mAccount;

        }



        @Override

        public void run() throws RemoteException {

            mAuthenticator.getAccountRemovalAllowed(this, mAccount);

        }



        @Override

        public void onResult(Bundle result) {

            if (result != null && result.containsKey(AccountManager.KEY_BOOLEAN_RESULT)

                    && !result.containsKey(AccountManager.KEY_INTENT)) {
@@ -1215,16 +1229,19 @@ public class AccountManagerService 
        try {

            new Session(accounts, response, accountType, false,

                    false /* stripAuthTokenFromResult */) {

                @Override

                protected String toDebugString(long now) {

                    return super.toDebugString(now) + ", getAuthTokenLabel"

                            + ", " + accountType

                            + ", authTokenType " + authTokenType;

                }



                @Override

                public void run() throws RemoteException {

                    mAuthenticator.getAuthTokenLabel(this, authTokenType);

                }



                @Override

                public void onResult(Bundle result) {

                    if (result != null) {

                        String label = result.getString(AccountManager.KEY_AUTH_TOKEN_LABEL);
@@ -1302,6 +1319,7 @@ public class AccountManagerService 


            new Session(accounts, response, account.type, expectActivityLaunch,

                    false /* stripAuthTokenFromResult */) {

                @Override

                protected String toDebugString(long now) {

                    if (loginOptions != null) loginOptions.keySet();

                    return super.toDebugString(now) + ", getAuthToken"
@@ -1311,6 +1329,7 @@ public class AccountManagerService 
                            + ", notifyOnAuthFailure " + notifyOnAuthFailure;

                }



                @Override

                public void run() throws RemoteException {

                    // If the caller doesn't have permission then create and return the

                    // "grant permission" intent instead of the "getAuthToken" intent.
@@ -1321,6 +1340,7 @@ public class AccountManagerService 
                    }

                }



                @Override

                public void onResult(Bundle result) {

                    if (result != null) {

                        if (result.containsKey(AccountManager.KEY_AUTH_TOKEN_LABEL)) {
@@ -1485,11 +1505,13 @@ public class AccountManagerService 
        try {

            new Session(accounts, response, accountType, expectActivityLaunch,

                    true /* stripAuthTokenFromResult */) {

                @Override

                public void run() throws RemoteException {

                    mAuthenticator.addAccount(this, mAccountType, authTokenType, requiredFeatures,

                            options);

                }



                @Override

                protected String toDebugString(long now) {

                    return super.toDebugString(now) + ", addAccount"

                            + ", accountType " + accountType
@@ -1530,9 +1552,11 @@ public class AccountManagerService 
        try {

            new Session(accounts, response, account.type, expectActivityLaunch,

                    true /* stripAuthTokenFromResult */) {

                @Override

                public void run() throws RemoteException {

                    mAuthenticator.confirmCredentials(this, account, options);

                }

                @Override

                protected String toDebugString(long now) {

                    return super.toDebugString(now) + ", confirmCredentials"

                            + ", " + account;
@@ -1563,9 +1587,11 @@ public class AccountManagerService 
        try {

            new Session(accounts, response, account.type, expectActivityLaunch,

                    true /* stripAuthTokenFromResult */) {

                @Override

                public void run() throws RemoteException {

                    mAuthenticator.updateCredentials(this, account, authTokenType, loginOptions);

                }

                @Override

                protected String toDebugString(long now) {

                    if (loginOptions != null) loginOptions.keySet();

                    return super.toDebugString(now) + ", updateCredentials"
@@ -1596,9 +1622,11 @@ public class AccountManagerService 
        try {

            new Session(accounts, response, accountType, expectActivityLaunch,

                    true /* stripAuthTokenFromResult */) {

                @Override

                public void run() throws RemoteException {

                    mAuthenticator.editProperties(this, mAccountType);

                }

                @Override

                protected String toDebugString(long now) {

                    return super.toDebugString(now) + ", editProperties"

                            + ", accountType " + accountType;
@@ -1624,6 +1652,7 @@ public class AccountManagerService 
            mFeatures = features;

        }



        @Override

        public void run() throws RemoteException {

            synchronized (mAccounts.cacheLock) {

                mAccountsOfType = getAccountsFromCacheLocked(mAccounts, mAccountType, mCallingUid,
@@ -1661,6 +1690,7 @@ public class AccountManagerService 
            }

        }



        @Override

        public void onResult(Bundle result) {

            mNumResults++;

            if (result == null) {
@@ -1699,6 +1729,7 @@ public class AccountManagerService 
        }





        @Override

        protected String toDebugString(long now) {

            return super.toDebugString(now) + ", getAccountsByTypeAndFeatures"

                    + ", " + (mFeatures != null ? TextUtils.join(",", mFeatures) : null);
@@ -2108,9 +2139,31 @@ public class AccountManagerService 
            }

        }



        @Override

        public void onResult(Bundle result) {

            mNumResults++;

            if (result != null && !TextUtils.isEmpty(result.getString(AccountManager.KEY_AUTHTOKEN))) {

            Intent intent = null;

            if (result != null

                    && (intent = result.getParcelable(AccountManager.KEY_INTENT)) != null) {

                /*

                 * The Authenticator API allows third party authenticators to

                 * supply arbitrary intents to other apps that they can run,

                 * this can be very bad when those apps are in the system like

                 * the System Settings.

                 */

                PackageManager pm = mContext.getPackageManager();

                ResolveInfo resolveInfo = pm.resolveActivity(intent, 0);

                int targetUid = resolveInfo.activityInfo.applicationInfo.uid;

                int authenticatorUid = Binder.getCallingUid();

                if (PackageManager.SIGNATURE_MATCH !=

                        pm.checkSignatures(authenticatorUid, targetUid)) {

                    throw new SecurityException(

                            "Activity to be started with KEY_INTENT must " +

                            "share Authenticator's signatures");

                }

            }

            if (result != null

                    && !TextUtils.isEmpty(result.getString(AccountManager.KEY_AUTHTOKEN))) {

                String accountName = result.getString(AccountManager.KEY_ACCOUNT_NAME);

                String accountType = result.getString(AccountManager.KEY_ACCOUNT_TYPE);

                if (!TextUtils.isEmpty(accountName) && !TextUtils.isEmpty(accountType)) {
@@ -2220,6 +2273,7 @@ public class AccountManagerService 
            super(looper);

        }



        @Override

        public void handleMessage(Message msg) {

            switch (msg.what) {

                case MESSAGE_TIMED_OUT: