diff options
6 files changed, 188 insertions, 18 deletions
diff --git a/core/java/android/os/PersistableBundle.java b/core/java/android/os/PersistableBundle.java index 02704f5b346b..236194d16ad8 100644 --- a/core/java/android/os/PersistableBundle.java +++ b/core/java/android/os/PersistableBundle.java @@ -294,6 +294,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 683bc3189c43..7a7c6676e18f 100644 --- a/core/java/android/os/UserManager.java +++ b/core/java/android/os/UserManager.java @@ -104,6 +104,21 @@ public class UserManager { /** Whether the device is in headless system user mode; null until cached. */ private static Boolean sIsHeadlessSystemUser = null; + /** 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; + /** * User type representing a {@link UserHandle#USER_SYSTEM system} user that is a human user. * This type of user cannot be created; it can only pre-exist on first boot. @@ -4205,15 +4220,15 @@ public class UserManager { * This API should only be called if the current user is an {@link #isAdminUser() admin} user, * as otherwise the returned intent will not be able to create a 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 0a28997885ff..b4e87498f09b 100644 --- a/core/java/com/android/internal/app/ConfirmUserCreationActivity.java +++ b/core/java/com/android/internal/app/ConfirmUserCreationActivity.java @@ -116,6 +116,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; @@ -144,4 +152,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 ffb4cb3aa127..03231472d51d 100644 --- a/services/core/java/com/android/server/pm/UserManagerService.java +++ b/services/core/java/com/android/server/pm/UserManagerService.java @@ -284,8 +284,6 @@ public class UserManagerService extends IUserManager.Stub { private static final int USER_VERSION = 11; - private static final int MAX_USER_STRING_LENGTH = 500; - private static final long EPOCH_PLUS_30_YEARS = 30L * 365 * 24 * 60 * 60 * 1000L; // ms static final int WRITE_USER_MSG = 1; @@ -4274,16 +4272,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) { @@ -4332,11 +4332,11 @@ public class UserManagerService extends IUserManager.Stub { serializer.endDocument(); } - 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); } /* @@ -4784,8 +4784,7 @@ public class UserManagerService extends IUserManager.Stub { @UserIdInt int parentId, boolean preCreate, @Nullable String[] disallowedPackages, @NonNull TimingsTraceAndSlog t, @Nullable Object token) throws UserManager.CheckedUserOperationException { - - String truncatedName = truncateString(name); + String truncatedName = truncateString(name, UserManager.MAX_USER_NAME_LENGTH); final UserTypeDetails userTypeDetails = mUserTypes.get(userType); if (userTypeDetails == null) { throwCheckedUserOperationException( @@ -6386,9 +6385,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) { diff --git a/services/tests/mockingservicestests/src/com/android/server/pm/UserManagerServiceTest.java b/services/tests/mockingservicestests/src/com/android/server/pm/UserManagerServiceTest.java index c8ff758c014c..5f18e9e91ea1 100644 --- a/services/tests/mockingservicestests/src/com/android/server/pm/UserManagerServiceTest.java +++ b/services/tests/mockingservicestests/src/com/android/server/pm/UserManagerServiceTest.java @@ -331,7 +331,7 @@ public final class UserManagerServiceTest { @Test public void testCreateUserWithLongName_TruncatesName() { UserInfo user = mUms.createUserWithThrow(generateLongString(), USER_TYPE_FULL_SECONDARY, 0); - assertThat(user.name.length()).isEqualTo(500); + assertThat(user.name.length()).isEqualTo(UserManager.MAX_USER_NAME_LENGTH); UserInfo user1 = mUms.createUserWithThrow("Test", USER_TYPE_FULL_SECONDARY, 0); assertThat(user1.name.length()).isEqualTo(4); } diff --git a/services/tests/servicestests/src/com/android/server/pm/UserManagerTest.java b/services/tests/servicestests/src/com/android/server/pm/UserManagerTest.java index e511c79812ac..48f36f80dc01 100644 --- a/services/tests/servicestests/src/com/android/server/pm/UserManagerTest.java +++ b/services/tests/servicestests/src/com/android/server/pm/UserManagerTest.java @@ -19,6 +19,7 @@ package com.android.server.pm; import static com.google.common.truth.Truth.assertThat; import static com.google.common.truth.Truth.assertWithMessage; +import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; import static org.junit.Assume.assumeTrue; import static org.testng.Assert.assertEquals; @@ -33,6 +34,7 @@ import android.content.pm.UserProperties; import android.content.res.Resources; import android.graphics.drawable.Drawable; import android.os.Bundle; +import android.os.PersistableBundle; import android.os.UserHandle; import android.os.UserManager; import android.platform.test.annotations.Postsubmit; @@ -1420,6 +1422,106 @@ public final class UserManagerTest { assertThat(userInfo.name).isEqualTo(newName); } + @Test + public void testAddUserAccountData_validStringValuesAreSaved_validBundleIsSaved() { + assumeManagedUsersSupported(); + + String userName = "User"; + String accountName = "accountName"; + String accountType = "accountType"; + String arrayKey = "StringArrayKey"; + String stringKey = "StringKey"; + String intKey = "IntKey"; + String nestedBundleKey = "PersistableBundleKey"; + String value1 = "Value 1"; + String value2 = "Value 2"; + String value3 = "Value 3"; + + UserInfo userInfo = mUserManager.createUser(userName, + UserManager.USER_TYPE_FULL_SECONDARY, 0); + + PersistableBundle accountOptions = new PersistableBundle(); + String[] stringArray = {value1, value2}; + accountOptions.putInt(intKey, 1234); + PersistableBundle nested = new PersistableBundle(); + nested.putString(stringKey, value3); + accountOptions.putPersistableBundle(nestedBundleKey, nested); + accountOptions.putStringArray(arrayKey, stringArray); + + mUserManager.clearSeedAccountData(); + mUserManager.setSeedAccountData(mContext.getUserId(), accountName, + accountType, accountOptions); + + //assert userName accountName and accountType were saved correctly + assertTrue(mUserManager.getUserInfo(userInfo.id).name.equals(userName)); + assertTrue(mUserManager.getSeedAccountName().equals(accountName)); + assertTrue(mUserManager.getSeedAccountType().equals(accountType)); + + //assert bundle with correct values was added + assertThat(mUserManager.getSeedAccountOptions().containsKey(arrayKey)).isTrue(); + assertThat(mUserManager.getSeedAccountOptions().getPersistableBundle(nestedBundleKey) + .getString(stringKey)).isEqualTo(value3); + assertThat(mUserManager.getSeedAccountOptions().getStringArray(arrayKey)[0]) + .isEqualTo(value1); + + mUserManager.removeUser(userInfo.id); + } + + @Test + public void testAddUserAccountData_invalidStringValuesAreTruncated_invalidBundleIsDropped() { + assumeManagedUsersSupported(); + + String tooLongString = generateLongString(); + String userName = "User " + tooLongString; + String accountType = "Account Type " + tooLongString; + String accountName = "accountName " + tooLongString; + String arrayKey = "StringArrayKey"; + String stringKey = "StringKey"; + String intKey = "IntKey"; + String nestedBundleKey = "PersistableBundleKey"; + String value1 = "Value 1"; + String value2 = "Value 2"; + + UserInfo userInfo = mUserManager.createUser(userName, + UserManager.USER_TYPE_FULL_SECONDARY, 0); + + PersistableBundle accountOptions = new PersistableBundle(); + String[] stringArray = {value1, value2}; + accountOptions.putInt(intKey, 1234); + PersistableBundle nested = new PersistableBundle(); + nested.putString(stringKey, tooLongString); + accountOptions.putPersistableBundle(nestedBundleKey, nested); + accountOptions.putStringArray(arrayKey, stringArray); + mUserManager.clearSeedAccountData(); + mUserManager.setSeedAccountData(mContext.getUserId(), accountName, + accountType, accountOptions); + + //assert userName was truncated + assertTrue(mUserManager.getUserInfo(userInfo.id).name.length() + == UserManager.MAX_USER_NAME_LENGTH); + + //assert accountName and accountType got truncated + assertTrue(mUserManager.getSeedAccountName().length() + == UserManager.MAX_ACCOUNT_STRING_LENGTH); + assertTrue(mUserManager.getSeedAccountType().length() + == UserManager.MAX_ACCOUNT_STRING_LENGTH); + + //assert bundle with invalid values was dropped + assertThat(mUserManager.getSeedAccountOptions() == null).isTrue(); + + mUserManager.removeUser(userInfo.id); + } + + private String generateLongString() { + String partialString = "Test Name Test Name Test Name Test Name Test Name Test Name Test " + + "Name Test Name Test Name Test Name "; //String of length 100 + StringBuilder resultString = new StringBuilder(); + for (int i = 0; i < 600; i++) { + resultString.append(partialString); + } + return resultString.toString(); + } + private boolean isPackageInstalledForUser(String packageName, int userId) { try { return mPackageManager.getPackageInfoAsUser(packageName, 0, userId) != null; |