Git-Mirrors/DivestOS
1
0
Fork 0
mirror of https://github.com/Divested-Mobile/DivestOS-Build.git synced 2024-12-27 00:19:26 -05:00
Code Issues Packages Projects Releases Wiki Activity
dcc02bc9c9
DivestOS/Patches/LineageOS-14.1/android_frameworks_base/388831.patch
Tavi 2c9a3903e5
14.1: ASB Picks 
Signed-off-by: Tavi <tavi@divested.dev>
2024-04-06 15:43:21 -04:00

218 lines
11 KiB
Diff
Raw Blame History

From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
From: Tetiana Meronyk <tetianameronyk@google.com>
Date: Wed, 10 Jan 2024 16:25:13 +0000
Subject: [PATCH] Fix security vulnerability that creates user with no
restrictions when accountOptions are too long.
Bug: 293602970
Test: atest UserManagerTest#testAddUserAccountData_validStringValuesAreSaved_validBundleIsSaved && atest UserManagerTest#testAddUserAccountData_invalidStringValuesAreTruncated_invalidBundleIsDropped
(cherry picked from https://googleplex-android-review.googlesource.com/q/commit:944ea959ab8464c39a8f6a4fc391fb6953e1df89)
Merged-In: I23c971f671546ac085060add89485cfac6691ca3
Change-Id: I23c971f671546ac085060add89485cfac6691ca3
---
core/java/android/os/PersistableBundle.java | 37 +++++++++++++++++++
core/java/android/os/UserManager.java | 23 ++++++++++--
.../app/ConfirmUserCreationActivity.java | 12 ++++++
.../android/server/pm/UserManagerService.java | 29 +++++++++------
4 files changed, 85 insertions(+), 16 deletions(-)
diff --git a/core/java/android/os/PersistableBundle.java b/core/java/android/os/PersistableBundle.java
index c3e4b759b103..5c9e83a1d157 100644
--- a/core/java/android/os/PersistableBundle.java
+++ b/core/java/android/os/PersistableBundle.java
@@ -243,6 +243,43 @@ public final class PersistableBundle extends BaseBundle implements Cloneable, Pa
XmlUtils.writeMapXml(mMap, out, this);
}
+ /**
+ * Checks whether all keys and values are within the given character limit.
+ * Note: Maximum character limit of String that can be saved to XML as part of bundle is 65535.
+ * Otherwise IOException is thrown.
+ * @param limit length of String keys and values in the PersistableBundle, including nested
+ * PersistableBundles to check against.
+ *
+ * @hide
+ */
+ public boolean isBundleContentsWithinLengthLimit(int limit) {
+ unparcel();
+ if (mMap == null) {
+ return true;
+ }
+ for (int i = 0; i < mMap.size(); i++) {
+ if (mMap.keyAt(i) != null && mMap.keyAt(i).length() > limit) {
+ return false;
+ }
+ final Object value = mMap.valueAt(i);
+ if (value instanceof String && ((String) value).length() > limit) {
+ return false;
+ } else if (value instanceof String[]) {
+ String[] stringArray = (String[]) value;
+ for (int j = 0; j < stringArray.length; j++) {
+ if (stringArray[j] != null
+ && stringArray[j].length() > limit) {
+ return false;
+ }
+ }
+ } else if (value instanceof PersistableBundle
+ && !((PersistableBundle) value).isBundleContentsWithinLengthLimit(limit)) {
+ return false;
+ }
+ }
+ return true;
+ }
+
/** @hide */
static class MyReadMapCallback implements XmlUtils.ReadMapCallback {
@Override
diff --git a/core/java/android/os/UserManager.java b/core/java/android/os/UserManager.java
index cad5e9ff31a5..a0f457f452cc 100644
--- a/core/java/android/os/UserManager.java
+++ b/core/java/android/os/UserManager.java
@@ -66,6 +66,21 @@ public class UserManager {
private final IUserManager mService;
private final Context mContext;
+ /** Maximum length of username.
+ * @hide
+ */
+ public static final int MAX_USER_NAME_LENGTH = 100;
+
+ /** Maximum length of user property String value.
+ * @hide
+ */
+ public static final int MAX_ACCOUNT_STRING_LENGTH = 500;
+
+ /** Maximum length of account options String values.
+ * @hide
+ */
+ public static final int MAX_ACCOUNT_OPTIONS_LENGTH = 1000;
+
/**
* @hide
* No user restriction.
@@ -1382,15 +1397,15 @@ public class UserManager {
* time, the preferred user name and account information are used by the setup process for that
* user.
*
- * @param userName Optional name to assign to the user.
+ * @param userName Optional name to assign to the user. Character limit is 100.
* @param accountName Optional account name that will be used by the setup wizard to initialize
- * the user.
+ * the user. Character limit is 500.
* @param accountType Optional account type for the account to be created. This is required
- * if the account name is specified.
+ * if the account name is specified. Character limit is 500.
* @param accountOptions Optional bundle of data to be passed in during account creation in the
* new user via {@link AccountManager#addAccount(String, String, String[],
* Bundle, android.app.Activity, android.accounts.AccountManagerCallback,
- * Handler)}.
+ * Handler)}. Character limit is 1000.
* @return An Intent that can be launched from an Activity.
* @see #USER_CREATION_FAILED_NOT_PERMITTED
* @see #USER_CREATION_FAILED_NO_MORE_USERS
diff --git a/core/java/com/android/internal/app/ConfirmUserCreationActivity.java b/core/java/com/android/internal/app/ConfirmUserCreationActivity.java
index 03da9bc939ec..74dedc38a922 100644
--- a/core/java/com/android/internal/app/ConfirmUserCreationActivity.java
+++ b/core/java/com/android/internal/app/ConfirmUserCreationActivity.java
@@ -110,6 +110,14 @@ public class ConfirmUserCreationActivity extends AlertActivity
if (cantCreateUser) {
setResult(UserManager.USER_CREATION_FAILED_NOT_PERMITTED);
return null;
+ } else if (!(isUserPropertyWithinLimit(mUserName, UserManager.MAX_USER_NAME_LENGTH)
+ && isUserPropertyWithinLimit(mAccountName, UserManager.MAX_ACCOUNT_STRING_LENGTH)
+ && isUserPropertyWithinLimit(mAccountType, UserManager.MAX_ACCOUNT_STRING_LENGTH))
+ || (mAccountOptions != null && !mAccountOptions.isBundleContentsWithinLengthLimit(
+ UserManager.MAX_ACCOUNT_OPTIONS_LENGTH))) {
+ setResult(UserManager.USER_CREATION_FAILED_NOT_PERMITTED);
+ Log.i(TAG, "User properties must not exceed their character limits");
+ return null;
} else if (cantCreateAnyMoreUsers) {
setResult(UserManager.USER_CREATION_FAILED_NO_MORE_USERS);
return null;
@@ -137,4 +145,8 @@ public class ConfirmUserCreationActivity extends AlertActivity
}
finish();
}
+
+ private boolean isUserPropertyWithinLimit(String property, int limit) {
+ return property == null || property.length() <= limit;
+ }
}
diff --git a/services/core/java/com/android/server/pm/UserManagerService.java b/services/core/java/com/android/server/pm/UserManagerService.java
index d19a95a5e229..8d1072e0bea2 100644
--- a/services/core/java/com/android/server/pm/UserManagerService.java
+++ b/services/core/java/com/android/server/pm/UserManagerService.java
@@ -188,8 +188,6 @@ public class UserManagerService extends IUserManager.Stub {
private static final int USER_VERSION = 6;
- private static final int MAX_USER_STRING_LENGTH = 500;
-
private static final long EPOCH_PLUS_30_YEARS = 30L * 365 * 24 * 60 * 60 * 1000L; // ms
// Maximum number of managed profiles permitted per user is 1. This cannot be increased
@@ -1924,16 +1922,18 @@ public class UserManagerService extends IUserManager.Stub {
if (userData.persistSeedData) {
if (userData.seedAccountName != null) {
serializer.attribute(null, ATTR_SEED_ACCOUNT_NAME,
- truncateString(userData.seedAccountName));
+ truncateString(userData.seedAccountName,
+ UserManager.MAX_ACCOUNT_STRING_LENGTH));
}
if (userData.seedAccountType != null) {
serializer.attribute(null, ATTR_SEED_ACCOUNT_TYPE,
- truncateString(userData.seedAccountType));
+ truncateString(userData.seedAccountType,
+ UserManager.MAX_ACCOUNT_STRING_LENGTH));
}
}
if (userInfo.name != null) {
serializer.startTag(null, TAG_NAME);
- serializer.text(truncateString(userInfo.name));
+ serializer.text(truncateString(userInfo.name, UserManager.MAX_USER_NAME_LENGTH));
serializer.endTag(null, TAG_NAME);
}
synchronized (mRestrictionsLock) {
@@ -1965,11 +1965,11 @@ public class UserManagerService extends IUserManager.Stub {
}
}
- private String truncateString(String original) {
- if (original == null || original.length() <= MAX_USER_STRING_LENGTH) {
+ private String truncateString(String original, int limit) {
+ if (original == null || original.length() <= limit) {
return original;
}
- return original.substring(0, MAX_USER_STRING_LENGTH);
+ return original.substring(0, limit);
}
/*
@@ -2230,7 +2230,7 @@ public class UserManagerService extends IUserManager.Stub {
if (ActivityManager.isLowRamDeviceStatic()) {
return null;
}
- String truncatedName = truncateString(name);
+ String truncatedName = truncateString(name, UserManager.MAX_USER_NAME_LENGTH);
final boolean isGuest = (flags & UserInfo.FLAG_GUEST) != 0;
final boolean isManagedProfile = (flags & UserInfo.FLAG_MANAGED_PROFILE) != 0;
final boolean isRestricted = (flags & UserInfo.FLAG_RESTRICTED) != 0;
@@ -3107,9 +3107,14 @@ public class UserManagerService extends IUserManager.Stub {
Slog.e(LOG_TAG, "No such user for settings seed data u=" + userId);
return;
}
- userData.seedAccountName = truncateString(accountName);
- userData.seedAccountType = truncateString(accountType);
- userData.seedAccountOptions = accountOptions;
+ userData.seedAccountName = truncateString(accountName,
+ UserManager.MAX_ACCOUNT_STRING_LENGTH);
+ userData.seedAccountType = truncateString(accountType,
+ UserManager.MAX_ACCOUNT_STRING_LENGTH);
+ if (accountOptions != null && accountOptions.isBundleContentsWithinLengthLimit(
+ UserManager.MAX_ACCOUNT_OPTIONS_LENGTH)) {
+ userData.seedAccountOptions = accountOptions;
+ }
userData.persistSeedData = persist;
}
if (persist) {
Reference in New Issue View Git Blame Copy Permalink