Fix checkKeyIntentParceledCorrectly's bypass

The checkKeyIntentParceledCorrectly method was added in checkKeyIntent, which was originaly  only invoked when AccountManagerService deserializes the KEY_INTENT value as not NULL. However, due to the self-changing bundle technique in Parcel mismatch problems, the Intent value can change after reparceling; hence would bypass the added checkKeyIntentParceledCorrectly call.

This CL did the following:

- Ensure the checkKeyIntent method is also called when result.getParcelable(AccountManager.KEY_INTENT, Intent.class) == null.
- Migrate to the safer Bundle.getParcelable(String, Class<T>) API call
  in AccountManagerService.

Bug: 260567867
Bug: 262230405
Test: local test, see b/262230405
Test: atest CtsAccountManagerTestCases
Merged-In: I7b528f52c41767ae12731838fdd36aa26a8f3477
Change-Id: I7b528f52c41767ae12731838fdd36aa26a8f3477
This commit is contained in:
Hao Ke
2022-12-12 15:49:16 +00:00
parent 7b6b4fecb9
commit 3723f400e2

View File

@@ -3091,7 +3091,7 @@ public class AccountManagerService
} }
} }
Intent intent = result.getParcelable(AccountManager.KEY_INTENT); Intent intent = result.getParcelable(AccountManager.KEY_INTENT, Intent.class);
if (intent != null && notifyOnAuthFailure && !customTokens) { if (intent != null && notifyOnAuthFailure && !customTokens) {
/* /*
* Make sure that the supplied intent is owned by the authenticator * Make sure that the supplied intent is owned by the authenticator
@@ -3516,8 +3516,7 @@ public class AccountManagerService
Bundle.setDefusable(result, true); Bundle.setDefusable(result, true);
mNumResults++; mNumResults++;
Intent intent = null; Intent intent = null;
if (result != null if (result != null) {
&& (intent = result.getParcelable(AccountManager.KEY_INTENT)) != null) {
if (!checkKeyIntent( if (!checkKeyIntent(
Binder.getCallingUid(), Binder.getCallingUid(),
result)) { result)) {
@@ -4876,8 +4875,10 @@ public class AccountManagerService
EventLog.writeEvent(0x534e4554, "250588548", authUid, ""); EventLog.writeEvent(0x534e4554, "250588548", authUid, "");
return false; return false;
} }
Intent intent = bundle.getParcelable(AccountManager.KEY_INTENT, Intent.class); Intent intent = bundle.getParcelable(AccountManager.KEY_INTENT, Intent.class);
if (intent == null) {
return true;
}
// Explicitly set an empty ClipData to ensure that we don't offer to // Explicitly set an empty ClipData to ensure that we don't offer to
// promote any Uris contained inside for granting purposes // promote any Uris contained inside for granting purposes
if (intent.getClipData() == null) { if (intent.getClipData() == null) {
@@ -4927,8 +4928,12 @@ public class AccountManagerService
Bundle simulateBundle = p.readBundle(); Bundle simulateBundle = p.readBundle();
p.recycle(); p.recycle();
Intent intent = bundle.getParcelable(AccountManager.KEY_INTENT, Intent.class); Intent intent = bundle.getParcelable(AccountManager.KEY_INTENT, Intent.class);
return (intent.filterEquals(simulateBundle.getParcelable(AccountManager.KEY_INTENT, Intent simulateIntent = simulateBundle.getParcelable(AccountManager.KEY_INTENT,
Intent.class))); Intent.class);
if (intent == null) {
return (simulateIntent == null);
}
return intent.filterEquals(simulateIntent);
} }
private boolean isExportedSystemActivity(ActivityInfo activityInfo) { private boolean isExportedSystemActivity(ActivityInfo activityInfo) {
@@ -5073,8 +5078,7 @@ public class AccountManagerService
} }
} }
} }
if (result != null if (result != null) {
&& (intent = result.getParcelable(AccountManager.KEY_INTENT)) != null) {
if (!checkKeyIntent( if (!checkKeyIntent(
Binder.getCallingUid(), Binder.getCallingUid(),
result)) { result)) {