Skip to content

Commit d203ccf

Browse files
author
Android Build Coastguard Worker
committed
Merge cherrypicks of ['googleplex-android-review.googlesource.com/23905843', 'googleplex-android-review.googlesource.com/23918399', 'googleplex-android-review.googlesource.com/24404277', 'googleplex-android-review.googlesource.com/20064768', 'googleplex-android-review.googlesource.com/24046929', 'googleplex-android-review.googlesource.com/24573549', 'googleplex-android-review.googlesource.com/24342145', 'googleplex-android-review.googlesource.com/23623415', 'googleplex-android-review.googlesource.com/24608667', 'googleplex-android-review.googlesource.com/24182288', 'googleplex-android-review.googlesource.com/24757286', 'googleplex-android-review.googlesource.com/24058898', 'googleplex-android-review.googlesource.com/24805388', 'googleplex-android-review.googlesource.com/22465041', 'googleplex-android-review.googlesource.com/24945646', 'googleplex-android-review.googlesource.com/24555955'] into security-aosp-tm-release.
Change-Id: I8265bdcfbebf77a1a652fd66bc73e1451970684b
2 parents 51640ed + 1a42ae5 commit d203ccf

File tree

22 files changed

+464
-42
lines changed

22 files changed

+464
-42
lines changed

‎cmds/incidentd/src/IncidentService.cpp‎

Lines changed: 12 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -500,9 +500,13 @@ status_t IncidentService::onTransact(uint32_t code, const Parcel& data, Parcel*
500500

501501
switch (code) {
502502
case SHELL_COMMAND_TRANSACTION: {
503-
int in = data.readFileDescriptor();
504-
int out = data.readFileDescriptor();
505-
int err = data.readFileDescriptor();
503+
unique_fd in, out, err;
504+
if (status_t status = data.readUniqueFileDescriptor(&in); status != OK) return status;
505+
506+
if (status_t status = data.readUniqueFileDescriptor(&out); status != OK) return status;
507+
508+
if (status_t status = data.readUniqueFileDescriptor(&err); status != OK) return status;
509+
506510
int argc = data.readInt32();
507511
Vector<String8> args;
508512
for (int i = 0; i < argc && data.dataAvail() > 0; i++) {
@@ -512,15 +516,15 @@ status_t IncidentService::onTransact(uint32_t code, const Parcel& data, Parcel*
512516
sp<IResultReceiver> resultReceiver =
513517
IResultReceiver::asInterface(data.readStrongBinder());
514518

515-
FILE* fin = fdopen(in, "r");
516-
FILE* fout = fdopen(out, "w");
517-
FILE* ferr = fdopen(err, "w");
519+
FILE* fin = fdopen(in.release(), "r");
520+
FILE* fout = fdopen(out.release(), "w");
521+
FILE* ferr = fdopen(err.release(), "w");
518522

519523
if (fin == NULL || fout == NULL || ferr == NULL) {
520524
resultReceiver->send(NO_MEMORY);
521525
} else {
522-
err = command(fin, fout, ferr, args);
523-
resultReceiver->send(err);
526+
status_t result = command(fin, fout, ferr, args);
527+
resultReceiver->send(result);
524528
}
525529

526530
if (fin != NULL) {

‎core/java/android/app/IActivityTaskManager.aidl‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -239,6 +239,7 @@ interface IActivityTaskManager {
239239
* {@link android.view.WindowManagerPolicyConstants#KEYGUARD_GOING_AWAY_FLAG_TO_SHADE}
240240
* etc.
241241
*/
242+
@JavaPassthrough(annotation="@android.annotation.RequiresPermission(android.Manifest.permission.CONTROL_KEYGUARD)")
242243
void keyguardGoingAway(int flags);
243244

244245
void suppressResizeConfigChanges(boolean suppress);

‎core/java/android/app/IUriGrantsManager.aidl‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,4 +39,7 @@ interface IUriGrantsManager {
3939
void clearGrantedUriPermissions(in String packageName, int userId);
4040
ParceledListSlice getUriPermissions(in String packageName, boolean incoming,
4141
boolean persistedOnly);
42+
43+
int checkGrantUriPermission_ignoreNonSystem(
44+
int sourceUid, String targetPkg, in Uri uri, int modeFlags, int userId);
4245
}

‎core/java/android/app/Notification.java‎

Lines changed: 33 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -2138,6 +2138,10 @@ public Action build() {
21382138
}
21392139
}
21402140

2141+
private void visitUris(@NonNull Consumer<Uri> visitor) {
2142+
visitIconUri(visitor, getIcon());
2143+
}
2144+
21412145
@Override
21422146
public Action clone() {
21432147
return new Action(
@@ -2823,7 +2827,7 @@ public void visitUris(@NonNull Consumer<Uri> visitor) {
28232827

28242828
if (actions != null) {
28252829
for (Action action : actions) {
2826-
visitIconUri(visitor, action.getIcon());
2830+
action.visitUris(visitor);
28272831
}
28282832
}
28292833

@@ -2846,18 +2850,14 @@ public void visitUris(@NonNull Consumer<Uri> visitor) {
28462850
visitor.accept(Uri.parse(extras.getString(EXTRA_BACKGROUND_IMAGE_URI)));
28472851
}
28482852

2849-
ArrayList<Person> people = extras.getParcelableArrayList(EXTRA_PEOPLE_LIST);
2853+
ArrayList<Person> people = extras.getParcelableArrayList(EXTRA_PEOPLE_LIST,
2854+
Person.class);
28502855
if (people != null && !people.isEmpty()) {
28512856
for (Person p : people) {
28522857
visitor.accept(p.getIconUri());
28532858
}
28542859
}
28552860

2856-
final Person person = extras.getParcelable(EXTRA_MESSAGING_PERSON, Person.class);
2857-
if (person != null) {
2858-
visitor.accept(person.getIconUri());
2859-
}
2860-
28612861
final RemoteInputHistoryItem[] history = extras.getParcelableArray(
28622862
Notification.EXTRA_REMOTE_INPUT_HISTORY_ITEMS,
28632863
RemoteInputHistoryItem.class);
@@ -2869,10 +2869,16 @@ public void visitUris(@NonNull Consumer<Uri> visitor) {
28692869
}
28702870
}
28712871
}
2872-
}
28732872

2874-
if (isStyle(MessagingStyle.class) && extras != null) {
2875-
final Parcelable[] messages = extras.getParcelableArray(EXTRA_MESSAGES);
2873+
// Extras for MessagingStyle. We visit them even if not isStyle(MessagingStyle), since
2874+
// Notification Listeners might use directly (without the isStyle check).
2875+
final Person person = extras.getParcelable(EXTRA_MESSAGING_PERSON, Person.class);
2876+
if (person != null) {
2877+
visitor.accept(person.getIconUri());
2878+
}
2879+
2880+
final Parcelable[] messages = extras.getParcelableArray(EXTRA_MESSAGES,
2881+
Parcelable.class);
28762882
if (!ArrayUtils.isEmpty(messages)) {
28772883
for (MessagingStyle.Message message : MessagingStyle.Message
28782884
.getMessagesFromBundleArray(messages)) {
@@ -2885,7 +2891,8 @@ public void visitUris(@NonNull Consumer<Uri> visitor) {
28852891
}
28862892
}
28872893

2888-
final Parcelable[] historic = extras.getParcelableArray(EXTRA_HISTORIC_MESSAGES);
2894+
final Parcelable[] historic = extras.getParcelableArray(EXTRA_HISTORIC_MESSAGES,
2895+
Parcelable.class);
28892896
if (!ArrayUtils.isEmpty(historic)) {
28902897
for (MessagingStyle.Message message : MessagingStyle.Message
28912898
.getMessagesFromBundleArray(historic)) {
@@ -2898,20 +2905,24 @@ public void visitUris(@NonNull Consumer<Uri> visitor) {
28982905
}
28992906
}
29002907

2901-
visitIconUri(visitor, extras.getParcelable(EXTRA_CONVERSATION_ICON));
2902-
}
2908+
visitIconUri(visitor, extras.getParcelable(EXTRA_CONVERSATION_ICON, Icon.class));
29032909

2904-
if (isStyle(CallStyle.class) & extras != null) {
2905-
Person callPerson = extras.getParcelable(EXTRA_CALL_PERSON);
2910+
// Extras for CallStyle (same reason for visiting without checking isStyle).
2911+
Person callPerson = extras.getParcelable(EXTRA_CALL_PERSON, Person.class);
29062912
if (callPerson != null) {
29072913
visitor.accept(callPerson.getIconUri());
29082914
}
2909-
visitIconUri(visitor, extras.getParcelable(EXTRA_VERIFICATION_ICON));
2915+
visitIconUri(visitor, extras.getParcelable(EXTRA_VERIFICATION_ICON, Icon.class));
29102916
}
29112917

29122918
if (mBubbleMetadata != null) {
29132919
visitIconUri(visitor, mBubbleMetadata.getIcon());
29142920
}
2921+
2922+
if (extras != null && extras.containsKey(WearableExtender.EXTRA_WEARABLE_EXTENSIONS)) {
2923+
WearableExtender extender = new WearableExtender(this);
2924+
extender.visitUris(visitor);
2925+
}
29152926
}
29162927

29172928
/**
@@ -11597,6 +11608,12 @@ private void setFlag(int mask, boolean value) {
1159711608
mFlags &= ~mask;
1159811609
}
1159911610
}
11611+
11612+
private void visitUris(@NonNull Consumer<Uri> visitor) {
11613+
for (Action action : mActions) {
11614+
action.visitUris(visitor);
11615+
}
11616+
}
1160011617
}
1160111618

1160211619
/**

‎core/java/android/os/PersistableBundle.java‎

Lines changed: 34 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121
import android.annotation.NonNull;
2222
import android.annotation.Nullable;
2323
import android.util.ArrayMap;
24+
import android.util.Slog;
2425
import android.util.TypedXmlPullParser;
2526
import android.util.TypedXmlSerializer;
2627
import android.util.Xml;
@@ -50,6 +51,8 @@
5051
*/
5152
public final class PersistableBundle extends BaseBundle implements Cloneable, Parcelable,
5253
XmlUtils.WriteMapCallback {
54+
private static final String TAG = "PersistableBundle";
55+
5356
private static final String TAG_PERSISTABLEMAP = "pbundle_as_map";
5457

5558
/** An unmodifiable {@code PersistableBundle} that is always {@link #isEmpty() empty}. */
@@ -118,7 +121,11 @@ public PersistableBundle(PersistableBundle b) {
118121
* @hide
119122
*/
120123
public PersistableBundle(Bundle b) {
121-
this(b.getItemwiseMap());
124+
this(b, true);
125+
}
126+
127+
private PersistableBundle(Bundle b, boolean throwException) {
128+
this(b.getItemwiseMap(), throwException);
122129
}
123130

124131
/**
@@ -127,7 +134,7 @@ public PersistableBundle(Bundle b) {
127134
* @param map a Map containing only those items that can be persisted.
128135
* @throws IllegalArgumentException if any element of #map cannot be persisted.
129136
*/
130-
private PersistableBundle(ArrayMap<String, Object> map) {
137+
private PersistableBundle(ArrayMap<String, Object> map, boolean throwException) {
131138
super();
132139
mFlags = FLAG_DEFUSABLE;
133140

@@ -136,16 +143,23 @@ private PersistableBundle(ArrayMap<String, Object> map) {
136143

137144
// Now verify each item throwing an exception if there is a violation.
138145
final int N = mMap.size();
139-
for (int i=0; i<N; i++) {
146+
for (int i = N - 1; i >= 0; --i) {
140147
Object value = mMap.valueAt(i);
141148
if (value instanceof ArrayMap) {
142149
// Fix up any Maps by replacing them with PersistableBundles.
143-
mMap.setValueAt(i, new PersistableBundle((ArrayMap<String, Object>) value));
150+
mMap.setValueAt(i,
151+
new PersistableBundle((ArrayMap<String, Object>) value, throwException));
144152
} else if (value instanceof Bundle) {
145-
mMap.setValueAt(i, new PersistableBundle(((Bundle) value)));
153+
mMap.setValueAt(i, new PersistableBundle((Bundle) value, throwException));
146154
} else if (!isValidType(value)) {
147-
throw new IllegalArgumentException("Bad value in PersistableBundle key="
148-
+ mMap.keyAt(i) + " value=" + value);
155+
final String errorMsg = "Bad value in PersistableBundle key="
156+
+ mMap.keyAt(i) + " value=" + value;
157+
if (throwException) {
158+
throw new IllegalArgumentException(errorMsg);
159+
} else {
160+
Slog.wtfStack(TAG, errorMsg);
161+
mMap.removeAt(i);
162+
}
149163
}
150164
}
151165
}
@@ -268,6 +282,15 @@ public void saveToXml(XmlSerializer out) throws IOException, XmlPullParserExcept
268282
/** @hide */
269283
public void saveToXml(TypedXmlSerializer out) throws IOException, XmlPullParserException {
270284
unparcel();
285+
// Explicitly drop invalid types an attacker may have added before persisting.
286+
for (int i = mMap.size() - 1; i >= 0; --i) {
287+
final Object value = mMap.valueAt(i);
288+
if (!isValidType(value)) {
289+
Slog.e(TAG, "Dropping bad data before persisting: "
290+
+ mMap.keyAt(i) + "=" + value);
291+
mMap.removeAt(i);
292+
}
293+
}
271294
XmlUtils.writeMapXml(mMap, out, this);
272295
}
273296

@@ -322,9 +345,12 @@ public static PersistableBundle restoreFromXml(TypedXmlPullParser in) throws IOE
322345
while (((event = in.next()) != XmlPullParser.END_DOCUMENT) &&
323346
(event != XmlPullParser.END_TAG || in.getDepth() < outerDepth)) {
324347
if (event == XmlPullParser.START_TAG) {
348+
// Don't throw an exception when restoring from XML since an attacker could try to
349+
// input invalid data in the persisted file.
325350
return new PersistableBundle((ArrayMap<String, Object>)
326351
XmlUtils.readThisArrayMapXml(in, startTag, tagName,
327-
new MyReadMapCallback()));
352+
new MyReadMapCallback()),
353+
/* throwException */ false);
328354
}
329355
}
330356
return new PersistableBundle(); // An empty mutable PersistableBundle

‎core/java/com/android/internal/app/IAppOpsService.aidl‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,8 @@ interface IAppOpsService {
5252
int checkAudioOperation(int code, int usage, int uid, String packageName);
5353
boolean shouldCollectNotes(int opCode);
5454
void setCameraAudioRestriction(int mode);
55+
void startWatchingModeWithFlags(int op, String packageName, int flags,
56+
IAppOpsCallback callback);
5557
// End of methods also called by native code.
5658
// Any new method exposed to native must be added after the last one, do not reorder
5759

@@ -110,8 +112,6 @@ interface IAppOpsService {
110112
void startWatchingStarted(in int[] ops, IAppOpsStartedCallback callback);
111113
void stopWatchingStarted(IAppOpsStartedCallback callback);
112114

113-
void startWatchingModeWithFlags(int op, String packageName, int flags, IAppOpsCallback callback);
114-
115115
void startWatchingNoted(in int[] ops, IAppOpsNotedCallback callback);
116116
void stopWatchingNoted(IAppOpsNotedCallback callback);
117117

‎packages/SystemUI/src/com/android/systemui/keyguard/KeyguardViewMediator.java‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1557,6 +1557,10 @@ public void setKeyguardEnabled(boolean enabled) {
15571557
mExternallyEnabled = enabled;
15581558

15591559
if (!enabled && mShowing) {
1560+
if (mLockPatternUtils.isUserInLockdown(KeyguardUpdateMonitor.getCurrentUser())) {
1561+
Log.d(TAG, "keyguardEnabled(false) overridden by user lockdown");
1562+
return;
1563+
}
15601564
if (mExitSecureCallback != null) {
15611565
if (DEBUG) Log.d(TAG, "in process of verifyUnlock request, ignoring");
15621566
// we're in the process of handling a request to verify the user
@@ -1782,9 +1786,9 @@ private void doKeyguardLocked(Bundle options) {
17821786
return;
17831787
}
17841788

1785-
// if another app is disabling us, don't show
1789+
// if another app is disabling us, don't show unless we're in lockdown mode
17861790
if (!mExternallyEnabled
1787-
&& !mLockPatternUtils.isUserInLockdown(KeyguardUpdateMonitor.getCurrentUser())) {
1791+
&& !mLockPatternUtils.isUserInLockdown(KeyguardUpdateMonitor.getCurrentUser())) {
17881792
if (DEBUG) Log.d(TAG, "doKeyguard: not showing because externally disabled");
17891793

17901794
mNeedToReshowWhenReenabled = true;

‎packages/SystemUI/src/com/android/systemui/media/MediaDataManager.kt‎

Lines changed: 30 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -18,11 +18,13 @@ package com.android.systemui.media
1818

1919
import android.app.Notification
2020
import android.app.PendingIntent
21+
import android.app.UriGrantsManager
2122
import android.app.smartspace.SmartspaceConfig
2223
import android.app.smartspace.SmartspaceManager
2324
import android.app.smartspace.SmartspaceSession
2425
import android.app.smartspace.SmartspaceTarget
2526
import android.content.BroadcastReceiver
27+
import android.content.ContentProvider
2628
import android.content.ContentResolver
2729
import android.content.Context
2830
import android.content.Intent
@@ -592,20 +594,21 @@ class MediaDataManager(
592594
Log.d(TAG, "adding track for $userId from browser: $desc")
593595
}
594596

597+
val currentEntry = mediaEntries.get(packageName)
598+
val appUid = currentEntry?.appUid ?: Process.INVALID_UID
599+
595600
// Album art
596601
var artworkBitmap = desc.iconBitmap
597602
if (artworkBitmap == null && desc.iconUri != null) {
598-
artworkBitmap = loadBitmapFromUri(desc.iconUri!!)
603+
artworkBitmap = loadBitmapFromUriForUser(desc.iconUri!!, userId, appUid, packageName)
599604
}
600605
val artworkIcon = if (artworkBitmap != null) {
601606
Icon.createWithBitmap(artworkBitmap)
602607
} else {
603608
null
604609
}
605610

606-
val currentEntry = mediaEntries.get(packageName)
607611
val instanceId = currentEntry?.instanceId ?: logger.getNewInstanceId()
608-
val appUid = currentEntry?.appUid ?: Process.INVALID_UID
609612

610613
val mediaAction = getResumeMediaAction(resumeAction)
611614
val lastActive = systemClock.elapsedRealtime()
@@ -1000,6 +1003,30 @@ class MediaDataManager(
10001003
false
10011004
}
10021005
}
1006+
1007+
/** Returns a bitmap if the user can access the given URI, else null */
1008+
private fun loadBitmapFromUriForUser(
1009+
uri: Uri,
1010+
userId: Int,
1011+
appUid: Int,
1012+
packageName: String,
1013+
): Bitmap? {
1014+
try {
1015+
val ugm = UriGrantsManager.getService()
1016+
ugm.checkGrantUriPermission_ignoreNonSystem(
1017+
appUid,
1018+
packageName,
1019+
ContentProvider.getUriWithoutUserId(uri),
1020+
Intent.FLAG_GRANT_READ_URI_PERMISSION,
1021+
ContentProvider.getUserIdFromUri(uri, userId)
1022+
)
1023+
return loadBitmapFromUri(uri)
1024+
} catch (e: SecurityException) {
1025+
Log.e(TAG, "Failed to get URI permission: $e")
1026+
}
1027+
return null
1028+
}
1029+
10031030
/**
10041031
* Load a bitmap from a URI
10051032
* @param uri the uri to load

0 commit comments

Comments
 (0)