diff options
6 files changed, 321 insertions, 62 deletions
diff --git a/core/java/android/content/ClipData.java b/core/java/android/content/ClipData.java index ff0bb25bbccc..cc57dc05d6b1 100644 --- a/core/java/android/content/ClipData.java +++ b/core/java/android/content/ClipData.java @@ -398,6 +398,7 @@ public class ClipData implements Parcelable { * Retrieve the raw Intent contained in this Item. */ public Intent getIntent() { + Intent.maybeMarkAsMissingCreatorToken(mIntent); return mIntent; } diff --git a/core/java/android/content/Intent.java b/core/java/android/content/Intent.java index 6fa5a9b82858..c054b79f617e 100644 --- a/core/java/android/content/Intent.java +++ b/core/java/android/content/Intent.java @@ -87,6 +87,7 @@ import android.util.AttributeSet; import android.util.Log; import android.util.proto.ProtoOutputStream; +import com.android.internal.annotations.VisibleForTesting; import com.android.internal.util.XmlUtils; import com.android.modules.expresslog.Counter; @@ -108,6 +109,7 @@ import java.util.Locale; import java.util.Objects; import java.util.Set; import java.util.TimeZone; +import java.util.function.Consumer; /** * An intent is an abstract description of an operation to be performed. It @@ -892,6 +894,20 @@ public class Intent implements Parcelable, Cloneable { public static void maybeMarkAsMissingCreatorToken(Object object) { if (object instanceof Intent intent) { maybeMarkAsMissingCreatorTokenInternal(intent); + } else if (object instanceof Parcelable[] parcelables) { + for (Parcelable p : parcelables) { + if (p instanceof Intent intent) { + maybeMarkAsMissingCreatorTokenInternal(intent); + } + } + } else if (object instanceof ArrayList parcelables) { + int N = parcelables.size(); + for (int i = 0; i < N; i++) { + Object p = parcelables.get(i); + if (p instanceof Intent intent) { + maybeMarkAsMissingCreatorTokenInternal(intent); + } + } } } @@ -12204,7 +12220,70 @@ public class Intent implements Parcelable, Cloneable { // Stores a creator token for an intent embedded as an extra intent in a top level intent, private IBinder mCreatorToken; // Stores all extra keys whose values are intents for a top level intent. - private ArraySet<String> mExtraIntentKeys; + private ArraySet<NestedIntentKey> mNestedIntentKeys; + } + + /** + * @hide + */ + public static class NestedIntentKey { + /** @hide */ + @IntDef(flag = true, prefix = {"NESTED_INTENT_KEY_TYPE"}, value = { + NESTED_INTENT_KEY_TYPE_EXTRA_PARCEL, + NESTED_INTENT_KEY_TYPE_EXTRA_PARCEL_ARRAY, + NESTED_INTENT_KEY_TYPE_EXTRA_PARCEL_LIST, + NESTED_INTENT_KEY_TYPE_CLIP_DATA, + }) + @Retention(RetentionPolicy.SOURCE) + private @interface NestedIntentKeyType { + } + + /** + * This flag indicates the key is for an extra parcel in mExtras. + */ + private static final int NESTED_INTENT_KEY_TYPE_EXTRA_PARCEL = 1 << 0; + + /** + * This flag indicates the key is for an extra parcel array in mExtras and the index is the + * index of that array. + */ + private static final int NESTED_INTENT_KEY_TYPE_EXTRA_PARCEL_ARRAY = 1 << 1; + + /** + * This flag indicates the key is for an extra parcel list in mExtras and the index is the + * index of that list. + */ + private static final int NESTED_INTENT_KEY_TYPE_EXTRA_PARCEL_LIST = 1 << 2; + + /** + * This flag indicates the key is for an extra parcel in mClipData.mItems. + */ + private static final int NESTED_INTENT_KEY_TYPE_CLIP_DATA = 1 << 3; + + // type can be a short or even byte. But then probably cannot use @IntDef?? Also not sure + // if it is necessary. + private final @NestedIntentKeyType int mType; + private final String mKey; + private final int mIndex; + + private NestedIntentKey(@NestedIntentKeyType int type, String key, int index) { + this.mType = type; + this.mKey = key; + this.mIndex = index; + } + + @Override + public boolean equals(Object o) { + if (this == o) return true; + if (o == null || getClass() != o.getClass()) return false; + NestedIntentKey that = (NestedIntentKey) o; + return mType == that.mType && mIndex == that.mIndex && Objects.equals(mKey, that.mKey); + } + + @Override + public int hashCode() { + return Objects.hash(mType, mKey, mIndex); + } } private @Nullable CreatorTokenInfo mCreatorTokenInfo; @@ -12227,8 +12306,9 @@ public class Intent implements Parcelable, Cloneable { } /** @hide */ - public Set<String> getExtraIntentKeys() { - return mCreatorTokenInfo == null ? null : mCreatorTokenInfo.mExtraIntentKeys; + @VisibleForTesting + public Set<NestedIntentKey> getExtraIntentKeys() { + return mCreatorTokenInfo == null ? null : mCreatorTokenInfo.mNestedIntentKeys; } /** @hide */ @@ -12246,45 +12326,168 @@ public class Intent implements Parcelable, Cloneable { * @hide */ public void collectExtraIntentKeys() { - if (!preventIntentRedirect()) return; + if (preventIntentRedirect()) { + collectNestedIntentKeysRecur(new ArraySet<>()); + } + } - if (mExtras != null && !mExtras.isEmpty()) { + private void collectNestedIntentKeysRecur(Set<Intent> visited) { + if (mExtras != null && !mExtras.isParcelled() && !mExtras.isEmpty()) { for (String key : mExtras.keySet()) { - if (mExtras.get(key) instanceof Intent) { - if (mCreatorTokenInfo == null) { - mCreatorTokenInfo = new CreatorTokenInfo(); - } - if (mCreatorTokenInfo.mExtraIntentKeys == null) { - mCreatorTokenInfo.mExtraIntentKeys = new ArraySet<>(); - } - mCreatorTokenInfo.mExtraIntentKeys.add(key); + Object value = mExtras.get(key); + + if (value instanceof Intent intent && !visited.contains(intent)) { + handleNestedIntent(intent, visited, new NestedIntentKey( + NestedIntentKey.NESTED_INTENT_KEY_TYPE_EXTRA_PARCEL, key, 0)); + } else if (value instanceof Parcelable[] parcelables) { + handleParcelableArray(parcelables, key, visited); + } else if (value instanceof ArrayList<?> parcelables) { + handleParcelableList(parcelables, key, visited); + } + } + } + + if (mClipData != null) { + for (int i = 0; i < mClipData.getItemCount(); i++) { + Intent intent = mClipData.getItemAt(i).mIntent; + if (intent != null && !visited.contains(intent)) { + handleNestedIntent(intent, visited, new NestedIntentKey( + NestedIntentKey.NESTED_INTENT_KEY_TYPE_CLIP_DATA, null, i)); } } } } + private void handleNestedIntent(Intent intent, Set<Intent> visited, NestedIntentKey key) { + visited.add(intent); + if (mCreatorTokenInfo == null) { + mCreatorTokenInfo = new CreatorTokenInfo(); + } + if (mCreatorTokenInfo.mNestedIntentKeys == null) { + mCreatorTokenInfo.mNestedIntentKeys = new ArraySet<>(); + } + mCreatorTokenInfo.mNestedIntentKeys.add(key); + intent.collectNestedIntentKeysRecur(visited); + } + + private void handleParcelableArray(Parcelable[] parcelables, String key, Set<Intent> visited) { + for (int i = 0; i < parcelables.length; i++) { + if (parcelables[i] instanceof Intent intent && !visited.contains(intent)) { + handleNestedIntent(intent, visited, new NestedIntentKey( + NestedIntentKey.NESTED_INTENT_KEY_TYPE_EXTRA_PARCEL_ARRAY, key, i)); + } + } + } + + private void handleParcelableList(ArrayList<?> parcelables, String key, Set<Intent> visited) { + for (int i = 0; i < parcelables.size(); i++) { + if (parcelables.get(i) instanceof Intent intent && !visited.contains(intent)) { + handleNestedIntent(intent, visited, new NestedIntentKey( + NestedIntentKey.NESTED_INTENT_KEY_TYPE_EXTRA_PARCEL_LIST, key, i)); + } + } + } + + private static final Consumer<Intent> CHECK_CREATOR_TOKEN_ACTION = intent -> { + intent.mLocalFlags |= LOCAL_FLAG_TRUSTED_CREATOR_TOKEN_PRESENT; + if (intent.mExtras != null) { + intent.mExtras.enableTokenVerification(); + } + }; + /** @hide */ public void checkCreatorToken() { - if (mExtras == null) return; - if (mCreatorTokenInfo != null && mCreatorTokenInfo.mExtraIntentKeys != null) { - for (String key : mCreatorTokenInfo.mExtraIntentKeys) { - try { - Intent extraIntent = mExtras.getParcelable(key, Intent.class); - if (extraIntent == null) { - Log.w(TAG, "The key {" + key - + "} does not correspond to an intent in the bundle."); - continue; - } - extraIntent.mLocalFlags |= LOCAL_FLAG_TRUSTED_CREATOR_TOKEN_PRESENT; - } catch (Exception e) { - Log.e(TAG, "Failed to validate creator token. key: " + key + ".", e); + forEachNestedCreatorToken(CHECK_CREATOR_TOKEN_ACTION); + + if (mExtras != null) { + // mark the bundle as intent extras after calls to getParcelable. + // otherwise, the logic to mark missing token would run before + // mark trusted creator token present. + mExtras.enableTokenVerification(); + } + } + + /** @hide */ + public void forEachNestedCreatorToken(Consumer<? super Intent> action) { + if (mExtras == null && mClipData == null) return; + + if (mCreatorTokenInfo != null && mCreatorTokenInfo.mNestedIntentKeys != null) { + int N = mCreatorTokenInfo.mNestedIntentKeys.size(); + for (int i = 0; i < N; i++) { + NestedIntentKey key = mCreatorTokenInfo.mNestedIntentKeys.valueAt(i); + Intent extraIntent = extractIntentFromKey(key); + + if (extraIntent != null) { + action.accept(extraIntent); + extraIntent.forEachNestedCreatorToken(action); + } else { + Log.w(TAG, getLogMessageForKey(key)); } } } - // mark the bundle as intent extras after calls to getParcelable. - // otherwise, the logic to mark missing token would run before - // mark trusted creator token present. - mExtras.setIsIntentExtra(); + } + + private Intent extractIntentFromKey(NestedIntentKey key) { + switch (key.mType) { + case NestedIntentKey.NESTED_INTENT_KEY_TYPE_EXTRA_PARCEL: + return mExtras == null ? null : mExtras.getParcelable(key.mKey, Intent.class); + case NestedIntentKey.NESTED_INTENT_KEY_TYPE_EXTRA_PARCEL_ARRAY: + if (mExtras == null) return null; + Intent[] extraIntents = mExtras.getParcelableArray(key.mKey, Intent.class); + if (extraIntents != null && key.mIndex < extraIntents.length) { + return extraIntents[key.mIndex]; + } + break; + case NestedIntentKey.NESTED_INTENT_KEY_TYPE_EXTRA_PARCEL_LIST: + if (mExtras == null) return null; + ArrayList<Intent> extraIntentsList = mExtras.getParcelableArrayList(key.mKey, + Intent.class); + if (extraIntentsList != null && key.mIndex < extraIntentsList.size()) { + return extraIntentsList.get(key.mIndex); + } + break; + case NestedIntentKey.NESTED_INTENT_KEY_TYPE_CLIP_DATA: + if (mClipData == null) return null; + if (key.mIndex < mClipData.getItemCount()) { + ClipData.Item item = mClipData.getItemAt(key.mIndex); + if (item != null) { + return item.mIntent; + } + } + break; + } + return null; + } + + private String getLogMessageForKey(NestedIntentKey key) { + switch (key.mType) { + case NestedIntentKey.NESTED_INTENT_KEY_TYPE_EXTRA_PARCEL: + return "The key {" + key + "} does not correspond to an intent in the bundle."; + case NestedIntentKey.NESTED_INTENT_KEY_TYPE_EXTRA_PARCEL_ARRAY: + if (mExtras.getParcelableArray(key.mKey, Intent.class) == null) { + return "The key {" + key + + "} does not correspond to a Parcelable[] in the bundle."; + } else { + return "Parcelable[" + key.mIndex + "] for key {" + key + "} is not an intent."; + } + case NestedIntentKey.NESTED_INTENT_KEY_TYPE_EXTRA_PARCEL_LIST: + if (mExtras.getParcelableArrayList(key.mKey, Intent.class) == null) { + return "The key {" + key + + "} does not correspond to an ArrayList<Parcelable> in the bundle."; + } else { + return "List.get(" + key.mIndex + ") for key {" + key + "} is not an intent."; + } + case NestedIntentKey.NESTED_INTENT_KEY_TYPE_CLIP_DATA: + if (key.mIndex >= mClipData.getItemCount()) { + return "Index out of range for clipData items. index: " + key.mIndex + + ". item counts: " + mClipData.getItemCount(); + } else { + return "clipData items at index [" + key.mIndex + + "] is null or does not contain an intent."; + } + default: + return "Unknown key type: " + key.mType; + } } /** @@ -12357,7 +12560,19 @@ public class Intent implements Parcelable, Cloneable { } else { out.writeInt(1); out.writeStrongBinder(mCreatorTokenInfo.mCreatorToken); - out.writeArraySet(mCreatorTokenInfo.mExtraIntentKeys); + + if (mCreatorTokenInfo.mNestedIntentKeys != null) { + final int N = mCreatorTokenInfo.mNestedIntentKeys.size(); + out.writeInt(N); + for (int i = 0; i < N; i++) { + NestedIntentKey key = mCreatorTokenInfo.mNestedIntentKeys.valueAt(i); + out.writeInt(key.mType); + out.writeString8(key.mKey); + out.writeInt(key.mIndex); + } + } else { + out.writeInt(0); + } } } } @@ -12422,7 +12637,18 @@ public class Intent implements Parcelable, Cloneable { if (in.readInt() != 0) { mCreatorTokenInfo = new CreatorTokenInfo(); mCreatorTokenInfo.mCreatorToken = in.readStrongBinder(); - mCreatorTokenInfo.mExtraIntentKeys = (ArraySet<String>) in.readArraySet(null); + + N = in.readInt(); + if (N > 0) { + mCreatorTokenInfo.mNestedIntentKeys = new ArraySet<>(N); + for (int i = 0; i < N; i++) { + int type = in.readInt(); + String key = in.readString8(); + int index = in.readInt(); + mCreatorTokenInfo.mNestedIntentKeys.append( + new NestedIntentKey(type, key, index)); + } + } } } } diff --git a/core/java/android/os/Bundle.java b/core/java/android/os/Bundle.java index c18fb0c38b82..99e7d166446e 100644 --- a/core/java/android/os/Bundle.java +++ b/core/java/android/os/Bundle.java @@ -281,7 +281,7 @@ public final class Bundle extends BaseBundle implements Cloneable, Parcelable { } /** {@hide} */ - public void setIsIntentExtra() { + public void enableTokenVerification() { mFlags |= FLAG_VERIFY_TOKENS_PRESENT; } diff --git a/core/tests/coretests/src/android/content/IntentTest.java b/core/tests/coretests/src/android/content/IntentTest.java index d169ce3c07d0..7bc4abd935b6 100644 --- a/core/tests/coretests/src/android/content/IntentTest.java +++ b/core/tests/coretests/src/android/content/IntentTest.java @@ -37,6 +37,10 @@ import org.junit.Rule; import org.junit.Test; import org.junit.runner.RunWith; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.List; + /** * Build/Install/Run: * atest FrameworksCoreTests:IntentTest @@ -57,7 +61,12 @@ public class IntentTest { public void testReadFromParcelWithExtraIntentKeys() { Intent intent = new Intent("TEST_ACTION"); intent.putExtra(TEST_EXTRA_NAME, new Intent(TEST_ACTION)); + // Not an intent, don't count. intent.putExtra(TEST_EXTRA_NAME + "2", 1); + ArrayList<Intent> intents = new ArrayList<>(); + intents.add(new Intent(TEST_ACTION)); + intent.putParcelableArrayListExtra(TEST_EXTRA_NAME + "3", intents); + intent.setClipData(ClipData.newIntent("label", new Intent(TEST_ACTION))); intent.collectExtraIntentKeys(); final Parcel parcel = Parcel.obtain(); @@ -68,7 +77,7 @@ public class IntentTest { assertEquals(intent.getAction(), target.getAction()); assertEquals(intent.getExtraIntentKeys(), target.getExtraIntentKeys()); - assertThat(intent.getExtraIntentKeys()).hasSize(1); + assertThat(intent.getExtraIntentKeys()).hasSize(3); } @Test @@ -87,13 +96,37 @@ public class IntentTest { @RequiresFlagsEnabled(Flags.FLAG_PREVENT_INTENT_REDIRECT) public void testCollectExtraIntentKeys() { Intent intent = new Intent(TEST_ACTION); - Intent extraIntent = new Intent(TEST_ACTION, TEST_URI); - intent.putExtra(TEST_EXTRA_NAME, extraIntent); + + Intent[] intents = new Intent[10]; + for (int i = 0; i < intents.length; i++) { + intents[i] = new Intent("action" + i); + } + Intent[] intents2 = new Intent[2]; // intents[6-7] + System.arraycopy(intents, 6, intents2, 0, intents2.length); + ArrayList<Intent> intents3 = new ArrayList<>(2); + intents3.addAll(Arrays.asList(intents).subList(8, 10)); // intents[8-9] + intent.putExtra("key1", intents[0]); + intent.putExtra("array-key", intents2); + intent.setClipData(ClipData.newIntent("label2", intents[1])); + intent.putExtra("intkey", 1); + intents[0].putExtra("key3", intents[2]); + intents[0].setClipData(ClipData.newIntent("label4", intents[3])); + intents[0].putParcelableArrayListExtra("array-list-key", intents3); + intents[1].putExtra("key3", intents[4]); + intents[1].setClipData(ClipData.newIntent("label4", intents[5])); + intents[5].putExtra("intkey", 2); intent.collectExtraIntentKeys(); - assertThat(intent.getExtraIntentKeys()).hasSize(1); - assertThat(intent.getExtraIntentKeys()).contains(TEST_EXTRA_NAME); + // collect all actions of nested intents. + final List<String> actions = new ArrayList<>(); + intent.forEachNestedCreatorToken(intent1 -> { + actions.add(intent1.getAction()); + }); + assertThat(actions).hasSize(10); + for (int i = 0; i < intents.length; i++) { + assertThat(actions).contains("action" + i); + } } } diff --git a/services/core/java/com/android/server/am/ActivityManagerService.java b/services/core/java/com/android/server/am/ActivityManagerService.java index 1c3569dd52d0..86c54a539f62 100644 --- a/services/core/java/com/android/server/am/ActivityManagerService.java +++ b/services/core/java/com/android/server/am/ActivityManagerService.java @@ -19287,31 +19287,18 @@ public class ActivityManagerService extends IActivityManager.Stub public void addCreatorToken(@Nullable Intent intent, String creatorPackage) { if (!preventIntentRedirect()) return; - if (intent == null || intent.getExtraIntentKeys() == null) return; - for (String key : intent.getExtraIntentKeys()) { - try { - Intent extraIntent = intent.getParcelableExtra(key, Intent.class); - if (extraIntent == null) { - Slog.w(TAG, "The key {" + key - + "} does not correspond to an intent in the extra bundle."); - continue; - } - IntentCreatorToken creatorToken = createIntentCreatorToken(extraIntent, - creatorPackage); - if (creatorToken != null) { - extraIntent.setCreatorToken(creatorToken); - Slog.wtf(TAG, "A creator token is added to an intent. creatorPackage: " - + creatorPackage + "; intent: " + intent); - FrameworkStatsLog.write(INTENT_CREATOR_TOKEN_ADDED, - creatorToken.getCreatorUid()); - } - } catch (Exception e) { - Slog.wtf(TAG, - "Something went wrong when trying to add creator token for embedded " - + "intents of intent: ." - + intent, e); + if (intent == null) return; + intent.forEachNestedCreatorToken(extraIntent -> { + IntentCreatorToken creatorToken = createIntentCreatorToken(extraIntent, creatorPackage); + if (creatorToken != null) { + extraIntent.setCreatorToken(creatorToken); + // TODO remove Slog.wtf once proven FrameworkStatsLog works. b/375396329 + Slog.wtf(TAG, "A creator token is added to an intent. creatorPackage: " + + creatorPackage + "; intent: " + intent); + FrameworkStatsLog.write(INTENT_CREATOR_TOKEN_ADDED, + creatorToken.getCreatorUid()); } - } + }); } private IntentCreatorToken createIntentCreatorToken(Intent intent, String creatorPackage) { diff --git a/services/tests/mockingservicestests/src/com/android/server/am/ActivityManagerServiceTest.java b/services/tests/mockingservicestests/src/com/android/server/am/ActivityManagerServiceTest.java index 2a825f35bf62..dcbc23410fdb 100644 --- a/services/tests/mockingservicestests/src/com/android/server/am/ActivityManagerServiceTest.java +++ b/services/tests/mockingservicestests/src/com/android/server/am/ActivityManagerServiceTest.java @@ -1308,6 +1308,8 @@ public class ActivityManagerServiceTest { Intent intent = new Intent(); Intent extraIntent = new Intent("EXTRA_INTENT_ACTION"); intent.putExtra("EXTRA_INTENT0", extraIntent); + Intent nestedIntent = new Intent("NESTED_INTENT_ACTION"); + extraIntent.putExtra("NESTED_INTENT", nestedIntent); intent.collectExtraIntentKeys(); mAms.addCreatorToken(intent, TEST_PACKAGE); @@ -1317,6 +1319,11 @@ public class ActivityManagerServiceTest { assertThat(token).isNotNull(); assertThat(token.getCreatorUid()).isEqualTo(mInjector.getCallingUid()); assertThat(token.getCreatorPackage()).isEqualTo(TEST_PACKAGE); + + token = (ActivityManagerService.IntentCreatorToken) nestedIntent.getCreatorToken(); + assertThat(token).isNotNull(); + assertThat(token.getCreatorUid()).isEqualTo(mInjector.getCallingUid()); + assertThat(token.getCreatorPackage()).isEqualTo(TEST_PACKAGE); } @Test @@ -1349,6 +1356,8 @@ public class ActivityManagerServiceTest { Intent intent = new Intent(); Intent extraIntent = new Intent("EXTRA_INTENT_ACTION"); intent.putExtra("EXTRA_INTENT", extraIntent); + Intent nestedIntent = new Intent("NESTED_INTENT_ACTION"); + extraIntent.putExtra("NESTED_INTENT", nestedIntent); intent.collectExtraIntentKeys(); @@ -1374,9 +1383,12 @@ public class ActivityManagerServiceTest { extraIntent = intent.getParcelableExtra("EXTRA_INTENT", Intent.class); extraIntent2 = intent.getParcelableExtra("EXTRA_INTENT2", Intent.class); extraIntent3 = intent.getParcelableExtra("EXTRA_INTENT3", Intent.class); + nestedIntent = extraIntent.getParcelableExtra("NESTED_INTENT", Intent.class); assertThat(extraIntent.getExtendedFlags() & Intent.EXTENDED_FLAG_MISSING_CREATOR_OR_INVALID_TOKEN).isEqualTo(0); + assertThat(nestedIntent.getExtendedFlags() + & Intent.EXTENDED_FLAG_MISSING_CREATOR_OR_INVALID_TOKEN).isEqualTo(0); // sneaked in intent should have EXTENDED_FLAG_MISSING_CREATOR_OR_INVALID_TOKEN set. assertThat(extraIntent2.getExtendedFlags() & Intent.EXTENDED_FLAG_MISSING_CREATOR_OR_INVALID_TOKEN).isNotEqualTo(0); |