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
diff --git a/services/java/com/android/server/accounts/AccountManagerService.java b/services/java/com/android/server/accounts/AccountManagerService.java index 2145b76..3a3dfd5 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.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; @@ -580,15 +581,18 @@ 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)) { @@ -613,11 +617,13 @@ 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); @@ -632,6 +638,7 @@ } } + @Override public void onResult(Bundle result) { if (result != null) { if (result.getBoolean(AccountManager.KEY_BOOLEAN_RESULT, false)) { @@ -646,6 +653,7 @@ } } + @Override public void onError(int errorCode, String errorMessage) { super.onError(errorCode, errorMessage); // TODO: Show error notification to user @@ -774,6 +782,7 @@ mAccount = account; } + @Override public void run() throws RemoteException { try { mAuthenticator.hasFeatures(this, mAccount, mFeatures); @@ -782,6 +791,7 @@ } } + @Override public void onResult(Bundle result) { IAccountManagerResponse response = getResponseAndClose(); if (response != null) { @@ -807,6 +817,7 @@ } } + @Override protected String toDebugString(long now) { return super.toDebugString(now) + ", hasFeatures" + ", " + mAccount @@ -863,15 +874,18 @@ 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)) { @@ -1212,16 +1226,19 @@ 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); @@ -1299,6 +1316,7 @@ 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" @@ -1308,6 +1326,7 @@ + ", 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. @@ -1318,6 +1337,7 @@ } } + @Override public void onResult(Bundle result) { if (result != null) { if (result.containsKey(AccountManager.KEY_AUTH_TOKEN_LABEL)) { @@ -1482,11 +1502,13 @@ 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 @@ 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 @@ 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 @@ 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 @@ mFeatures = features; } + @Override public void run() throws RemoteException { synchronized (mAccounts.cacheLock) { mAccountsOfType = getAccountsFromCacheLocked(mAccounts, mAccountType, mCallingUid, @@ -1661,6 +1690,7 @@ } } + @Override public void onResult(Bundle result) { mNumResults++; if (result == null) { @@ -1699,6 +1729,7 @@ } + @Override protected String toDebugString(long now) { return super.toDebugString(now) + ", getAccountsByTypeAndFeatures" + ", " + (mFeatures != null ? TextUtils.join(",", mFeatures) : null); @@ -2111,9 +2142,31 @@ } } + @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)) { @@ -2223,6 +2276,7 @@ super(looper); } + @Override public void handleMessage(Message msg) { switch (msg.what) { case MESSAGE_TIMED_OUT: