Bug 1351739 - Part 2 - Convert CustomTabsActivity to SafeIntents. r=sebastian,walkingice

These are potentially untrusted external intents, so we should use SafeIntents for interacting with them.

MozReview-Commit-ID: 3nmjg85wbr1

--HG--
extra : rebase_source : 97c54e1dee8fc132aa3bb1d9c42051a4eb330018

mobile/android/base/java/org/mozilla/gecko/customtabs/CustomTabsActivity.java

@@ -43,6 +43,7 @@ import org.mozilla.gecko.gfx.DynamicToolbarAnimator;
 import org.mozilla.gecko.gfx.DynamicToolbarAnimator.PinReason;
 import org.mozilla.gecko.menu.GeckoMenu;
 import org.mozilla.gecko.menu.GeckoMenuInflater;
+import org.mozilla.gecko.mozglue.SafeIntent;
 import org.mozilla.gecko.util.Clipboard;
 import org.mozilla.gecko.util.ColorUtil;
 import org.mozilla.gecko.util.GeckoBundle;
@@ -68,7 +69,7 @@ public class CustomTabsActivity extends GeckoApp implements Tabs.OnTabsChangedLi
 
     // Bug 1351605 - getIntent() not always returns the intent which started this activity.
     // Therefore we make a copy in case of this Activity is re-created.
-    private Intent startIntent;
+    private SafeIntent startIntent;
 
     private MenuItem menuItemControl;
 
@@ -77,10 +78,11 @@ public class CustomTabsActivity extends GeckoApp implements Tabs.OnTabsChangedLi
         super.onCreate(savedInstanceState);
 
         if (savedInstanceState != null) {
-            startIntent = savedInstanceState.getParcelable(SAVED_START_INTENT);
+            final Intent restoredIntent = savedInstanceState.getParcelable(SAVED_START_INTENT);
+            startIntent = new SafeIntent(restoredIntent);
         } else {
             sendTelemetry();
-            startIntent = getIntent();
+            startIntent = new SafeIntent(getIntent());
             final String host = getReferrerHost();
             recordCustomTabUsage(host);
         }
@@ -222,7 +224,7 @@ public class CustomTabsActivity extends GeckoApp implements Tabs.OnTabsChangedLi
     @Override
     protected void onSaveInstanceState(Bundle outState) {
         super.onSaveInstanceState(outState);
-        outState.putParcelable(SAVED_START_INTENT, startIntent);
+        outState.putParcelable(SAVED_START_INTENT, startIntent.getUnsafe());
     }
 
     @Override

mobile/android/base/java/org/mozilla/gecko/customtabs/IntentUtil.java

@@ -6,7 +6,6 @@
 package org.mozilla.gecko.customtabs;
 
 import android.app.PendingIntent;
-import android.content.Intent;
 import android.graphics.Bitmap;
 import android.os.Build;
 import android.os.Bundle;
@@ -15,6 +14,8 @@ import android.support.annotation.NonNull;
 import android.support.annotation.VisibleForTesting;
 import android.support.customtabs.CustomTabsIntent;
 
+import org.mozilla.gecko.mozglue.SafeIntent;
+
 import java.util.ArrayList;
 import java.util.List;
 
