summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
author Hao Ke <haok@google.com> 2022-10-05 20:05:33 +0000
committer Automerger Merge Worker <android-build-automerger-merge-worker@system.gserviceaccount.com> 2022-10-05 20:05:33 +0000
commitac02d51ea3301672f731ec5115a21e701be91fd7 (patch)
treecab54601d495b1f7870af70c9bed6ceb239a10ae
parent886fab1952816466e6f7c3942596290e2b68d579 (diff)
parent64e4cbc1d84c8c710dd10d01064db4150004842c (diff)
Merge "Add safety checks on KEY_INTENT mismatch." into qt-dev am: 459808b2c0 am: 64e4cbc1d8
Original change: https://googleplex-android-review.googlesource.com/c/platform/frameworks/base/+/20105519 Change-Id: I8ebe172f9d76d5b21e3beb0f99b36189511fc71e Signed-off-by: Automerger Merge Worker <android-build-automerger-merge-worker@system.gserviceaccount.com>
-rw-r--r--services/core/java/com/android/server/accounts/AccountManagerService.java34
1 files changed, 30 insertions, 4 deletions
diff --git a/services/core/java/com/android/server/accounts/AccountManagerService.java b/services/core/java/com/android/server/accounts/AccountManagerService.java
index 4c320e5bda2f..2e8dd9cd5881 100644
--- a/services/core/java/com/android/server/accounts/AccountManagerService.java
+++ b/services/core/java/com/android/server/accounts/AccountManagerService.java
@@ -87,6 +87,7 @@ import android.os.SystemClock;
import android.os.UserHandle;
import android.os.UserManager;
import android.text.TextUtils;
+import android.util.EventLog;
import android.util.Log;
import android.util.Pair;
import android.util.Slog;
@@ -3022,7 +3023,7 @@ public class AccountManagerService
*/
if (!checkKeyIntent(
Binder.getCallingUid(),
- intent)) {
+ result)) {
onError(AccountManager.ERROR_CODE_INVALID_RESPONSE,
"invalid intent in bundle returned");
return;
@@ -3432,7 +3433,7 @@ public class AccountManagerService
&& (intent = result.getParcelable(AccountManager.KEY_INTENT)) != null) {
if (!checkKeyIntent(
Binder.getCallingUid(),
- intent)) {
+ result)) {
onError(AccountManager.ERROR_CODE_INVALID_RESPONSE,
"invalid intent in bundle returned");
return;
@@ -4783,7 +4784,13 @@ public class AccountManagerService
* into launching arbitrary intents on the device via by tricking to click authenticator
* supplied entries in the system Settings app.
*/
- protected boolean checkKeyIntent(int authUid, Intent intent) {
+ protected boolean checkKeyIntent(int authUid, Bundle bundle) {
+ if (!checkKeyIntentParceledCorrectly(bundle)) {
+ EventLog.writeEvent(0x534e4554, "250588548", authUid, "");
+ return false;
+ }
+
+ Intent intent = bundle.getParcelable(AccountManager.KEY_INTENT);
// Explicitly set an empty ClipData to ensure that we don't offer to
// promote any Uris contained inside for granting purposes
if (intent.getClipData() == null) {
@@ -4820,6 +4827,25 @@ public class AccountManagerService
}
}
+ /**
+ * Simulate the client side's deserialization of KEY_INTENT value, to make sure they don't
+ * violate our security policy.
+ *
+ * In particular we want to make sure the Authenticator doesn't trick users
+ * into launching arbitrary intents on the device via exploiting any other Parcel read/write
+ * mismatch problems.
+ */
+ private boolean checkKeyIntentParceledCorrectly(Bundle bundle) {
+ Parcel p = Parcel.obtain();
+ p.writeBundle(bundle);
+ p.setDataPosition(0);
+ Bundle simulateBundle = p.readBundle();
+ p.recycle();
+ Intent intent = bundle.getParcelable(AccountManager.KEY_INTENT);
+ Intent simulateIntent = simulateBundle.getParcelable(AccountManager.KEY_INTENT);
+ return (intent.filterEquals(simulateIntent));
+ }
+
private boolean isExportedSystemActivity(ActivityInfo activityInfo) {
String className = activityInfo.name;
return "android".equals(activityInfo.packageName) &&
@@ -4966,7 +4992,7 @@ public class AccountManagerService
&& (intent = result.getParcelable(AccountManager.KEY_INTENT)) != null) {
if (!checkKeyIntent(
Binder.getCallingUid(),
- intent)) {
+ result)) {
onError(AccountManager.ERROR_CODE_INVALID_RESPONSE,
"invalid intent in bundle returned");
return;