From 211f81afe632cc7b5e085adfb3d7949d80e0b810 Mon Sep 17 00:00:00 2001 From: Josh Hou Date: Mon, 9 Jan 2023 18:18:11 +0800 Subject: [PATCH 1/2] Fix the security vulnerability issue in AppLocalePickerActivity Examine whether the packages is allowed to display app locales list when creating the AppLocalePickerActivity, and examine whether the target user is the same as the calling user. Bug: 257954050 Test: Follows the test step listed in b/257954050#comment14 Change-Id: I2e25a308bcba6ea0edee89c7a78465f766bdbeac Merged-In: I2e25a308bcba6ea0edee89c7a78465f766bdbeac (cherry picked from commit 5d7d1665fecd9f0dba90ccf5e827d3a295dafdcc) Merged-In: I2e25a308bcba6ea0edee89c7a78465f766bdbeac --- .../localepicker/AppLocalePickerActivity.java | 19 +- .../AppLocalePickerActivityTest.java | 172 +++++++++++++++++- 2 files changed, 183 insertions(+), 8 deletions(-) diff --git a/src/com/android/settings/localepicker/AppLocalePickerActivity.java b/src/com/android/settings/localepicker/AppLocalePickerActivity.java index b7fef30a58e..691344da60f 100644 --- a/src/com/android/settings/localepicker/AppLocalePickerActivity.java +++ b/src/com/android/settings/localepicker/AppLocalePickerActivity.java @@ -19,6 +19,7 @@ package com.android.settings.localepicker; import android.app.FragmentTransaction; import android.app.LocaleManager; import android.content.Context; +import android.content.pm.PackageManager; import android.net.Uri; import android.os.Bundle; import android.os.LocaleList; @@ -34,6 +35,7 @@ import com.android.internal.app.LocalePickerWithRegion; import com.android.internal.app.LocaleStore; import com.android.settings.R; import com.android.settings.applications.AppInfoBase; +import com.android.settings.applications.AppLocaleUtil; import com.android.settings.applications.appinfo.AppLocaleDetails; import com.android.settings.core.SettingsBaseActivity; @@ -64,12 +66,17 @@ public class AppLocalePickerActivity extends SettingsBaseActivity } mContextAsUser = this; if (getIntent().hasExtra(AppInfoBase.ARG_PACKAGE_UID)) { - int userId = getIntent().getIntExtra(AppInfoBase.ARG_PACKAGE_UID, -1); - if (userId != -1) { - UserHandle userHandle = UserHandle.getUserHandleForUid(userId); + int uid = getIntent().getIntExtra(AppInfoBase.ARG_PACKAGE_UID, -1); + if (uid != -1) { + UserHandle userHandle = UserHandle.getUserHandleForUid(uid); mContextAsUser = createContextAsUser(userHandle, 0); } } + if (!canDisplayLocaleUi() || mContextAsUser.getUserId() != UserHandle.myUserId()) { + Log.w(TAG, "Not allow to display Locale Settings UI."); + finish(); + return; + } setTitle(R.string.app_locale_picker_title); getActionBar().setDisplayHomeAsUpEnabled(true); @@ -160,4 +167,10 @@ public class AppLocalePickerActivity extends SettingsBaseActivity .replace(R.id.content_frame, mLocalePickerWithRegion) .commit(); } + + private boolean canDisplayLocaleUi() { + return AppLocaleUtil.canDisplayLocaleUi(mContextAsUser, mPackageName, + mContextAsUser.getPackageManager().queryIntentActivities( + AppLocaleUtil.LAUNCHER_ENTRY_INTENT, PackageManager.GET_META_DATA)); + } } diff --git a/tests/robotests/src/com/android/settings/localepicker/AppLocalePickerActivityTest.java b/tests/robotests/src/com/android/settings/localepicker/AppLocalePickerActivityTest.java index 332a39b3436..77b2abb76dc 100644 --- a/tests/robotests/src/com/android/settings/localepicker/AppLocalePickerActivityTest.java +++ b/tests/robotests/src/com/android/settings/localepicker/AppLocalePickerActivityTest.java @@ -18,25 +18,35 @@ package com.android.settings.localepicker; import static com.google.common.truth.Truth.assertThat; +import static org.mockito.Mockito.spy; import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; +import android.annotation.Nullable; import android.app.Activity; import android.app.ApplicationPackageManager; +import android.app.LocaleConfig; import android.content.Context; import android.content.Intent; +import android.content.pm.ApplicationInfo; import android.content.pm.InstallSourceInfo; +import android.content.pm.PackageInfo; +import android.content.pm.ResolveInfo; +import android.content.res.Resources; import android.net.Uri; +import android.os.LocaleList; import android.os.Process; import android.os.UserHandle; import android.telephony.TelephonyManager; +import androidx.annotation.ArrayRes; + import com.android.internal.app.LocaleStore; import com.android.settings.applications.AppInfoBase; -import java.util.Locale; - +import org.junit.After; +import org.junit.Before; import org.junit.Rule; import org.junit.Test; import org.junit.runner.RunWith; @@ -45,17 +55,26 @@ import org.mockito.junit.MockitoJUnit; import org.mockito.junit.MockitoRule; import org.robolectric.Robolectric; import org.robolectric.RobolectricTestRunner; +import org.robolectric.RuntimeEnvironment; import org.robolectric.Shadows; import org.robolectric.android.controller.ActivityController; import org.robolectric.annotation.Config; import org.robolectric.annotation.Implementation; import org.robolectric.annotation.Implements; +import org.robolectric.shadows.ShadowPackageManager; import org.robolectric.shadows.ShadowTelephonyManager; +import java.util.ArrayList; +import java.util.List; +import java.util.Locale; + @RunWith(RobolectricTestRunner.class) @Config( shadows = { AppLocalePickerActivityTest.ShadowApplicationPackageManager.class, + AppLocalePickerActivityTest.ShadowResources.class, + AppLocalePickerActivityTest.ShadowUserHandle.class, + AppLocalePickerActivityTest.ShadowLocaleConfig.class, }) public class AppLocalePickerActivityTest { private static final String TEST_PACKAGE_NAME = "com.android.settings"; @@ -67,21 +86,99 @@ public class AppLocalePickerActivityTest { @Rule public MockitoRule rule = MockitoJUnit.rule(); + private Context mContext; + private ShadowPackageManager mPackageManager; + + @Before + public void setUp() { + mContext = spy(RuntimeEnvironment.application); + mPackageManager = Shadows.shadowOf(mContext.getPackageManager()); + } + + @After + public void tearDown() { + mPackageManager.removePackage(TEST_PACKAGE_NAME); + ShadowResources.setDisAllowPackage(false); + ShadowApplicationPackageManager.setNoLaunchEntry(false); + ShadowUserHandle.setUserId(0); + ShadowLocaleConfig.setStatus(LocaleConfig.STATUS_SUCCESS); + } + @Test public void launchAppLocalePickerActivity_hasPackageName_success() { ActivityController controller = initActivityController(true); - controller.create(); assertThat(controller.get().isFinishing()).isFalse(); } + @Test + public void launchAppLocalePickerActivity_appNoLocaleConfig_failed() { + ShadowLocaleConfig.setStatus(LocaleConfig.STATUS_NOT_SPECIFIED); + + ActivityController controller = + initActivityController(true); + controller.create(); + + assertThat(controller.get().isFinishing()).isTrue(); + } + + @Test + public void launchAppLocalePickerActivity_appSignPlatformKey_failed() { + final ApplicationInfo applicationInfo = new ApplicationInfo(); + applicationInfo.privateFlags |= ApplicationInfo.PRIVATE_FLAG_SIGNED_WITH_PLATFORM_KEY; + applicationInfo.packageName = TEST_PACKAGE_NAME; + + final PackageInfo packageInfo = new PackageInfo(); + packageInfo.packageName = TEST_PACKAGE_NAME; + packageInfo.applicationInfo = applicationInfo; + mPackageManager.installPackage(packageInfo); + + ActivityController controller = + initActivityController(true); + controller.create(); + + assertThat(controller.get().isFinishing()).isTrue(); + } + + @Test + public void launchAppLocalePickerActivity_appMatchDisallowedPackage_failed() { + ShadowResources.setDisAllowPackage(true); + + ActivityController controller = + initActivityController(true); + controller.create(); + + assertThat(controller.get().isFinishing()).isTrue(); + } + + @Test + public void launchAppLocalePickerActivity_appNoLaunchEntry_failed() { + ShadowApplicationPackageManager.setNoLaunchEntry(true); + + ActivityController controller = + initActivityController(true); + controller.create(); + + assertThat(controller.get().isFinishing()).isTrue(); + } + + @Test + public void launchAppLocalePickerActivity_modifyAppLocalesOfAnotherUser_failed() { + ShadowUserHandle.setUserId(10); + + ActivityController controller = + initActivityController(true); + controller.create(); + + assertThat(controller.get().isFinishing()).isTrue(); + } + @Test public void launchAppLocalePickerActivity_intentWithoutPackageName_failed() { ActivityController controller = initActivityController(false); - controller.create(); assertThat(controller.get().isFinishing()).isTrue(); @@ -125,7 +222,7 @@ public class AppLocalePickerActivityTest { if (hasPackageName) { data.setData(TEST_PACKAGE_URI); } - data.putExtra(AppInfoBase.ARG_PACKAGE_UID, UserHandle.getUserId(Process.myUid())); + data.putExtra(AppInfoBase.ARG_PACKAGE_UID, Process.myUid()); ActivityController activityController = Robolectric.buildActivity(TestAppLocalePickerActivity.class, data); Activity activity = activityController.get(); @@ -149,10 +246,75 @@ public class AppLocalePickerActivityTest { @Implements(ApplicationPackageManager.class) public static class ShadowApplicationPackageManager extends org.robolectric.shadows.ShadowApplicationPackageManager { + private static boolean sNoLaunchEntry = false; @Implementation protected Object getInstallSourceInfo(String packageName) { return new InstallSourceInfo("", null, null, ""); } + + @Implementation + protected List queryIntentActivities(Intent intent, int flags) { + if (sNoLaunchEntry) { + return new ArrayList(); + } else { + return super.queryIntentActivities(intent, flags); + } + } + + private static void setNoLaunchEntry(boolean noLaunchEntry) { + sNoLaunchEntry = noLaunchEntry; + } + } + + @Implements(Resources.class) + public static class ShadowResources extends + org.robolectric.shadows.ShadowResources { + private static boolean sDisAllowPackage = false; + + @Implementation + public String[] getStringArray(@ArrayRes int id) { + if (sDisAllowPackage) { + return new String[]{TEST_PACKAGE_NAME}; + } else { + return new String[0]; + } + } + + private static void setDisAllowPackage(boolean disAllowPackage) { + sDisAllowPackage = disAllowPackage; + } + } + + @Implements(UserHandle.class) + public static class ShadowUserHandle { + private static int sUserId = 0; + private static void setUserId(int userId) { + sUserId = userId; + } + + @Implementation + public static int getUserId(int userId) { + return sUserId; + } + } + + @Implements(LocaleConfig.class) + public static class ShadowLocaleConfig { + private static int sStatus = 0; + + @Implementation + public @Nullable LocaleList getSupportedLocales() { + return LocaleList.forLanguageTags("en-US"); + } + + private static void setStatus(@LocaleConfig.Status int status) { + sStatus = status; + } + + @Implementation + public @LocaleConfig.Status int getStatus() { + return sStatus; + } } } From 78b42aeb37c04c0da0fe07f46c2558f15a7efc6b Mon Sep 17 00:00:00 2001 From: Jack Yu Date: Thu, 28 Jul 2022 19:42:27 +0800 Subject: [PATCH 2/2] Only primary user is allowed to control secure nfc Bug: 238298970 Test: manual Merged-In: I945490ef1e62af479a732c9a260ed94bdd8bc313 Change-Id: I945490ef1e62af479a732c9a260ed94bdd8bc313 (cherry picked from commit 0e57ff90cdae3575c243d21d490e2b6384d33397) Merged-In: I945490ef1e62af479a732c9a260ed94bdd8bc313 --- src/com/android/settings/nfc/SecureNfcEnabler.java | 2 +- src/com/android/settings/nfc/SecureNfcPreferenceController.java | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/com/android/settings/nfc/SecureNfcEnabler.java b/src/com/android/settings/nfc/SecureNfcEnabler.java index f31a382a571..ad5c4ab7e83 100644 --- a/src/com/android/settings/nfc/SecureNfcEnabler.java +++ b/src/com/android/settings/nfc/SecureNfcEnabler.java @@ -61,7 +61,7 @@ public class SecureNfcEnabler extends BaseNfcEnabler { } private boolean isToggleable() { - if (mUserManager.isGuestUser()) { + if (!mUserManager.isPrimaryUser()) { return false; } return true; diff --git a/src/com/android/settings/nfc/SecureNfcPreferenceController.java b/src/com/android/settings/nfc/SecureNfcPreferenceController.java index cf43ec30ded..460eca3c142 100644 --- a/src/com/android/settings/nfc/SecureNfcPreferenceController.java +++ b/src/com/android/settings/nfc/SecureNfcPreferenceController.java @@ -109,7 +109,7 @@ public class SecureNfcPreferenceController extends TogglePreferenceController } private boolean isToggleable() { - if (mUserManager.isGuestUser()) { + if (!mUserManager.isPrimaryUser()) { return false; } return true;