@@ -44,7 +45,7 @@ class IntentUtil {
      * @param intent which to launch a Custom-Tabs-Activity
      * @return true, if intent has all necessary information.
      */
-    static boolean hasActionButton(@NonNull Intent intent) {
+    static boolean hasActionButton(@NonNull SafeIntent intent) {
         return (getActionButtonBundle(intent) != null)
                 && (getActionButtonIcon(intent) != null)
                 && (getActionButtonDescription(intent) != null)
@@ -57,7 +58,7 @@ class IntentUtil {
      * @param intent which to launch a Custom-Tabs-Activity
      * @return true, if intent requires to add share action to menu item.
      */
-    static boolean hasShareItem(@NonNull Intent intent) {
+    static boolean hasShareItem(@NonNull SafeIntent intent) {
         return intent.getBooleanExtra(CustomTabsIntent.EXTRA_DEFAULT_SHARE_MENU_ITEM, false);
     }
 
@@ -67,7 +68,7 @@ class IntentUtil {
      * @param intent which to launch a Custom-Tabs-Activity
      * @return bitmap icon, if any. Otherwise, null.
      */
-    static Bitmap getActionButtonIcon(@NonNull Intent intent) {
+    static Bitmap getActionButtonIcon(@NonNull SafeIntent intent) {
         final Bundle bundle = getActionButtonBundle(intent);
         return (bundle == null) ? null : (Bitmap) bundle.getParcelable(CustomTabsIntent.KEY_ICON);
     }
@@ -79,7 +80,7 @@ class IntentUtil {
      * @param intent which to launch a Custom-Tabs-Activity
      * @return true, if the caller customized the color.
      */
-    static boolean hasToolbarColor(@NonNull Intent intent) {
+    static boolean hasToolbarColor(@NonNull SafeIntent intent) {
         return intent.hasExtra(CustomTabsIntent.EXTRA_TOOLBAR_COLOR);
     }
 
@@ -91,7 +92,7 @@ class IntentUtil {
      * @return color code in integer type.
      */
     @ColorInt
-    static int getToolbarColor(@NonNull Intent intent) {
+    static int getToolbarColor(@NonNull SafeIntent intent) {
         @ColorInt int toolbarColor = intent.getIntExtra(CustomTabsIntent.EXTRA_TOOLBAR_COLOR,
                 DEFAULT_ACTION_BAR_COLOR);
 
@@ -107,7 +108,7 @@ class IntentUtil {
      * @param intent which to launch a Custom-Tabs-Activity
      * @return description, if any. Otherwise, null.
      */
-    static String getActionButtonDescription(@NonNull Intent intent) {
+    static String getActionButtonDescription(@NonNull SafeIntent intent) {
         final Bundle bundle = getActionButtonBundle(intent);
         return (bundle == null) ? null : bundle.getString(CustomTabsIntent.KEY_DESCRIPTION);
     }
@@ -118,7 +119,7 @@ class IntentUtil {
      * @param intent which to launch a Custom-Tabs-Activity
      * @return PendingIntent, if any. Otherwise, null.
      */
-    static PendingIntent getActionButtonPendingIntent(@NonNull Intent intent) {
+    static PendingIntent getActionButtonPendingIntent(@NonNull SafeIntent intent) {
         final Bundle bundle = getActionButtonBundle(intent);
         return (bundle == null)
                 ? null
@@ -131,7 +132,7 @@ class IntentUtil {
      * @param intent which to launch a Custom-Tabs-Activity
      * @return true, if Action-Button should be tinted. Default value is false.
      */
-    static boolean isActionButtonTinted(@NonNull Intent intent) {
+    static boolean isActionButtonTinted(@NonNull SafeIntent intent) {
         return intent.getBooleanExtra(CustomTabsIntent.EXTRA_TINT_ACTION_BUTTON, false);
     }
 
@@ -141,7 +142,7 @@ class IntentUtil {
      * @param intent which to launch a Custom-Tabs-Activity
      * @return bundle for Action-Button, if any. Otherwise, null.
      */
-    private static Bundle getActionButtonBundle(@NonNull Intent intent) {
+    private static Bundle getActionButtonBundle(@NonNull SafeIntent intent) {
         return intent.getBundleExtra(CustomTabsIntent.EXTRA_ACTION_BUTTON_BUNDLE);
     }
 
@@ -153,7 +154,7 @@ class IntentUtil {
      * @param intent which to launch a Custom-Tabs-Activity
      * @return package name, if the intent defined extra exit-animation bundle. Otherwise, null.
      */
-    static String getAnimationPackageName(@NonNull Intent intent) {
+    static String getAnimationPackageName(@NonNull SafeIntent intent) {
         final Bundle bundle = getAnimationBundle(intent);
         return (bundle == null) ? null : bundle.getString(KEY_PACKAGE_NAME);
     }
@@ -166,7 +167,7 @@ class IntentUtil {
      * @param intent which to launch a Custom-Tabs-Activity
      * @return A list of string as title for each menu items
      */
-    static List<String> getMenuItemsTitle(@NonNull Intent intent) {
+    static List<String> getMenuItemsTitle(@NonNull SafeIntent intent) {
         final List<Bundle> bundles = getMenuItemsBundle(intent);
         final List<String> titles = new ArrayList<>();
         for (Bundle b : bundles) {
@@ -183,7 +184,7 @@ class IntentUtil {
      * @param intent which to launch a Custom-Tabs-Activity
      * @return A list of pending-intent for each menu items
      */
-    static List<PendingIntent> getMenuItemsPendingIntent(@NonNull Intent intent) {
+    static List<PendingIntent> getMenuItemsPendingIntent(@NonNull SafeIntent intent) {
         final List<Bundle> bundles = getMenuItemsBundle(intent);
         final List<PendingIntent> intents = new ArrayList<>();
         for (Bundle b : bundles) {
@@ -199,7 +200,7 @@ class IntentUtil {
      * @param intent which to launch a Custom-Tabs-Activity
      * @return true, if the intent has necessary information.
      */
-    static boolean hasExitAnimation(@NonNull Intent intent) {
+    static boolean hasExitAnimation(@NonNull SafeIntent intent) {
         final Bundle bundle = getAnimationBundle(intent);
         return (bundle != null)
                 && (getAnimationPackageName(intent) != null)
@@ -215,7 +216,7 @@ class IntentUtil {
      * @param intent which to launch a Custom-Tabs-Activity
      * @return animation resource id if any; otherwise, NO_ANIMATION_RESOURCE;
      */
-    static int getEnterAnimationRes(@NonNull Intent intent) {
+    static int getEnterAnimationRes(@NonNull SafeIntent intent) {
         final Bundle bundle = getAnimationBundle(intent);
         return (bundle == null)
                 ? NO_ANIMATION_RESOURCE
@@ -230,7 +231,7 @@ class IntentUtil {
      * @param intent which to launch a Custom-Tabs-Activity
      * @return animation resource id if any; otherwise, NO_ANIMATION_RESOURCE.
      */
-    static int getExitAnimationRes(@NonNull Intent intent) {
+    static int getExitAnimationRes(@NonNull SafeIntent intent) {
         final Bundle bundle = getAnimationBundle(intent);
         return (bundle == null)
                 ? NO_ANIMATION_RESOURCE
@@ -243,7 +244,7 @@ class IntentUtil {
      * @param intent which to launch a Custom-Tabs-Activity
      * @return bundle for extra exit-animation, if any. Otherwise, null.
      */
-    private static Bundle getAnimationBundle(@NonNull Intent intent) {
+    private static Bundle getAnimationBundle(@NonNull SafeIntent intent) {
         return intent.getBundleExtra(CustomTabsIntent.EXTRA_EXIT_ANIMATION_BUNDLE);
     }
 
@@ -255,8 +256,8 @@ class IntentUtil {
      * @param intent which to launch a Custom-Tabs-Activity
      * @return bundle for menu items, if any. Otherwise, an empty list.
      */
-    private static List<Bundle> getMenuItemsBundle(@NonNull Intent intent) {
-        ArrayList<Bundle> extra = intent.getParcelableArrayListExtra(
+    private static List<Bundle> getMenuItemsBundle(@NonNull SafeIntent intent) {
+        ArrayList<Bundle> extra = intent.getUnsafe().getParcelableArrayListExtra(
                 CustomTabsIntent.EXTRA_MENU_ITEMS);
         return (extra == null) ? new ArrayList<Bundle>() : extra;
     }

mobile/android/tests/background/junit4/src/org/mozilla/gecko/customtabs/TestCustomTabsActivity.java

@@ -22,6 +22,7 @@ import org.junit.runner.RunWith;
 import org.mockito.internal.util.reflection.Whitebox;
 import org.mozilla.gecko.R;
 import org.mozilla.gecko.background.testhelpers.TestRunner;
+import org.mozilla.gecko.mozglue.SafeIntent;
 import org.robolectric.RuntimeEnvironment;
 import org.robolectric.fakes.RoboMenu;
 
@@ -63,7 +64,7 @@ public class TestCustomTabsActivity {
     @Test
     public void testFinishWithoutCustomAnimation() {
         final CustomTabsIntent.Builder builder = new CustomTabsIntent.Builder();
-        final Intent i = builder.build().intent;
+        final SafeIntent i = new SafeIntent(builder.build().intent);
 
         Whitebox.setInternalState(spyActivity, "startIntent", i);
 
@@ -78,7 +79,7 @@ public class TestCustomTabsActivity {
     public void testFinish() {
         final CustomTabsIntent.Builder builder = new CustomTabsIntent.Builder();
         builder.setExitAnimations(spyContext, enterRes, exitRes);
-        final Intent i = builder.build().intent;
+        final SafeIntent i = new SafeIntent(builder.build().intent);
 
         Whitebox.setInternalState(spyActivity, "startIntent", i);
 
@@ -93,7 +94,7 @@ public class TestCustomTabsActivity {
     public void testGetPackageName() {
         final CustomTabsIntent.Builder builder = new CustomTabsIntent.Builder();
         builder.setExitAnimations(spyContext, enterRes, exitRes);
-        final Intent i = builder.build().intent;
+        final SafeIntent i = new SafeIntent(builder.build().intent);
 
         Whitebox.setInternalState(spyActivity, "usingCustomAnimation", true);
         Whitebox.setInternalState(spyActivity, "startIntent", i);

mobile/android/tests/background/junit4/src/org/mozilla/gecko/customtabs/TestIntentUtil.java

@@ -20,6 +20,7 @@ import org.junit.Test;
 import org.junit.runner.RunWith;
 import org.mozilla.gecko.R;
 import org.mozilla.gecko.background.testhelpers.TestRunner;
+import org.mozilla.gecko.mozglue.SafeIntent;
 import org.robolectric.RuntimeEnvironment;
 
 import java.util.List;
@@ -54,7 +55,7 @@ public class TestIntentUtil {
         final CustomTabsIntent.Builder builder = new CustomTabsIntent.Builder();
         builder.setActionButton(bitmap, description, pendingIntent, tinted);
 
-        Intent intent = builder.build().intent;
+        SafeIntent intent = new SafeIntent(builder.build().intent);
         Assert.assertTrue(IntentUtil.hasActionButton(intent));
         Assert.assertEquals(tinted, IntentUtil.isActionButtonTinted(intent));
         Assert.assertEquals(bitmap, IntentUtil.getActionButtonIcon(intent));
@@ -67,7 +68,7 @@ public class TestIntentUtil {
     public void testIntentWithoutActionButton() {
         final CustomTabsIntent.Builder builder = new CustomTabsIntent.Builder();
 
-        Intent intent = builder.build().intent;
+        SafeIntent intent = new SafeIntent(builder.build().intent);
         Assert.assertFalse(IntentUtil.hasActionButton(intent));
         Assert.assertFalse(IntentUtil.isActionButtonTinted(intent));
         Assert.assertNull(IntentUtil.getActionButtonIcon(intent));
@@ -82,7 +83,7 @@ public class TestIntentUtil {
 
         final CustomTabsIntent.Builder builder = new CustomTabsIntent.Builder();
         builder.setExitAnimations(spyContext, enterRes, exitRes);
-        final Intent i = builder.build().intent;
+        final SafeIntent i = new SafeIntent(builder.build().intent);
 
         Assert.assertEquals(true, IntentUtil.hasExitAnimation(i));
         Assert.assertEquals(THIRD_PARTY_PACKAGE_NAME, IntentUtil.getAnimationPackageName(i));
@@ -93,7 +94,7 @@ public class TestIntentUtil {
     @Test
     public void testIntentWithoutCustomAnimation() {
         final CustomTabsIntent.Builder builder = new CustomTabsIntent.Builder();
-        final Intent i = builder.build().intent;
+        final SafeIntent i = new SafeIntent(builder.build().intent);
 
         Assert.assertEquals(false, IntentUtil.hasExitAnimation(i));
         Assert.assertEquals(null, IntentUtil.getAnimationPackageName(i));
@@ -114,7 +115,7 @@ public class TestIntentUtil {
         builder.addMenuItem("Label 1", intent1);
         builder.addMenuItem("Label 2", intent2);
 
-        final Intent intent = builder.build().intent;
+        final SafeIntent intent = new SafeIntent(builder.build().intent);
         List<String> titles = IntentUtil.getMenuItemsTitle(intent);
         List<PendingIntent> intents = IntentUtil.getMenuItemsPendingIntent(intent);
         Assert.assertEquals(3, titles.size());
@@ -131,28 +132,28 @@ public class TestIntentUtil {
     public void testToolbarColor() {
         final CustomTabsIntent.Builder builder = new CustomTabsIntent.Builder();
 
-        Assert.assertEquals(IntentUtil.getToolbarColor(builder.build().intent),
+        Assert.assertEquals(IntentUtil.getToolbarColor(new SafeIntent(builder.build().intent)),
                 IntentUtil.DEFAULT_ACTION_BAR_COLOR);
 
         // Test red color
         builder.setToolbarColor(0xFF0000);
-        Assert.assertEquals(IntentUtil.getToolbarColor(builder.build().intent), 0xFFFF0000);
+        Assert.assertEquals(IntentUtil.getToolbarColor(new SafeIntent(builder.build().intent)), 0xFFFF0000);
         builder.setToolbarColor(0xFFFF0000);
-        Assert.assertEquals(IntentUtil.getToolbarColor(builder.build().intent), 0xFFFF0000);
+        Assert.assertEquals(IntentUtil.getToolbarColor(new SafeIntent(builder.build().intent)), 0xFFFF0000);
 
         // Test translucent green color, it should force alpha value to be 0xFF
         builder.setToolbarColor(0x0000FF00);
-        Assert.assertEquals(IntentUtil.getToolbarColor(builder.build().intent), 0xFF00FF00);
+        Assert.assertEquals(IntentUtil.getToolbarColor(new SafeIntent(builder.build().intent)), 0xFF00FF00);
     }
 
     @Test
     public void testMenuShareItem() {
         final CustomTabsIntent.Builder builderNoShareItem = new CustomTabsIntent.Builder();
-        Assert.assertFalse(IntentUtil.hasShareItem(builderNoShareItem.build().intent));
+        Assert.assertFalse(IntentUtil.hasShareItem(new SafeIntent(builderNoShareItem.build().intent)));
 
         final CustomTabsIntent.Builder builderHasShareItem = new CustomTabsIntent.Builder();
         builderHasShareItem.addDefaultShareMenuItem();
-        Assert.assertTrue(IntentUtil.hasShareItem(builderHasShareItem.build().intent));
+        Assert.assertTrue(IntentUtil.hasShareItem(new SafeIntent(builderHasShareItem.build().intent)));
     }
 
     private PendingIntent createPendingIntent(int reqCode, @Nullable String uri) {