am c45bd7f2: Prevent authenticators from using Settings to launch arbitrary activities.

* commit 'c45bd7f20987ed2fd29b9192edb38d02b4caa23b':
  Prevent authenticators from using Settings to  launch arbitrary activities.

1 files changed, 55 insertions, 1 deletions

diff --git a/services/java/com/android/server/accounts/AccountManagerService.java b/services/java/com/android/server/accounts/AccountManagerService.java
index 0fbde37a20c5..18c7d2c9567c 100644
--- a/services/java/com/android/server/accounts/AccountManagerService.java
+++ b/services/java/com/android/server/accounts/AccountManagerService.java
@@ -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: