Truncate user data to a limit of 500 characters

Fix vulnerability that allows creating users with no restrictions. This is done by creating an intent to create a user and putting extras that are too long to be serialized. It causes IOException and the restrictions are not written in the file.

By truncating the string values when writing them to the file, we ensure that the exception does not happen and it can be recorded correctly.

Bug: 293602317
Test: install app provided in the bug, open app and click add. Check logcat to see there is no more IOException. Reboot the device by either opening User details page or running adb shell dumpsys user | grep -A12 heen and see that the restrictions are in place.
Change-Id: I633dc10974a64ef2abd07e67ff2d209847129989
Merged-In: I633dc10974a64ef2abd07e67ff2d209847129989

2 files changed, 41 insertions, 8 deletions

diff --git a/services/core/java/com/android/server/pm/UserManagerService.java b/services/core/java/com/android/server/pm/UserManagerService.java
index 7e88e13e1788..b03fb31fbb1e 100644
--- a/services/core/java/com/android/server/pm/UserManagerService.java
+++ b/services/core/java/com/android/server/pm/UserManagerService.java
@@ -284,6 +284,8 @@ 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;
@@ -4256,15 +4258,17 @@ public class UserManagerService extends IUserManager.Stub {
         // Write seed data
         if (userData.persistSeedData) {
             if (userData.seedAccountName != null) {
-                serializer.attribute(null, ATTR_SEED_ACCOUNT_NAME, userData.seedAccountName);
+                serializer.attribute(null, ATTR_SEED_ACCOUNT_NAME,
+                        truncateString(userData.seedAccountName));
             }
             if (userData.seedAccountType != null) {
-                serializer.attribute(null, ATTR_SEED_ACCOUNT_TYPE, userData.seedAccountType);
+                serializer.attribute(null, ATTR_SEED_ACCOUNT_TYPE,
+                        truncateString(userData.seedAccountType));
             }
         }
         if (userInfo.name != null) {
             serializer.startTag(null, TAG_NAME);
-            serializer.text(userInfo.name);
+            serializer.text(truncateString(userInfo.name));
             serializer.endTag(null, TAG_NAME);
         }
         synchronized (mRestrictionsLock) {
@@ -4313,6 +4317,13 @@ public class UserManagerService extends IUserManager.Stub {
         serializer.endDocument();
     }
 
+    private String truncateString(String original) {
+        if (original == null || original.length() <= MAX_USER_STRING_LENGTH) {
+            return original;
+        }
+        return original.substring(0, MAX_USER_STRING_LENGTH);
+    }
+
     /*
      * Writes the user list file in this format:
      *
@@ -4749,6 +4760,7 @@ public class UserManagerService extends IUserManager.Stub {
             @NonNull TimingsTraceAndSlog t, @Nullable Object token)
             throws UserManager.CheckedUserOperationException {
 
+        String truncatedName = truncateString(name);
         final UserTypeDetails userTypeDetails = mUserTypes.get(userType);
         if (userTypeDetails == null) {
             throwCheckedUserOperationException(
@@ -4783,8 +4795,8 @@ public class UserManagerService extends IUserManager.Stub {
 
         // Try to use a pre-created user (if available).
         if (!preCreate && parentId < 0 && isUserTypeEligibleForPreCreation(userTypeDetails)) {
-            final UserInfo preCreatedUser = convertPreCreatedUserIfPossible(userType, flags, name,
-                    token);
+            final UserInfo preCreatedUser = convertPreCreatedUserIfPossible(userType, flags,
+                    truncatedName, token);
             if (preCreatedUser != null) {
                 return preCreatedUser;
             }
@@ -4871,7 +4883,7 @@ public class UserManagerService extends IUserManager.Stub {
                         flags |= UserInfo.FLAG_EPHEMERAL_ON_CREATE;
                     }
 
-                    userInfo = new UserInfo(userId, name, null, flags, userType);
+                    userInfo = new UserInfo(userId, truncatedName, null, flags, userType);
                     userInfo.serialNumber = mNextSerialNumber++;
                     userInfo.creationTime = getCreationTime();
                     userInfo.partial = true;
@@ -6349,8 +6361,8 @@ public class UserManagerService extends IUserManager.Stub {
                     Slog.e(LOG_TAG, "No such user for settings seed data u=" + userId);
                     return;
                 }
-                userData.seedAccountName = accountName;
-                userData.seedAccountType = accountType;
+                userData.seedAccountName = truncateString(accountName);
+                userData.seedAccountType = truncateString(accountType);
                 userData.seedAccountOptions = accountOptions;
                 userData.persistSeedData = 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 e7b3e6f88dce..c8ff758c014c 100644
--- a/services/tests/mockingservicestests/src/com/android/server/pm/UserManagerServiceTest.java
+++ b/services/tests/mockingservicestests/src/com/android/server/pm/UserManagerServiceTest.java
@@ -15,9 +15,12 @@
  */
 package com.android.server.pm;
 
+import static android.os.UserManager.USER_TYPE_FULL_SECONDARY;
+
 import static com.android.dx.mockito.inline.extended.ExtendedMockito.doNothing;
 import static com.android.dx.mockito.inline.extended.ExtendedMockito.doReturn;
 
+import static com.google.common.truth.Truth.assertThat;
 import static com.google.common.truth.Truth.assertWithMessage;
 
 import static org.junit.Assert.assertThrows;
@@ -325,6 +328,24 @@ public final class UserManagerServiceTest {
                 () -> mUmi.getBootUser(/* waitUntilSet= */ false));
     }
 
+    @Test
+    public void testCreateUserWithLongName_TruncatesName() {
+        UserInfo user = mUms.createUserWithThrow(generateLongString(), USER_TYPE_FULL_SECONDARY, 0);
+        assertThat(user.name.length()).isEqualTo(500);
+        UserInfo user1 = mUms.createUserWithThrow("Test", USER_TYPE_FULL_SECONDARY, 0);
+        assertThat(user1.name.length()).isEqualTo(4);
+    }
+
+    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 < 660; i++) {
+            resultString.append(partialString);
+        }
+        return resultString.toString();
+    }
+
     private void mockCurrentUser(@UserIdInt int userId) {
         mockGetLocalService(ActivityManagerInternal.class, mActivityManagerInternal);