diff --git a/aosp_diff/preliminary/build/make/0004-Update-security_patch_level-string.patch b/aosp_diff/preliminary/build/make/0004-Update-security_patch_level-string.patch index 3570700016..c285441e57 100644 --- a/aosp_diff/preliminary/build/make/0004-Update-security_patch_level-string.patch +++ b/aosp_diff/preliminary/build/make/0004-Update-security_patch_level-string.patch @@ -20,7 +20,7 @@ index 419ff1aadc..fbbe777754 100644 # It must match one of the Android Security Patch Level strings of the Public Security Bulletins. # If there is no $PLATFORM_SECURITY_PATCH set, keep it empty. - PLATFORM_SECURITY_PATCH := 2023-05-05 -+ PLATFORM_SECURITY_PATCH := 2024-12-01 ++ PLATFORM_SECURITY_PATCH := 2025-01-01 endif include $(BUILD_SYSTEM)/version_util.mk diff --git a/aosp_diff/preliminary/external/giflib/0001-Fix-potential-overflow-when-calculating-ImageSize.bulletin.patch b/aosp_diff/preliminary/external/giflib/0001-Fix-potential-overflow-when-calculating-ImageSize.bulletin.patch new file mode 100644 index 0000000000..49a3a59409 --- /dev/null +++ b/aosp_diff/preliminary/external/giflib/0001-Fix-potential-overflow-when-calculating-ImageSize.bulletin.patch @@ -0,0 +1,38 @@ +From 21f1b1b5945f1eadef0707732cec3f150916ac42 Mon Sep 17 00:00:00 2001 +From: Sadaf Ebrahimi +Date: Tue, 17 Sep 2024 21:02:42 +0000 +Subject: [PATCH] Fix potential overflow when calculating ImageSize + +The modified if statement doesn't check the size of ImageDesc.Width and +ImageDesc.Height. If ImageDesc.Width and ImageDesc.Height are larger +than SIZE_MAX, then ImageSize overflows. + +Bug: 355461643 +Test: TreeHugger +(cherry picked from https://googleplex-android-review.googlesource.com/q/commit:90a054f323a9b460a79e7619e3669dd8b9f94338) +Merged-In: Ieef04e789acf783eda2dff2cd9284ed204f1d117 +Change-Id: Ieef04e789acf783eda2dff2cd9284ed204f1d117 +--- + dgif_lib.c | 6 ++++-- + 1 file changed, 4 insertions(+), 2 deletions(-) + +diff --git a/dgif_lib.c b/dgif_lib.c +index 66a1d6a..7b43a6a 100644 +--- a/dgif_lib.c ++++ b/dgif_lib.c +@@ -1099,8 +1099,10 @@ DGifSlurp(GifFileType *GifFile) + + sp = &GifFile->SavedImages[GifFile->ImageCount - 1]; + /* Allocate memory for the image */ +- if (sp->ImageDesc.Width < 0 && sp->ImageDesc.Height < 0 && +- sp->ImageDesc.Width > (INT_MAX / sp->ImageDesc.Height)) { ++ if (sp->ImageDesc.Width <= 0 || ++ sp->ImageDesc.Height <= 0 || ++ sp->ImageDesc.Width > ++ (INT_MAX / sp->ImageDesc.Height)) { + return GIF_ERROR; + } + ImageSize = sp->ImageDesc.Width * sp->ImageDesc.Height; +-- +2.46.1.824.gd892dcdcdd-goog + diff --git a/aosp_diff/preliminary/frameworks/base/99_0226-RingtoneManager-verify-default-ringtone-is-audio.bulletin.patch b/aosp_diff/preliminary/frameworks/base/99_0226-RingtoneManager-verify-default-ringtone-is-audio.bulletin.patch new file mode 100644 index 0000000000..a743e8fa7d --- /dev/null +++ b/aosp_diff/preliminary/frameworks/base/99_0226-RingtoneManager-verify-default-ringtone-is-audio.bulletin.patch @@ -0,0 +1,63 @@ +From 88cf23cfe50b80aa9d65cc22946de1765972cb80 Mon Sep 17 00:00:00 2001 +From: Jean-Michel Trivi +Date: Wed, 7 Dec 2022 04:36:46 +0000 +Subject: [PATCH] RingtoneManager: verify default ringtone is audio + +When a ringtone picker tries to set a ringtone through +RingtoneManager.setActualDefaultRingtoneUri (also +called by com.android.settings.DefaultRingtonePreference), +verify the mimeType can be obtained (not found when caller +doesn't have access to it) and it is an audio resource. + +Bug: 205837340 +Test: atest android.media.audio.cts.RingtoneManagerTest +(cherry picked from commit 38618f9fb16d3b5617e2289354d47abe5af17dad) +(cherry picked from https://googleplex-android-review.googlesource.com/q/commit:b8c2d03b720f0cc200ac59f6cfb411fddc3b119c) +Merged-In: I3f2c487ded405c0c1a83ef0a2fe99cff7cc9328e +Change-Id: I3f2c487ded405c0c1a83ef0a2fe99cff7cc9328e +--- + media/java/android/media/RingtoneManager.java | 19 +++++++++++++++++-- + 1 file changed, 17 insertions(+), 2 deletions(-) + +diff --git a/media/java/android/media/RingtoneManager.java b/media/java/android/media/RingtoneManager.java +index 27727699d05c..19720c2ed8f9 100644 +--- a/media/java/android/media/RingtoneManager.java ++++ b/media/java/android/media/RingtoneManager.java +@@ -776,10 +776,10 @@ public class RingtoneManager { + + return ringtoneUri; + } +- ++ + /** + * Sets the {@link Uri} of the default sound for a given sound type. +- * ++ * + * @param context A context used for querying. + * @param type The type whose default sound should be set. One of + * {@link #TYPE_RINGTONE}, {@link #TYPE_NOTIFICATION}, or +@@ -795,6 +795,21 @@ public class RingtoneManager { + if(!isInternalRingtoneUri(ringtoneUri)) { + ringtoneUri = ContentProvider.maybeAddUserId(ringtoneUri, context.getUserId()); + } ++ ++ if (ringtoneUri != null) { ++ final String mimeType = resolver.getType(ringtoneUri); ++ if (mimeType == null) { ++ Log.e(TAG, "setActualDefaultRingtoneUri for URI:" + ringtoneUri ++ + " ignored: failure to find mimeType (no access from this context?)"); ++ return; ++ } ++ if (!(mimeType.startsWith("audio/") || mimeType.equals("application/ogg"))) { ++ Log.e(TAG, "setActualDefaultRingtoneUri for URI:" + ringtoneUri ++ + " ignored: associated mimeType:" + mimeType + " is not an audio type"); ++ return; ++ } ++ } ++ + Settings.System.putStringForUser(resolver, setting, + ringtoneUri != null ? ringtoneUri.toString() : null, context.getUserId()); + +-- +2.46.1.824.gd892dcdcdd-goog + diff --git a/aosp_diff/preliminary/frameworks/base/99_0227-RingtoneManager-allow-video-ringtone-URI.bulletin.patch b/aosp_diff/preliminary/frameworks/base/99_0227-RingtoneManager-allow-video-ringtone-URI.bulletin.patch new file mode 100644 index 0000000000..d02b554f7b --- /dev/null +++ b/aosp_diff/preliminary/frameworks/base/99_0227-RingtoneManager-allow-video-ringtone-URI.bulletin.patch @@ -0,0 +1,79 @@ +From 76e75c11b5085582d97d39ec09409a4011b0849b Mon Sep 17 00:00:00 2001 +From: Jean-Michel Trivi +Date: Mon, 24 Jun 2024 17:29:14 -0700 +Subject: [PATCH] RingtoneManager: allow video ringtone URI + +When checking the MIME type for the default ringtone, also +allow it to refer to video content. + +Bug: 205837340 +Test: see POC + atest android.media.audio.cts.RingtoneManagerTest +(cherry picked from https://googleplex-android-review.googlesource.com/q/commit:de83ef4f51cad7ea1eb91f5d328d79b719251abe) +Merged-In: Iac9f27f14bae29e0fabc31e05da2357f6f4f16c7 +Change-Id: Iac9f27f14bae29e0fabc31e05da2357f6f4f16c7 +--- + media/java/android/media/RingtoneManager.java | 8 ++++++-- + .../android/providers/settings/SettingsProvider.java | 11 +++++++---- + 2 files changed, 13 insertions(+), 6 deletions(-) + +diff --git a/media/java/android/media/RingtoneManager.java b/media/java/android/media/RingtoneManager.java +index 19720c2ed8f9..368d27a174c3 100644 +--- a/media/java/android/media/RingtoneManager.java ++++ b/media/java/android/media/RingtoneManager.java +@@ -803,9 +803,13 @@ public class RingtoneManager { + + " ignored: failure to find mimeType (no access from this context?)"); + return; + } +- if (!(mimeType.startsWith("audio/") || mimeType.equals("application/ogg"))) { ++ if (!(mimeType.startsWith("audio/") || mimeType.equals("application/ogg") ++ || mimeType.equals("application/x-flac") ++ // also check for video ringtones ++ || mimeType.startsWith("video/") || mimeType.equals("application/mp4"))) { + Log.e(TAG, "setActualDefaultRingtoneUri for URI:" + ringtoneUri +- + " ignored: associated mimeType:" + mimeType + " is not an audio type"); ++ + " ignored: associated MIME type:" + mimeType ++ + " is not a recognized audio or video type"); + return; + } + } +diff --git a/packages/SettingsProvider/src/com/android/providers/settings/SettingsProvider.java b/packages/SettingsProvider/src/com/android/providers/settings/SettingsProvider.java +index f80d587058f1..c387b0db79c4 100644 +--- a/packages/SettingsProvider/src/com/android/providers/settings/SettingsProvider.java ++++ b/packages/SettingsProvider/src/com/android/providers/settings/SettingsProvider.java +@@ -1928,7 +1928,7 @@ public class SettingsProvider extends ContentProvider { + cacheName = Settings.System.ALARM_ALERT_CACHE; + } + if (cacheName != null) { +- if (!isValidAudioUri(name, value)) { ++ if (!isValidMediaUri(name, value)) { + return false; + } + final File cacheFile = new File( +@@ -1963,7 +1963,7 @@ public class SettingsProvider extends ContentProvider { + } + } + +- private boolean isValidAudioUri(String name, String uri) { ++ private boolean isValidMediaUri(String name, String uri) { + if (uri != null) { + Uri audioUri = Uri.parse(uri); + if (Settings.AUTHORITY.equals( +@@ -1981,10 +1981,13 @@ public class SettingsProvider extends ContentProvider { + return false; + } + if (!(mimeType.startsWith("audio/") || mimeType.equals("application/ogg") +- || mimeType.equals("application/x-flac"))) { ++ || mimeType.equals("application/x-flac") ++ // also check for video ringtones ++ || mimeType.startsWith("video/") || mimeType.equals("application/mp4"))) { + Slog.e(LOG_TAG, + "mutateSystemSetting for setting: " + name + " URI: " + audioUri +- + " ignored: associated mimeType: " + mimeType + " is not an audio type"); ++ + " ignored: associated MIME type: " + mimeType ++ + " is not a recognized audio or video type"); + return false; + } + } +-- +2.46.1.824.gd892dcdcdd-goog + diff --git a/aosp_diff/preliminary/frameworks/base/99_0228-enforce-limits-for-VisualVoicemailSmsFilterSettings-properties.bulletin.patch b/aosp_diff/preliminary/frameworks/base/99_0228-enforce-limits-for-VisualVoicemailSmsFilterSettings-properties.bulletin.patch new file mode 100644 index 0000000000..fc53b71101 --- /dev/null +++ b/aosp_diff/preliminary/frameworks/base/99_0228-enforce-limits-for-VisualVoicemailSmsFilterSettings-properties.bulletin.patch @@ -0,0 +1,84 @@ +From 6d42793c60a14b2d81d62259b55e9fa9fa9db444 Mon Sep 17 00:00:00 2001 +From: Thomas Stuart +Date: Thu, 6 Jun 2024 22:36:40 +0000 +Subject: [PATCH] enforce limits for VisualVoicemailSmsFilterSettings + properties + +- clientPrefix is now limited to 256 characters +- originatingNumbers is now limited to a list size of 100 and + each element is also limited to 256 characters + +Bug: 308932906 +Test: CTS +(cherry picked from https://googleplex-android-review.googlesource.com/q/commit:8201f7239c50316aa3c36d90e9f90d0a29e74be1) +Merged-In: Id4b4358b141bb211a7e340b979774850b4bd2403 +Change-Id: Id4b4358b141bb211a7e340b979774850b4bd2403 +--- + .../VisualVoicemailSmsFilterSettings.java | 27 +++++++++++++++++++ + 1 file changed, 27 insertions(+) + +diff --git a/telephony/java/android/telephony/VisualVoicemailSmsFilterSettings.java b/telephony/java/android/telephony/VisualVoicemailSmsFilterSettings.java +index eadb726bf63b..2b515c9b5cd1 100644 +--- a/telephony/java/android/telephony/VisualVoicemailSmsFilterSettings.java ++++ b/telephony/java/android/telephony/VisualVoicemailSmsFilterSettings.java +@@ -64,6 +64,14 @@ public final class VisualVoicemailSmsFilterSettings implements Parcelable { + * @hide + */ + public static final int DEFAULT_DESTINATION_PORT = DESTINATION_PORT_ANY; ++ /** ++ * @hide ++ */ ++ public static final int MAX_STRING_LENGTH = 256; ++ /** ++ * @hide ++ */ ++ public static final int MAX_LIST_SIZE = 100; + + /** + * Builder class for {@link VisualVoicemailSmsFilterSettings} objects. +@@ -82,11 +90,16 @@ public final class VisualVoicemailSmsFilterSettings implements Parcelable { + /** + * Sets the client prefix for the visual voicemail SMS filter. The client prefix will appear + * at the start of a visual voicemail SMS message, followed by a colon(:). ++ * @throws IllegalArgumentException if the string length is greater than 256 characters + */ + public Builder setClientPrefix(String clientPrefix) { + if (clientPrefix == null) { + throw new IllegalArgumentException("Client prefix cannot be null"); + } ++ if (clientPrefix.length() > MAX_STRING_LENGTH) { ++ throw new IllegalArgumentException("Client prefix cannot be greater than " ++ + MAX_STRING_LENGTH + " characters"); ++ } + mClientPrefix = clientPrefix; + return this; + } +@@ -95,11 +108,25 @@ public final class VisualVoicemailSmsFilterSettings implements Parcelable { + * Sets the originating number allow list for the visual voicemail SMS filter. If the list + * is not null only the SMS messages from a number in the list can be considered as a visual + * voicemail SMS. Otherwise, messages from any address will be considered. ++ * @throws IllegalArgumentException if the size of the originatingNumbers list is greater ++ * than 100 elements ++ * @throws IllegalArgumentException if an element within the originatingNumbers list has ++ * a string length greater than 256 + */ + public Builder setOriginatingNumbers(List originatingNumbers) { + if (originatingNumbers == null) { + throw new IllegalArgumentException("Originating numbers cannot be null"); + } ++ if (originatingNumbers.size() > MAX_LIST_SIZE) { ++ throw new IllegalArgumentException("The originatingNumbers list size cannot be" ++ + " greater than " + MAX_STRING_LENGTH + " elements"); ++ } ++ for (String num : originatingNumbers) { ++ if (num != null && num.length() > MAX_STRING_LENGTH) { ++ throw new IllegalArgumentException("Numbers within the originatingNumbers list" ++ + " cannot be greater than" + MAX_STRING_LENGTH + " characters"); ++ } ++ } + mOriginatingNumbers = originatingNumbers; + return this; + } +-- +2.46.1.824.gd892dcdcdd-goog + diff --git a/aosp_diff/preliminary/frameworks/base/99_0229-Fix-allowlist-token-issues.bulletin.patch b/aosp_diff/preliminary/frameworks/base/99_0229-Fix-allowlist-token-issues.bulletin.patch new file mode 100644 index 0000000000..07cca76b63 --- /dev/null +++ b/aosp_diff/preliminary/frameworks/base/99_0229-Fix-allowlist-token-issues.bulletin.patch @@ -0,0 +1,704 @@ +From d58fdba12322a63a9230b94aa726c6ec429137be Mon Sep 17 00:00:00 2001 +From: =?UTF-8?q?Mat=C3=ADas=20Hern=C3=A1ndez?= +Date: Tue, 12 Mar 2024 16:51:58 +0100 +Subject: [PATCH] Fix allowlist token issues + +1) Don't accept enqueued notifications with an unexpected token. +2) Ensure allowlist token matches for all parceled and unparceled notifications (by only using the "root notification" one). +3) Simplify cookie usage in allowlist token serialization. +4) Ensure group summary (and any notifications added directly by NMS) have the correct token. + +Bug: 328254922 +Bug: 305695605 +Test: atest NotificationManagerServiceTest ParcelTest CloseSystemDialogsTest + manually +(cherry picked from https://googleplex-android-review.googlesource.com/q/commit:8a7453435b633282a9445ee01a902f090adc138c) +Merged-In: I232e9b74eece745560ed2e762071b48984b3f176 +Change-Id: I232e9b74eece745560ed2e762071b48984b3f176 +--- + core/java/android/app/Notification.java | 48 ++- + core/java/android/os/Parcel.java | 22 ++ + .../coretests/src/android/os/ParcelTest.java | 49 +++ + .../NotificationManagerService.java | 18 +- + .../NotificationManagerServiceTest.java | 354 +++++++++++++++++- + 5 files changed, 474 insertions(+), 17 deletions(-) + mode change 100755 => 100644 services/core/java/com/android/server/notification/NotificationManagerService.java + +diff --git a/core/java/android/app/Notification.java b/core/java/android/app/Notification.java +index 787d920ba992..c7d406f46605 100644 +--- a/core/java/android/app/Notification.java ++++ b/core/java/android/app/Notification.java +@@ -2565,8 +2565,11 @@ public class Notification implements Parcelable + if (mAllowlistToken == null) { + mAllowlistToken = processAllowlistToken; + } +- // Propagate this token to all pending intents that are unmarshalled from the parcel. +- parcel.setClassCookie(PendingIntent.class, mAllowlistToken); ++ // Propagate this token to all pending intents that are unmarshalled from the parcel, ++ // or keep the one we're already propagating, if that's the case. ++ if (!parcel.hasClassCookie(PendingIntent.class)) { ++ parcel.setClassCookie(PendingIntent.class, mAllowlistToken); ++ } + + when = parcel.readLong(); + creationTime = parcel.readLong(); +@@ -3027,9 +3030,24 @@ public class Notification implements Parcelable + }); + } + try { +- // IMPORTANT: Add marshaling code in writeToParcelImpl as we +- // want to intercept all pending events written to the parcel. +- writeToParcelImpl(parcel, flags); ++ boolean mustClearCookie = false; ++ if (!parcel.hasClassCookie(Notification.class)) { ++ // This is the "root" notification, and not an "inner" notification (including ++ // publicVersion or anything else that might be embedded in extras). So we want ++ // to use its token for every inner notification (might be null). ++ parcel.setClassCookie(Notification.class, mAllowlistToken); ++ mustClearCookie = true; ++ } ++ try { ++ // IMPORTANT: Add marshaling code in writeToParcelImpl as we ++ // want to intercept all pending events written to the parcel. ++ writeToParcelImpl(parcel, flags); ++ } finally { ++ if (mustClearCookie) { ++ parcel.removeClassCookie(Notification.class, mAllowlistToken); ++ } ++ } ++ + synchronized (this) { + // Must be written last! + parcel.writeArraySet(allPendingIntents); +@@ -3044,7 +3062,10 @@ public class Notification implements Parcelable + private void writeToParcelImpl(Parcel parcel, int flags) { + parcel.writeInt(1); + +- parcel.writeStrongBinder(mAllowlistToken); ++ // Always use the same token as the root notification (might be null). ++ IBinder rootNotificationToken = (IBinder) parcel.getClassCookie(Notification.class); ++ parcel.writeStrongBinder(rootNotificationToken); ++ + parcel.writeLong(when); + parcel.writeLong(creationTime); + if (mSmallIcon == null && icon != 0) { +@@ -3400,18 +3421,23 @@ public class Notification implements Parcelable + * Sets the token used for background operations for the pending intents associated with this + * notification. + * +- * This token is automatically set during deserialization for you, you usually won't need to +- * call this unless you want to change the existing token, if any. ++ * Note: Should only be invoked by NotificationManagerService, since this is normally ++ * populated by unparceling (and also used there). Any other usage is suspect. + * + * @hide + */ +- public void clearAllowlistToken() { +- mAllowlistToken = null; ++ public void overrideAllowlistToken(IBinder token) { ++ mAllowlistToken = token; + if (publicVersion != null) { +- publicVersion.clearAllowlistToken(); ++ publicVersion.overrideAllowlistToken(token); + } + } + ++ /** @hide */ ++ public IBinder getAllowlistToken() { ++ return mAllowlistToken; ++ } ++ + /** + * @hide + */ +diff --git a/core/java/android/os/Parcel.java b/core/java/android/os/Parcel.java +index 85bf6f6f0bc8..f6a3fed08d7b 100644 +--- a/core/java/android/os/Parcel.java ++++ b/core/java/android/os/Parcel.java +@@ -782,6 +782,28 @@ public final class Parcel { + return mClassCookies != null ? mClassCookies.get(clz) : null; + } + ++ /** @hide */ ++ public void removeClassCookie(Class clz, Object expectedCookie) { ++ if (mClassCookies != null) { ++ Object removedCookie = mClassCookies.remove(clz); ++ if (removedCookie != expectedCookie) { ++ Log.wtf(TAG, "Expected to remove " + expectedCookie + " (with key=" + clz ++ + ") but instead removed " + removedCookie); ++ } ++ } else { ++ Log.wtf(TAG, "Expected to remove " + expectedCookie + " (with key=" + clz ++ + ") but no cookies were present"); ++ } ++ } ++ ++ /** ++ * Whether {@link #setClassCookie} has been called with the specified {@code clz}. ++ * @hide ++ */ ++ public boolean hasClassCookie(Class clz) { ++ return mClassCookies != null && mClassCookies.containsKey(clz); ++ } ++ + /** @hide */ + public final void adoptClassCookies(Parcel from) { + mClassCookies = from.mClassCookies; +diff --git a/core/tests/coretests/src/android/os/ParcelTest.java b/core/tests/coretests/src/android/os/ParcelTest.java +index fdd278b9c621..29d533c6eb7a 100644 +--- a/core/tests/coretests/src/android/os/ParcelTest.java ++++ b/core/tests/coretests/src/android/os/ParcelTest.java +@@ -16,18 +16,23 @@ + + package android.os; + ++import static com.google.common.truth.Truth.assertThat; ++ + import static org.junit.Assert.assertEquals; + import static org.junit.Assert.assertFalse; + import static org.junit.Assert.assertThrows; + import static org.junit.Assert.assertTrue; + + import android.platform.test.annotations.Presubmit; ++import android.util.Log; + + import androidx.test.runner.AndroidJUnit4; + + import org.junit.Test; + import org.junit.runner.RunWith; + ++import java.util.ArrayList; ++ + @Presubmit + @RunWith(AndroidJUnit4.class) + public class ParcelTest { +@@ -239,4 +244,48 @@ public class ParcelTest { + assertThrows(IllegalArgumentException.class, () -> Parcel.compareData(pA, -1, pB, iB, 0)); + assertThrows(IllegalArgumentException.class, () -> Parcel.compareData(pA, 0, pB, -1, 0)); + } ++ ++ @Test ++ public void testClassCookies() { ++ Parcel p = Parcel.obtain(); ++ assertThat(p.hasClassCookie(ParcelTest.class)).isFalse(); ++ ++ p.setClassCookie(ParcelTest.class, "string_cookie"); ++ assertThat(p.hasClassCookie(ParcelTest.class)).isTrue(); ++ assertThat(p.getClassCookie(ParcelTest.class)).isEqualTo("string_cookie"); ++ ++ p.removeClassCookie(ParcelTest.class, "string_cookie"); ++ assertThat(p.hasClassCookie(ParcelTest.class)).isFalse(); ++ assertThat(p.getClassCookie(ParcelTest.class)).isEqualTo(null); ++ ++ p.setClassCookie(ParcelTest.class, "to_be_discarded_cookie"); ++ p.recycle(); ++ assertThat(p.getClassCookie(ParcelTest.class)).isNull(); ++ } ++ ++ @Test ++ public void testClassCookies_removeUnexpected() { ++ Parcel p = Parcel.obtain(); ++ ++ assertLogsWtf(() -> p.removeClassCookie(ParcelTest.class, "not_present")); ++ ++ p.setClassCookie(ParcelTest.class, "value"); ++ ++ assertLogsWtf(() -> p.removeClassCookie(ParcelTest.class, "different")); ++ assertThat(p.getClassCookie(ParcelTest.class)).isNull(); // still removed ++ ++ p.recycle(); ++ } ++ ++ private static void assertLogsWtf(Runnable test) { ++ ArrayList wtfs = new ArrayList<>(); ++ Log.TerribleFailureHandler oldHandler = Log.setWtfHandler( ++ (tag, what, system) -> wtfs.add(what)); ++ try { ++ test.run(); ++ } finally { ++ Log.setWtfHandler(oldHandler); ++ } ++ assertThat(wtfs).hasSize(1); ++ } + } +diff --git a/services/core/java/com/android/server/notification/NotificationManagerService.java b/services/core/java/com/android/server/notification/NotificationManagerService.java +old mode 100755 +new mode 100644 +index 2562d88f8996..af7032095546 +--- a/services/core/java/com/android/server/notification/NotificationManagerService.java ++++ b/services/core/java/com/android/server/notification/NotificationManagerService.java +@@ -640,7 +640,7 @@ public class NotificationManagerService extends SystemService { + + private static final int MY_UID = Process.myUid(); + private static final int MY_PID = Process.myPid(); +- private static final IBinder ALLOWLIST_TOKEN = new Binder(); ++ static final IBinder ALLOWLIST_TOKEN = new Binder(); + protected RankingHandler mRankingHandler; + private long mLastOverRateLogTime; + private float mMaxPackageEnqueueRate = DEFAULT_MAX_NOTIFICATION_ENQUEUE_RATE; +@@ -4369,7 +4369,7 @@ public class NotificationManagerService extends SystemService { + // Remove background token before returning notification to untrusted app, this + // ensures the app isn't able to perform background operations that are + // associated with notification interactions. +- notification.clearAllowlistToken(); ++ notification.overrideAllowlistToken(null); + return new StatusBarNotification( + sbn.getPackageName(), + sbn.getOpPkg(), +@@ -6498,6 +6498,15 @@ public class NotificationManagerService extends SystemService { + + " trying to post for invalid pkg " + pkg + " in user " + incomingUserId); + } + ++ IBinder allowlistToken = notification.getAllowlistToken(); ++ if (allowlistToken != null && allowlistToken != ALLOWLIST_TOKEN) { ++ throw new SecurityException( ++ "Unexpected allowlist token received from " + callingUid); ++ } ++ // allowlistToken is populated by unparceling, so it can be null if the notification was ++ // posted from inside system_server. Ensure it's the expected value. ++ notification.overrideAllowlistToken(ALLOWLIST_TOKEN); ++ + checkRestrictedCategories(notification); + + // Fix the notification as best we can. +@@ -7349,6 +7358,11 @@ public class NotificationManagerService extends SystemService { + @Override + public void run() { + synchronized (mNotificationLock) { ++ // allowlistToken is populated by unparceling, so it will be absent if the ++ // EnqueueNotificationRunnable is created directly by NMS (as we do for group ++ // summaries) instead of via notify(). Fix that. ++ r.getNotification().overrideAllowlistToken(ALLOWLIST_TOKEN); ++ + final Long snoozeAt = + mSnoozeHelper.getSnoozeTimeForUnpostedNotification( + r.getUser().getIdentifier(), +diff --git a/services/tests/uiservicestests/src/com/android/server/notification/NotificationManagerServiceTest.java b/services/tests/uiservicestests/src/com/android/server/notification/NotificationManagerServiceTest.java +index 62adeefb8671..14e9539f0f66 100755 +--- a/services/tests/uiservicestests/src/com/android/server/notification/NotificationManagerServiceTest.java ++++ b/services/tests/uiservicestests/src/com/android/server/notification/NotificationManagerServiceTest.java +@@ -63,6 +63,7 @@ import static android.service.notification.Adjustment.KEY_USER_SENTIMENT; + import static android.service.notification.NotificationListenerService.FLAG_FILTER_TYPE_ALERTING; + import static android.service.notification.NotificationListenerService.FLAG_FILTER_TYPE_CONVERSATIONS; + import static android.service.notification.NotificationListenerService.FLAG_FILTER_TYPE_ONGOING; ++import static android.service.notification.NotificationListenerService.REASON_CANCEL; + import static android.service.notification.NotificationListenerService.REASON_CANCEL_ALL; + import static android.service.notification.NotificationListenerService.Ranking.USER_SENTIMENT_NEGATIVE; + import static android.service.notification.NotificationListenerService.Ranking.USER_SENTIMENT_NEUTRAL; +@@ -108,6 +109,7 @@ import static java.util.Collections.emptyList; + import static java.util.Collections.singletonList; + + import android.annotation.SuppressLint; ++import android.annotation.UserIdInt; + import android.app.ActivityManager; + import android.app.ActivityManagerInternal; + import android.app.AlarmManager; +@@ -161,6 +163,7 @@ import android.os.Bundle; + import android.os.IBinder; + import android.os.Looper; + import android.os.Parcel; ++import android.os.Parcelable; + import android.os.Process; + import android.os.RemoteException; + import android.os.SystemClock; +@@ -223,6 +226,7 @@ import com.android.server.wm.ActivityTaskManagerInternal; + import com.android.server.wm.WindowManagerInternal; + + import com.google.common.collect.ImmutableList; ++import com.google.common.collect.Lists; + + import org.junit.After; + import org.junit.Assert; +@@ -261,7 +265,12 @@ public class NotificationManagerServiceTest extends UiServiceTestCase { + private static final int TEST_TASK_ID = 1; + private static final int UID_HEADLESS = 1000000; + ++ private TestableContext mContext = spy(getContext()); + private final int mUid = Binder.getCallingUid(); ++ private final @UserIdInt int mUserId = UserHandle.getUserId(mUid); ++ private final UserHandle mUser = UserHandle.of(mUserId); ++ private final String mPkg = mContext.getPackageName(); ++ + private TestableNotificationManagerService mService; + private INotificationManager mBinderService; + private NotificationManagerInternal mInternalService; +@@ -279,7 +288,6 @@ public class NotificationManagerServiceTest extends UiServiceTestCase { + @Mock + private PermissionHelper mPermissionHelper; + private NotificationChannelLoggerFake mLogger = new NotificationChannelLoggerFake(); +- private TestableContext mContext = spy(getContext()); + private final String PKG = mContext.getPackageName(); + private TestableLooper mTestableLooper; + @Mock +@@ -1936,7 +1944,7 @@ public class NotificationManagerServiceTest extends UiServiceTestCase { + notif.getNotification().flags |= Notification.FLAG_NO_CLEAR; + mService.addNotification(notif); + mService.cancelAllNotificationsInt(mUid, 0, PKG, null, 0, 0, true, +- notif.getUserId(), 0, null); ++ notif.getUserId(), REASON_CANCEL, null); + waitForIdle(); + StatusBarNotification[] notifs = + mBinderService.getActiveNotifications(notif.getSbn().getPackageName()); +@@ -2633,7 +2641,7 @@ public class NotificationManagerServiceTest extends UiServiceTestCase { + notif.getNotification().flags |= Notification.FLAG_NO_CLEAR; + mService.addNotification(notif); + mService.cancelAllNotificationsInt(mUid, 0, PKG, null, 0, +- Notification.FLAG_ONGOING_EVENT, true, notif.getUserId(), 0, null); ++ Notification.FLAG_ONGOING_EVENT, true, notif.getUserId(), REASON_CANCEL, null); + waitForIdle(); + StatusBarNotification[] notifs = + mBinderService.getActiveNotifications(notif.getSbn().getPackageName()); +@@ -2661,7 +2669,7 @@ public class NotificationManagerServiceTest extends UiServiceTestCase { + notif.getNotification().flags |= Notification.FLAG_ONGOING_EVENT; + mService.addNotification(notif); + mService.cancelAllNotificationsInt(mUid, 0, PKG, null, 0, 0, true, +- notif.getUserId(), 0, null); ++ notif.getUserId(), REASON_CANCEL, null); + waitForIdle(); + StatusBarNotification[] notifs = + mBinderService.getActiveNotifications(notif.getSbn().getPackageName()); +@@ -10284,4 +10292,342 @@ public class NotificationManagerServiceTest extends UiServiceTestCase { + mInternalService.sendReviewPermissionsNotification(); + verify(mMockNm, never()).notify(anyString(), anyInt(), any(Notification.class)); + } ++ ++ @Test ++ public void enqueueNotification_acceptsCorrectToken() throws RemoteException { ++ Notification sent = new Notification.Builder(mContext, TEST_CHANNEL_ID) ++ .setContentIntent(createPendingIntent("content")) ++ .build(); ++ Notification received = parcelAndUnparcel(sent, Notification.CREATOR); ++ assertThat(received.getAllowlistToken()).isEqualTo( ++ NotificationManagerService.ALLOWLIST_TOKEN); ++ ++ mBinderService.enqueueNotificationWithTag(mPkg, mPkg, "tag", 1, ++ parcelAndUnparcel(received, Notification.CREATOR), mUserId); ++ waitForIdle(); ++ ++ assertThat(mService.mNotificationList).hasSize(1); ++ assertThat(mService.mNotificationList.get(0).getNotification().getAllowlistToken()) ++ .isEqualTo(NotificationManagerService.ALLOWLIST_TOKEN); ++ } ++ ++ @Test ++ public void enqueueNotification_acceptsNullToken_andPopulatesIt() throws RemoteException { ++ Notification receivedWithoutParceling = new Notification.Builder(mContext, TEST_CHANNEL_ID) ++ .setContentIntent(createPendingIntent("content")) ++ .build(); ++ assertThat(receivedWithoutParceling.getAllowlistToken()).isNull(); ++ ++ mBinderService.enqueueNotificationWithTag(mPkg, mPkg, "tag", 1, ++ parcelAndUnparcel(receivedWithoutParceling, Notification.CREATOR), mUserId); ++ waitForIdle(); ++ ++ assertThat(mService.mNotificationList).hasSize(1); ++ assertThat(mService.mNotificationList.get(0).getNotification().getAllowlistToken()) ++ .isEqualTo(NotificationManagerService.ALLOWLIST_TOKEN); ++ } ++ ++ @Test ++ public void enqueueNotification_directlyThroughRunnable_populatesAllowlistToken() { ++ Notification receivedWithoutParceling = new Notification.Builder(mContext, TEST_CHANNEL_ID) ++ .setContentIntent(createPendingIntent("content")) ++ .build(); ++ NotificationRecord record = new NotificationRecord( ++ mContext, ++ new StatusBarNotification(mPkg, mPkg, 1, "tag", mUid, 44, receivedWithoutParceling, ++ mUser, "groupKey", 0), ++ mTestNotificationChannel); ++ assertThat(record.getNotification().getAllowlistToken()).isNull(); ++ ++ mWorkerHandler.post( ++ mService.new EnqueueNotificationRunnable(mUserId, record, false, 0)); ++ waitForIdle(); ++ ++ assertThat(mService.mNotificationList).hasSize(1); ++ assertThat(mService.mNotificationList.get(0).getNotification().getAllowlistToken()) ++ .isEqualTo(NotificationManagerService.ALLOWLIST_TOKEN); ++ } ++ ++ @Test ++ public void enqueueNotification_rejectsOtherToken() throws RemoteException { ++ Notification sent = new Notification.Builder(mContext, TEST_CHANNEL_ID) ++ .setContentIntent(createPendingIntent("content")) ++ .build(); ++ sent.overrideAllowlistToken(new Binder()); ++ Notification received = parcelAndUnparcel(sent, Notification.CREATOR); ++ assertThat(received.getAllowlistToken()).isEqualTo(sent.getAllowlistToken()); ++ ++ assertThrows(SecurityException.class, () -> ++ mBinderService.enqueueNotificationWithTag(mPkg, mPkg, "tag", 1, ++ parcelAndUnparcel(received, Notification.CREATOR), mUserId)); ++ waitForIdle(); ++ ++ assertThat(mService.mNotificationList).isEmpty(); ++ } ++ ++ @Test ++ public void enqueueNotification_customParcelingWithFakeInnerToken_hasCorrectTokenInIntents() ++ throws RemoteException { ++ Notification sentFromApp = new Notification.Builder(mContext, TEST_CHANNEL_ID) ++ .setContentIntent(createPendingIntent("content")) ++ .setPublicVersion(new Notification.Builder(mContext, TEST_CHANNEL_ID) ++ .setContentIntent(createPendingIntent("public")) ++ .build()) ++ .build(); ++ sentFromApp.publicVersion.overrideAllowlistToken(new Binder()); ++ ++ // Instead of using the normal parceling, assume the caller parcels it by hand, including a ++ // null token in the outer notification (as would be expected, and as is verified by ++ // enqueue) but trying to sneak in a different one in the inner notification, hoping it gets ++ // propagated to the PendingIntents. ++ Parcel parcelSentFromApp = Parcel.obtain(); ++ writeNotificationToParcelCustom(parcelSentFromApp, sentFromApp, new ArraySet<>( ++ Lists.newArrayList(sentFromApp.contentIntent, ++ sentFromApp.publicVersion.contentIntent))); ++ ++ // Use the unparceling as received in enqueueNotificationWithTag() ++ parcelSentFromApp.setDataPosition(0); ++ Notification receivedByNms = new Notification(parcelSentFromApp); ++ ++ // Verify that all the pendingIntents have the correct token. ++ assertThat(receivedByNms.contentIntent.getWhitelistToken()).isEqualTo( ++ NotificationManagerService.ALLOWLIST_TOKEN); ++ assertThat(receivedByNms.publicVersion.contentIntent.getWhitelistToken()).isEqualTo( ++ NotificationManagerService.ALLOWLIST_TOKEN); ++ } ++ ++ /** ++ * Replicates the behavior of {@link Notification#writeToParcel} but excluding the ++ * "always use the same allowlist token as the root notification" parts. ++ */ ++ private static void writeNotificationToParcelCustom(Parcel parcel, Notification notif, ++ ArraySet allPendingIntents) { ++ int flags = 0; ++ parcel.writeInt(1); // version? ++ ++ parcel.writeStrongBinder(notif.getAllowlistToken()); ++ parcel.writeLong(notif.when); ++ parcel.writeLong(1234L); // notif.creationTime is private ++ if (notif.getSmallIcon() != null) { ++ parcel.writeInt(1); ++ notif.getSmallIcon().writeToParcel(parcel, 0); ++ } else { ++ parcel.writeInt(0); ++ } ++ parcel.writeInt(notif.number); ++ if (notif.contentIntent != null) { ++ parcel.writeInt(1); ++ notif.contentIntent.writeToParcel(parcel, 0); ++ } else { ++ parcel.writeInt(0); ++ } ++ if (notif.deleteIntent != null) { ++ parcel.writeInt(1); ++ notif.deleteIntent.writeToParcel(parcel, 0); ++ } else { ++ parcel.writeInt(0); ++ } ++ if (notif.tickerText != null) { ++ parcel.writeInt(1); ++ TextUtils.writeToParcel(notif.tickerText, parcel, flags); ++ } else { ++ parcel.writeInt(0); ++ } ++ if (notif.tickerView != null) { ++ parcel.writeInt(1); ++ notif.tickerView.writeToParcel(parcel, 0); ++ } else { ++ parcel.writeInt(0); ++ } ++ if (notif.contentView != null) { ++ parcel.writeInt(1); ++ notif.contentView.writeToParcel(parcel, 0); ++ } else { ++ parcel.writeInt(0); ++ } ++ if (notif.getLargeIcon() != null) { ++ parcel.writeInt(1); ++ notif.getLargeIcon().writeToParcel(parcel, 0); ++ } else { ++ parcel.writeInt(0); ++ } ++ ++ parcel.writeInt(notif.defaults); ++ parcel.writeInt(notif.flags); ++ ++ if (notif.sound != null) { ++ parcel.writeInt(1); ++ notif.sound.writeToParcel(parcel, 0); ++ } else { ++ parcel.writeInt(0); ++ } ++ parcel.writeInt(notif.audioStreamType); ++ ++ if (notif.audioAttributes != null) { ++ parcel.writeInt(1); ++ notif.audioAttributes.writeToParcel(parcel, 0); ++ } else { ++ parcel.writeInt(0); ++ } ++ ++ parcel.writeLongArray(notif.vibrate); ++ parcel.writeInt(notif.ledARGB); ++ parcel.writeInt(notif.ledOnMS); ++ parcel.writeInt(notif.ledOffMS); ++ parcel.writeInt(notif.iconLevel); ++ ++ if (notif.fullScreenIntent != null) { ++ parcel.writeInt(1); ++ notif.fullScreenIntent.writeToParcel(parcel, 0); ++ } else { ++ parcel.writeInt(0); ++ } ++ ++ parcel.writeInt(notif.priority); ++ ++ parcel.writeString8(notif.category); ++ ++ parcel.writeString8(notif.getGroup()); ++ ++ parcel.writeString8(notif.getSortKey()); ++ ++ parcel.writeBundle(notif.extras); // null ok ++ ++ parcel.writeTypedArray(notif.actions, 0); // null ok ++ ++ if (notif.bigContentView != null) { ++ parcel.writeInt(1); ++ notif.bigContentView.writeToParcel(parcel, 0); ++ } else { ++ parcel.writeInt(0); ++ } ++ ++ if (notif.headsUpContentView != null) { ++ parcel.writeInt(1); ++ notif.headsUpContentView.writeToParcel(parcel, 0); ++ } else { ++ parcel.writeInt(0); ++ } ++ ++ parcel.writeInt(notif.visibility); ++ ++ if (notif.publicVersion != null) { ++ parcel.writeInt(1); ++ writeNotificationToParcelCustom(parcel, notif.publicVersion, new ArraySet<>()); ++ } else { ++ parcel.writeInt(0); ++ } ++ ++ parcel.writeInt(notif.color); ++ ++ if (notif.getChannelId() != null) { ++ parcel.writeInt(1); ++ parcel.writeString8(notif.getChannelId()); ++ } else { ++ parcel.writeInt(0); ++ } ++ parcel.writeLong(notif.getTimeoutAfter()); ++ ++ if (notif.getShortcutId() != null) { ++ parcel.writeInt(1); ++ parcel.writeString8(notif.getShortcutId()); ++ } else { ++ parcel.writeInt(0); ++ } ++ ++ if (notif.getLocusId() != null) { ++ parcel.writeInt(1); ++ notif.getLocusId().writeToParcel(parcel, 0); ++ } else { ++ parcel.writeInt(0); ++ } ++ ++ parcel.writeInt(notif.getBadgeIconType()); ++ ++ if (notif.getSettingsText() != null) { ++ parcel.writeInt(1); ++ TextUtils.writeToParcel(notif.getSettingsText(), parcel, flags); ++ } else { ++ parcel.writeInt(0); ++ } ++ ++ parcel.writeInt(notif.getGroupAlertBehavior()); ++ ++ if (notif.getBubbleMetadata() != null) { ++ parcel.writeInt(1); ++ notif.getBubbleMetadata().writeToParcel(parcel, 0); ++ } else { ++ parcel.writeInt(0); ++ } ++ ++ parcel.writeBoolean(notif.getAllowSystemGeneratedContextualActions()); ++ ++ parcel.writeInt(Notification.FOREGROUND_SERVICE_DEFAULT); // no getter for mFgsDeferBehavior ++ ++ // mUsesStandardHeader is not written because it should be recomputed in listeners ++ ++ parcel.writeArraySet(allPendingIntents); ++ } ++ ++ @Test ++ @SuppressWarnings("unchecked") ++ public void getActiveNotifications_doesNotLeakAllowlistToken() throws RemoteException { ++ Notification sentFromApp = new Notification.Builder(mContext, TEST_CHANNEL_ID) ++ .setContentIntent(createPendingIntent("content")) ++ .setPublicVersion(new Notification.Builder(mContext, TEST_CHANNEL_ID) ++ .setContentIntent(createPendingIntent("public")) ++ .build()) ++ .extend(new Notification.WearableExtender() ++ .addPage(new Notification.Builder(mContext, TEST_CHANNEL_ID) ++ .setContentIntent(createPendingIntent("wearPage")) ++ .build())) ++ .build(); ++ // Binder transition: app -> NMS ++ Notification receivedByNms = parcelAndUnparcel(sentFromApp, Notification.CREATOR); ++ assertThat(receivedByNms.getAllowlistToken()).isEqualTo( ++ NotificationManagerService.ALLOWLIST_TOKEN); ++ mBinderService.enqueueNotificationWithTag(mPkg, mPkg, "tag", 1, ++ parcelAndUnparcel(receivedByNms, Notification.CREATOR), mUserId); ++ waitForIdle(); ++ assertThat(mService.mNotificationList).hasSize(1); ++ Notification posted = mService.mNotificationList.get(0).getNotification(); ++ assertThat(posted.getAllowlistToken()).isEqualTo( ++ NotificationManagerService.ALLOWLIST_TOKEN); ++ assertThat(posted.contentIntent.getWhitelistToken()).isEqualTo( ++ NotificationManagerService.ALLOWLIST_TOKEN); ++ ++ ParceledListSlice listSentFromNms = ++ mBinderService.getAppActiveNotifications(mPkg, mUserId); ++ // Binder transition: NMS -> app. App doesn't have the allowlist token so clear it ++ // (having a different one would produce the same effect; the relevant thing is to not let ++ // out ALLOWLIST_TOKEN). ++ // Note: for other tests, this is restored by constructing TestableNMS in setup(). ++ Notification.processAllowlistToken = null; ++ ParceledListSlice listReceivedByApp = parcelAndUnparcel( ++ listSentFromNms, ParceledListSlice.CREATOR); ++ Notification gottenBackByApp = listReceivedByApp.getList().get(0).getNotification(); ++ ++ assertThat(gottenBackByApp.getAllowlistToken()).isNull(); ++ assertThat(gottenBackByApp.contentIntent.getWhitelistToken()).isNull(); ++ assertThat(gottenBackByApp.publicVersion.getAllowlistToken()).isNull(); ++ assertThat(gottenBackByApp.publicVersion.contentIntent.getWhitelistToken()).isNull(); ++ assertThat(new Notification.WearableExtender(gottenBackByApp).getPages() ++ .get(0).getAllowlistToken()).isNull(); ++ assertThat(new Notification.WearableExtender(gottenBackByApp).getPages() ++ .get(0).contentIntent.getWhitelistToken()).isNull(); ++ } ++ ++ private static T parcelAndUnparcel(T source, ++ Parcelable.Creator creator) { ++ Parcel parcel = Parcel.obtain(); ++ source.writeToParcel(parcel, 0); ++ parcel.setDataPosition(0); ++ return creator.createFromParcel(parcel); ++ } ++ ++ private PendingIntent createPendingIntent(String action) { ++ return PendingIntent.getActivity(mContext, 0, ++ new Intent(action).setPackage(mContext.getPackageName()), ++ PendingIntent.FLAG_MUTABLE); ++ } + } +-- +2.46.1.824.gd892dcdcdd-goog + diff --git a/aosp_diff/preliminary/frameworks/base/99_0230-Update-checkKeyIntent.bulletin.patch b/aosp_diff/preliminary/frameworks/base/99_0230-Update-checkKeyIntent.bulletin.patch new file mode 100644 index 0000000000..495a06053b --- /dev/null +++ b/aosp_diff/preliminary/frameworks/base/99_0230-Update-checkKeyIntent.bulletin.patch @@ -0,0 +1,54 @@ +From ed032182f93d3930098ff152e5c6562c7be3a512 Mon Sep 17 00:00:00 2001 +From: Dmitry Dementyev +Date: Tue, 1 Oct 2024 14:57:44 -0700 +Subject: [PATCH] Update checkKeyIntent + +1) Explicityly set component after target activity check. +2) Update Intent subclass check. + +Bug: 360846772 +Test: manual +Flag: EXEMPT bugfix +(cherry picked from commit cde345a7ee06db716e613e12a2c218ce248ad1c4) +(cherry picked from https://googleplex-android-review.googlesource.com/q/commit:eaa1db81807eb7631515fccfd4288ff1e170977a) +Merged-In: Ied7961c73299681aa5b523cf3f00fd905893116f +Change-Id: Ied7961c73299681aa5b523cf3f00fd905893116f +--- + .../android/server/accounts/AccountManagerService.java | 9 ++++++--- + 1 file changed, 6 insertions(+), 3 deletions(-) + +diff --git a/services/core/java/com/android/server/accounts/AccountManagerService.java b/services/core/java/com/android/server/accounts/AccountManagerService.java +index fe5375233d8f..70f66cae50f0 100644 +--- a/services/core/java/com/android/server/accounts/AccountManagerService.java ++++ b/services/core/java/com/android/server/accounts/AccountManagerService.java +@@ -4917,6 +4917,8 @@ public class AccountManagerService + Log.e(TAG, String.format(tmpl, activityName, pkgName, mAccountType)); + return false; + } ++ intent.setComponent(targetActivityInfo.getComponentName()); ++ bundle.putParcelable(AccountManager.KEY_INTENT, intent); + return true; + } finally { + Binder.restoreCallingIdentity(bid); +@@ -4938,14 +4940,15 @@ public class AccountManagerService + Bundle simulateBundle = p.readBundle(); + p.recycle(); + Intent intent = bundle.getParcelable(AccountManager.KEY_INTENT, Intent.class); +- if (intent != null && intent.getClass() != Intent.class) { +- return false; +- } + Intent simulateIntent = simulateBundle.getParcelable(AccountManager.KEY_INTENT, + Intent.class); + if (intent == null) { + return (simulateIntent == null); + } ++ if (intent.getClass() != Intent.class || simulateIntent.getClass() != Intent.class) { ++ return false; ++ } ++ + if (!intent.filterEquals(simulateIntent)) { + return false; + } +-- +2.46.1.824.gd892dcdcdd-goog + diff --git a/aosp_diff/preliminary/frameworks/base/99_0231-Pass-SafeActivityOptions-with-actual-caller-for-startActivityInT.bulletin.patch b/aosp_diff/preliminary/frameworks/base/99_0231-Pass-SafeActivityOptions-with-actual-caller-for-startActivityInT.bulletin.patch new file mode 100644 index 0000000000..2740d781ea --- /dev/null +++ b/aosp_diff/preliminary/frameworks/base/99_0231-Pass-SafeActivityOptions-with-actual-caller-for-startActivityInT.bulletin.patch @@ -0,0 +1,71 @@ +From aa31697ed1de3b525a668d8954eabf3e75401e5d Mon Sep 17 00:00:00 2001 +From: Chris Li +Date: Wed, 9 Oct 2024 01:50:57 +0000 +Subject: [PATCH] Pass SafeActivityOptions with actual caller for + startActivityInTF + +We clearCallingUid before apply the WCT, but SafeActivityOptions will +query the Binder Uid when construct. Update to pass in the actual +caller. + +Flag: EXEMPT bug fix +Bug: 369103643 +Test: atest WmTests:WindowOrganizerTests# + testStartActivityInTaskFragment_checkCallerPermission +(cherry picked from commit 20c568e77eae5d469cd5e594b644d8645d830dbd) +(cherry picked from https://googleplex-android-review.googlesource.com/q/commit:6c76c394194d40d2704126181313d0a640979606) +Merged-In: I873ae576de0bc4a7402c2f522b45853bce48a0c5 +Change-Id: I873ae576de0bc4a7402c2f522b45853bce48a0c5 +--- + .../java/com/android/server/wm/ActivityStartController.java | 5 ++--- + .../com/android/server/wm/WindowOrganizerController.java | 4 +++- + 2 files changed, 5 insertions(+), 4 deletions(-) + +diff --git a/services/core/java/com/android/server/wm/ActivityStartController.java b/services/core/java/com/android/server/wm/ActivityStartController.java +index 32ed4725b3b4..d51b90ef1ec0 100644 +--- a/services/core/java/com/android/server/wm/ActivityStartController.java ++++ b/services/core/java/com/android/server/wm/ActivityStartController.java +@@ -39,7 +39,6 @@ import android.content.pm.ApplicationInfo; + import android.content.pm.PackageManager; + import android.content.pm.ResolveInfo; + import android.os.Binder; +-import android.os.Bundle; + import android.os.IBinder; + import android.os.UserHandle; + import android.provider.Settings; +@@ -500,14 +499,14 @@ public class ActivityStartController { + * Starts an activity in the TaskFragment. + * @param taskFragment TaskFragment {@link TaskFragment} to start the activity in. + * @param activityIntent intent to start the activity. +- * @param activityOptions ActivityOptions to start the activity with. ++ * @param activityOptions SafeActivityOptions to start the activity with. + * @param resultTo the caller activity + * @param callingUid the caller uid + * @param callingPid the caller pid + * @return the start result. + */ + int startActivityInTaskFragment(@NonNull TaskFragment taskFragment, +- @NonNull Intent activityIntent, @Nullable Bundle activityOptions, ++ @NonNull Intent activityIntent, @Nullable SafeActivityOptions activityOptions, + @Nullable IBinder resultTo, int callingUid, int callingPid, + @Nullable IBinder errorCallbackToken) { + final ActivityRecord caller = +diff --git a/services/core/java/com/android/server/wm/WindowOrganizerController.java b/services/core/java/com/android/server/wm/WindowOrganizerController.java +index 0bd28467d649..b7416e0cc97c 100644 +--- a/services/core/java/com/android/server/wm/WindowOrganizerController.java ++++ b/services/core/java/com/android/server/wm/WindowOrganizerController.java +@@ -721,8 +721,10 @@ class WindowOrganizerController extends IWindowOrganizerController.Stub + } + final Intent activityIntent = hop.getActivityIntent(); + final Bundle activityOptions = hop.getLaunchOptions(); ++ final SafeActivityOptions safeOptions = ++ SafeActivityOptions.fromBundle(activityOptions, caller.mPid, caller.mUid); + final int result = mService.getActivityStartController() +- .startActivityInTaskFragment(tf, activityIntent, activityOptions, ++ .startActivityInTaskFragment(tf, activityIntent, safeOptions, + hop.getCallingActivity(), caller.mUid, caller.mPid, + errorCallbackToken); + if (!isStartResultSuccessful(result)) { +-- +2.46.1.824.gd892dcdcdd-goog + diff --git a/aosp_diff/preliminary/frameworks/base/99_0232-Always-show-all-approved-apps.bulletin.patch b/aosp_diff/preliminary/frameworks/base/99_0232-Always-show-all-approved-apps.bulletin.patch new file mode 100644 index 0000000000..fc6562c02f --- /dev/null +++ b/aosp_diff/preliminary/frameworks/base/99_0232-Always-show-all-approved-apps.bulletin.patch @@ -0,0 +1,201 @@ +From 42d3fee9c4049a2da2792473d79b0cd6daf40942 Mon Sep 17 00:00:00 2001 +From: Julia Reynolds +Date: Mon, 7 Oct 2024 11:10:31 -0400 +Subject: [PATCH] Always show all approved apps + +Regardless of what the current criteria is in order to be approved, +show everything that's currently approved, since the criteria might +have been more lax when it was approved + +Test: manual +Test: ServiceListingTest +Flag: EXEMPT bug fix +Bug: 365738306 +(cherry picked from commit 234c5e843ca427b1dd47e91e3969f3309dd787bf) +(cherry picked from https://googleplex-android-review.googlesource.com/q/commit:0cafbe4e9bf9875709e9296e5d2fd5a7666364a2) +Merged-In: I6c19d3dbff6ecabc74729a7f021f293e26601944 +Change-Id: I6c19d3dbff6ecabc74729a7f021f293e26601944 +--- + .../applications/ServiceListing.java | 32 ++++++--- + .../applications/ServiceListingTest.java | 66 ++++++++++++++++++- + 2 files changed, 88 insertions(+), 10 deletions(-) + +diff --git a/packages/SettingsLib/src/com/android/settingslib/applications/ServiceListing.java b/packages/SettingsLib/src/com/android/settingslib/applications/ServiceListing.java +index c8bcabff1094..261c722e517c 100644 +--- a/packages/SettingsLib/src/com/android/settingslib/applications/ServiceListing.java ++++ b/packages/SettingsLib/src/com/android/settingslib/applications/ServiceListing.java +@@ -138,23 +138,37 @@ public class ServiceListing { + } + + final PackageManager pmWrapper = mContext.getPackageManager(); ++ // Add requesting apps, with full validation + List installedServices = pmWrapper.queryIntentServicesAsUser( + new Intent(mIntentAction), flags, user); + for (ResolveInfo resolveInfo : installedServices) { + ServiceInfo info = resolveInfo.serviceInfo; + +- if (!mPermission.equals(info.permission)) { +- Slog.w(mTag, "Skipping " + mNoun + " service " +- + info.packageName + "/" + info.name +- + ": it does not require the permission " +- + mPermission); +- continue; ++ if (!mEnabledServices.contains(info.getComponentName())) { ++ if (!mPermission.equals(info.permission)) { ++ Slog.w(mTag, "Skipping " + mNoun + " service " ++ + info.packageName + "/" + info.name ++ + ": it does not require the permission " ++ + mPermission); ++ continue; ++ } ++ if (mValidator != null && !mValidator.test(info)) { ++ continue; ++ } ++ mServices.add(info); + } +- if (mValidator != null && !mValidator.test(info)) { +- continue; ++ } ++ ++ // Add all apps with access, in case prior approval was granted without full validation ++ for (ComponentName cn : mEnabledServices) { ++ List enabledServices = pmWrapper.queryIntentServicesAsUser( ++ new Intent().setComponent(cn), flags, user); ++ for (ResolveInfo resolveInfo : enabledServices) { ++ ServiceInfo info = resolveInfo.serviceInfo; ++ mServices.add(info); + } +- mServices.add(info); + } ++ + for (Callback callback : mCallbacks) { + callback.onServicesReloaded(mServices); + } +diff --git a/packages/SettingsLib/tests/robotests/src/com/android/settingslib/applications/ServiceListingTest.java b/packages/SettingsLib/tests/robotests/src/com/android/settingslib/applications/ServiceListingTest.java +index 7ff0988c494d..feef559dfe26 100644 +--- a/packages/SettingsLib/tests/robotests/src/com/android/settingslib/applications/ServiceListingTest.java ++++ b/packages/SettingsLib/tests/robotests/src/com/android/settingslib/applications/ServiceListingTest.java +@@ -21,6 +21,7 @@ import static com.google.common.truth.Truth.assertThat; + import static org.mockito.ArgumentMatchers.any; + import static org.mockito.ArgumentMatchers.anyInt; + import static org.mockito.ArgumentMatchers.anyList; ++import static org.mockito.ArgumentMatchers.argThat; + import static org.mockito.Mockito.mock; + import static org.mockito.Mockito.spy; + import static org.mockito.Mockito.times; +@@ -29,6 +30,7 @@ import static org.mockito.Mockito.when; + + import android.content.ComponentName; + import android.content.Context; ++import android.content.Intent; + import android.content.pm.PackageManager; + import android.content.pm.ResolveInfo; + import android.content.pm.ServiceInfo; +@@ -42,6 +44,7 @@ import org.junit.Before; + import org.junit.Test; + import org.junit.runner.RunWith; + import org.mockito.ArgumentCaptor; ++import org.mockito.ArgumentMatcher; + import org.robolectric.RobolectricTestRunner; + import org.robolectric.RuntimeEnvironment; + +@@ -72,19 +75,26 @@ public class ServiceListingTest { + .build(); + } + ++ private ArgumentMatcher filterEquals(Intent intent) { ++ return (test) -> { ++ return intent.filterEquals(test); ++ }; ++ } ++ + @Test + public void testValidator() { + ServiceInfo s1 = new ServiceInfo(); + s1.permission = "testPermission"; + s1.packageName = "pkg"; ++ s1.name = "Service1"; + ServiceInfo s2 = new ServiceInfo(); + s2.permission = "testPermission"; + s2.packageName = "pkg2"; ++ s2.name = "service2"; + ResolveInfo r1 = new ResolveInfo(); + r1.serviceInfo = s1; + ResolveInfo r2 = new ResolveInfo(); + r2.serviceInfo = s2; +- + when(mPm.queryIntentServicesAsUser(any(), anyInt(), anyInt())).thenReturn( + ImmutableList.of(r1, r2)); + +@@ -118,9 +128,11 @@ public class ServiceListingTest { + ServiceInfo s1 = new ServiceInfo(); + s1.permission = "testPermission"; + s1.packageName = "pkg"; ++ s1.name = "Service1"; + ServiceInfo s2 = new ServiceInfo(); + s2.permission = "testPermission"; + s2.packageName = "pkg2"; ++ s2.name = "service2"; + ResolveInfo r1 = new ResolveInfo(); + r1.serviceInfo = s1; + ResolveInfo r2 = new ResolveInfo(); +@@ -193,4 +205,56 @@ public class ServiceListingTest { + assertThat(Settings.Secure.getString(RuntimeEnvironment.application.getContentResolver(), + TEST_SETTING)).contains(testComponent2.flattenToString()); + } ++ ++ @Test ++ public void testHasPermissionWithoutMeetingCurrentRegs() { ++ ServiceInfo s1 = new ServiceInfo(); ++ s1.permission = "testPermission"; ++ s1.packageName = "pkg"; ++ s1.name = "Service1"; ++ ServiceInfo s2 = new ServiceInfo(); ++ s2.permission = "testPermission"; ++ s2.packageName = "pkg2"; ++ s2.name = "service2"; ++ ResolveInfo r1 = new ResolveInfo(); ++ r1.serviceInfo = s1; ++ ResolveInfo r2 = new ResolveInfo(); ++ r2.serviceInfo = s2; ++ ++ ComponentName approvedComponent = new ComponentName(s2.packageName, s2.name); ++ ++ Settings.Secure.putString( ++ mContext.getContentResolver(), TEST_SETTING, approvedComponent.flattenToString()); ++ ++ when(mPm.queryIntentServicesAsUser(argThat( ++ filterEquals(new Intent(TEST_INTENT))), anyInt(), anyInt())) ++ .thenReturn(ImmutableList.of(r1)); ++ when(mPm.queryIntentServicesAsUser(argThat( ++ filterEquals(new Intent().setComponent(approvedComponent))), ++ anyInt(), anyInt())) ++ .thenReturn(ImmutableList.of(r2)); ++ ++ mServiceListing = new ServiceListing.Builder(mContext) ++ .setTag("testTag") ++ .setSetting(TEST_SETTING) ++ .setNoun("testNoun") ++ .setIntentAction(TEST_INTENT) ++ .setValidator(info -> { ++ if (info.packageName.equals("pkg")) { ++ return true; ++ } ++ return false; ++ }) ++ .setPermission("testPermission") ++ .build(); ++ ServiceListing.Callback callback = mock(ServiceListing.Callback.class); ++ mServiceListing.addCallback(callback); ++ mServiceListing.reload(); ++ ++ verify(mPm, times(2)).queryIntentServicesAsUser(any(), anyInt(), anyInt()); ++ ArgumentCaptor> captor = ArgumentCaptor.forClass(List.class); ++ verify(callback, times(1)).onServicesReloaded(captor.capture()); ++ ++ assertThat(captor.getValue()).containsExactlyElementsIn(ImmutableList.of(s2, s1)); ++ } + } +-- +2.46.1.824.gd892dcdcdd-goog + diff --git a/aosp_diff/preliminary/frameworks/native/11_0011-binder-fix-FD-handling-in-continueWrite.bulletin.patch b/aosp_diff/preliminary/frameworks/native/11_0011-binder-fix-FD-handling-in-continueWrite.bulletin.patch new file mode 100644 index 0000000000..d5a5980015 --- /dev/null +++ b/aosp_diff/preliminary/frameworks/native/11_0011-binder-fix-FD-handling-in-continueWrite.bulletin.patch @@ -0,0 +1,349 @@ +From 1d5f81bdd339e4468b2f476d609b7556a00ed9b3 Mon Sep 17 00:00:00 2001 +From: Frederick Mayle +Date: Mon, 14 Oct 2024 17:12:30 -0700 +Subject: [PATCH] binder: fix FD handling in continueWrite + +Only close FDs within the truncated part of the parcel. + +This change also fixes a bug where a parcel truncated into the middle of +an object would not properly free that object. That could have resulted +in an OOB access in `Parcel::truncateRpcObjects`, so more bounds +checking is added. + +The new tests show how to reproduce the bug by appending to or partially +truncating Parcels owned by the kernel. Two cases are disabled because +of a bug in the Parcel fdsan code (b/370824489). + +Cherry-pick notes: Add truncateFileDescriptors method instead of +modifying closeFileDescriptors to avoid API change errors. Large diffs +in this branch because it didn't have the disruptive RPC FD support, +main diff is that the closeFileDescriptors call is move out of the +mOwner implementation. Tweaked the test to support older C++ and +android-base libs. + +Flag: EXEMPT bugfix +Ignore-AOSP-First: security fix +Bug: 239222407, 359179312 +Test: atest binderLibTest +(cherry picked from https://googleplex-android-review.googlesource.com/q/commit:0e06c131798a1e908a9875d3c7515bb8901d3ebe) +Merged-In: Iadf7e2e98e3eb97c56ec2fed2b49d1e6492af9a3 +Change-Id: Iadf7e2e98e3eb97c56ec2fed2b49d1e6492af9a3 +--- + libs/binder/IPCThreadState.cpp | 3 +- + libs/binder/Parcel.cpp | 20 +++- + libs/binder/include/binder/Parcel.h | 3 + + libs/binder/tests/binderLibTest.cpp | 162 ++++++++++++++++++++++++++++ + 4 files changed, 184 insertions(+), 4 deletions(-) + +diff --git a/libs/binder/IPCThreadState.cpp b/libs/binder/IPCThreadState.cpp +index 3c97dcab93..a62425e984 100644 +--- a/libs/binder/IPCThreadState.cpp ++++ b/libs/binder/IPCThreadState.cpp +@@ -1443,7 +1443,7 @@ status_t IPCThreadState::freeze(pid_t pid, bool enable, uint32_t timeout_ms) { + return ret; + } + +-void IPCThreadState::freeBuffer(Parcel* parcel, const uint8_t* data, ++void IPCThreadState::freeBuffer(Parcel* /*parcel*/, const uint8_t* data, + size_t /*dataSize*/, + const binder_size_t* /*objects*/, + size_t /*objectsSize*/) +@@ -1453,7 +1453,6 @@ void IPCThreadState::freeBuffer(Parcel* parcel, const uint8_t* data, + alog << "Writing BC_FREE_BUFFER for " << data << endl; + } + ALOG_ASSERT(data != NULL, "Called with NULL data"); +- if (parcel != nullptr) parcel->closeFileDescriptors(); + IPCThreadState* state = self(); + state->mOut.writeInt32(BC_FREE_BUFFER); + state->mOut.writePointer((uintptr_t)data); +diff --git a/libs/binder/Parcel.cpp b/libs/binder/Parcel.cpp +index 58b0b35323..dd370caf1f 100644 +--- a/libs/binder/Parcel.cpp ++++ b/libs/binder/Parcel.cpp +@@ -2193,11 +2193,15 @@ const flat_binder_object* Parcel::readObject(bool nullMetaData) const + + void Parcel::closeFileDescriptors() + { ++ truncateFileDescriptors(0); ++} ++ ++void Parcel::truncateFileDescriptors(size_t newObjectsSize) { + size_t i = mObjectsSize; + if (i > 0) { + //ALOGI("Closing file descriptors for %zu objects...", i); + } +- while (i > 0) { ++ while (i > newObjectsSize) { + i--; + const flat_binder_object* flat + = reinterpret_cast(mData+mObjects[i]); +@@ -2345,6 +2349,7 @@ void Parcel::freeDataNoInit() + if (mOwner) { + LOG_ALLOC("Parcel %p: freeing other owner data", this); + //ALOGI("Freeing data ref of %p (pid=%d)", this, getpid()); ++ closeFileDescriptors(); + mOwner(this, mData, mDataSize, mObjects, mObjectsSize); + } else { + LOG_ALLOC("Parcel %p: freeing allocated data", this); +@@ -2460,8 +2465,9 @@ status_t Parcel::continueWrite(size_t desired) + if (desired == 0) { + objectsSize = 0; + } else { ++ validateReadData(mDataSize); // hack to sort the objects + while (objectsSize > 0) { +- if (mObjects[objectsSize-1] < desired) ++ if (mObjects[objectsSize-1] + sizeof(flat_binder_object) <= desired) + break; + objectsSize--; + } +@@ -2506,8 +2512,18 @@ status_t Parcel::continueWrite(size_t desired) + } + if (objects && mObjects) { + memcpy(objects, mObjects, objectsSize*sizeof(binder_size_t)); ++ // All FDs are owned when `mOwner`, even when `cookie == 0`. When ++ // we switch to `!mOwner`, we need to explicitly mark the FDs as ++ // owned. ++ for (size_t i = 0; i < objectsSize; i++) { ++ flat_binder_object* flat = reinterpret_cast(data + objects[i]); ++ if (flat->hdr.type == BINDER_TYPE_FD) { ++ flat->cookie = 1; ++ } ++ } + } + //ALOGI("Freeing data ref of %p (pid=%d)", this, getpid()); ++ truncateFileDescriptors(objectsSize); + mOwner(this, mData, mDataSize, mObjects, mObjectsSize); + mOwner = nullptr; + +diff --git a/libs/binder/include/binder/Parcel.h b/libs/binder/include/binder/Parcel.h +index e2b2c5128d..1b3b6f77d4 100644 +--- a/libs/binder/include/binder/Parcel.h ++++ b/libs/binder/include/binder/Parcel.h +@@ -592,6 +592,9 @@ public: + void print(TextOutput& to, uint32_t flags = 0) const; + + private: ++ // Close all file descriptors in the parcel at object positions >= newObjectsSize. ++ __attribute__((__visibility__("hidden"))) void truncateFileDescriptors(size_t newObjectsSize); ++ + typedef void (*release_func)(Parcel* parcel, + const uint8_t* data, size_t dataSize, + const binder_size_t* objects, size_t objectsSize); +diff --git a/libs/binder/tests/binderLibTest.cpp b/libs/binder/tests/binderLibTest.cpp +index b1e17b7892..adb5d4f162 100644 +--- a/libs/binder/tests/binderLibTest.cpp ++++ b/libs/binder/tests/binderLibTest.cpp +@@ -27,6 +27,7 @@ + #include + #include + ++#include + #include + #include + #include +@@ -42,6 +43,7 @@ + + #include + #include ++#include + #include + #include + #include +@@ -56,6 +58,7 @@ using namespace std::string_literals; + using namespace std::chrono_literals; + using android::base::testing::HasValue; + using android::base::testing::Ok; ++using android::base::unique_fd; + using testing::ExplainMatchResult; + using testing::Matcher; + using testing::Not; +@@ -104,6 +107,8 @@ enum BinderLibTestTranscationCode { + BINDER_LIB_TEST_LINK_DEATH_TRANSACTION, + BINDER_LIB_TEST_WRITE_FILE_TRANSACTION, + BINDER_LIB_TEST_WRITE_PARCEL_FILE_DESCRIPTOR_TRANSACTION, ++ BINDER_LIB_TEST_GET_FILE_DESCRIPTORS_OWNED_TRANSACTION, ++ BINDER_LIB_TEST_GET_FILE_DESCRIPTORS_UNOWNED_TRANSACTION, + BINDER_LIB_TEST_EXIT_TRANSACTION, + BINDER_LIB_TEST_DELAYED_EXIT_TRANSACTION, + BINDER_LIB_TEST_GET_PTR_SIZE_TRANSACTION, +@@ -437,6 +442,35 @@ class TestDeathRecipient : public IBinder::DeathRecipient, public BinderLibTestE + }; + }; + ++ssize_t countFds() { ++ DIR* dir = opendir("/proc/self/fd/"); ++ if (dir == nullptr) return -1; ++ ssize_t ret = 0; ++ dirent* ent; ++ while ((ent = readdir(dir)) != nullptr) ret++; ++ closedir(dir); ++ return ret; ++} ++ ++struct FdLeakDetector { ++ int startCount; ++ ++ FdLeakDetector() { ++ // This log statement is load bearing. We have to log something before ++ // counting FDs to make sure the logging system is initialized, otherwise ++ // the sockets it opens will look like a leak. ++ ALOGW("FdLeakDetector counting FDs."); ++ startCount = countFds(); ++ } ++ ~FdLeakDetector() { ++ int endCount = countFds(); ++ if (startCount != endCount) { ++ ADD_FAILURE() << "fd count changed (" << startCount << " -> " << endCount ++ << ") fd leak?"; ++ } ++ } ++}; ++ + TEST_F(BinderLibTest, CannotUseBinderAfterFork) { + // EXPECT_DEATH works by forking the process + EXPECT_DEATH({ ProcessState::self(); }, "libbinder ProcessState can not be used after fork"); +@@ -852,6 +886,100 @@ TEST_F(BinderLibTest, PassParcelFileDescriptor) { + EXPECT_EQ(0, read(read_end.get(), readbuf.data(), datasize)); + } + ++TEST_F(BinderLibTest, RecvOwnedFileDescriptors) { ++ FdLeakDetector fd_leak_detector; ++ ++ Parcel data; ++ Parcel reply; ++ EXPECT_EQ(NO_ERROR, ++ m_server->transact(BINDER_LIB_TEST_GET_FILE_DESCRIPTORS_OWNED_TRANSACTION, data, ++ &reply)); ++ unique_fd a, b; ++ EXPECT_EQ(OK, reply.readUniqueFileDescriptor(&a)); ++ EXPECT_EQ(OK, reply.readUniqueFileDescriptor(&b)); ++} ++ ++// Used to trigger fdsan error (b/239222407). ++TEST_F(BinderLibTest, RecvOwnedFileDescriptorsAndWriteInt) { ++ GTEST_SKIP() << "triggers fdsan false positive: b/370824489"; ++ ++ FdLeakDetector fd_leak_detector; ++ ++ Parcel data; ++ Parcel reply; ++ EXPECT_EQ(NO_ERROR, ++ m_server->transact(BINDER_LIB_TEST_GET_FILE_DESCRIPTORS_OWNED_TRANSACTION, data, ++ &reply)); ++ reply.setDataPosition(reply.dataSize()); ++ reply.writeInt32(0); ++ reply.setDataPosition(0); ++ unique_fd a, b; ++ EXPECT_EQ(OK, reply.readUniqueFileDescriptor(&a)); ++ EXPECT_EQ(OK, reply.readUniqueFileDescriptor(&b)); ++} ++ ++// Used to trigger fdsan error (b/239222407). ++TEST_F(BinderLibTest, RecvOwnedFileDescriptorsAndTruncate) { ++ GTEST_SKIP() << "triggers fdsan false positive: b/370824489"; ++ ++ FdLeakDetector fd_leak_detector; ++ ++ Parcel data; ++ Parcel reply; ++ EXPECT_EQ(NO_ERROR, ++ m_server->transact(BINDER_LIB_TEST_GET_FILE_DESCRIPTORS_OWNED_TRANSACTION, data, ++ &reply)); ++ reply.setDataSize(reply.dataSize() - sizeof(flat_binder_object)); ++ unique_fd a, b; ++ EXPECT_EQ(OK, reply.readUniqueFileDescriptor(&a)); ++ EXPECT_EQ(BAD_TYPE, reply.readUniqueFileDescriptor(&b)); ++} ++ ++TEST_F(BinderLibTest, RecvUnownedFileDescriptors) { ++ FdLeakDetector fd_leak_detector; ++ ++ Parcel data; ++ Parcel reply; ++ EXPECT_EQ(NO_ERROR, ++ m_server->transact(BINDER_LIB_TEST_GET_FILE_DESCRIPTORS_UNOWNED_TRANSACTION, data, ++ &reply)); ++ unique_fd a, b; ++ EXPECT_EQ(OK, reply.readUniqueFileDescriptor(&a)); ++ EXPECT_EQ(OK, reply.readUniqueFileDescriptor(&b)); ++} ++ ++// Used to trigger fdsan error (b/239222407). ++TEST_F(BinderLibTest, RecvUnownedFileDescriptorsAndWriteInt) { ++ FdLeakDetector fd_leak_detector; ++ ++ Parcel data; ++ Parcel reply; ++ EXPECT_EQ(NO_ERROR, ++ m_server->transact(BINDER_LIB_TEST_GET_FILE_DESCRIPTORS_UNOWNED_TRANSACTION, data, ++ &reply)); ++ reply.setDataPosition(reply.dataSize()); ++ reply.writeInt32(0); ++ reply.setDataPosition(0); ++ unique_fd a, b; ++ EXPECT_EQ(OK, reply.readUniqueFileDescriptor(&a)); ++ EXPECT_EQ(OK, reply.readUniqueFileDescriptor(&b)); ++} ++ ++// Used to trigger fdsan error (b/239222407). ++TEST_F(BinderLibTest, RecvUnownedFileDescriptorsAndTruncate) { ++ FdLeakDetector fd_leak_detector; ++ ++ Parcel data; ++ Parcel reply; ++ EXPECT_EQ(NO_ERROR, ++ m_server->transact(BINDER_LIB_TEST_GET_FILE_DESCRIPTORS_UNOWNED_TRANSACTION, data, ++ &reply)); ++ reply.setDataSize(reply.dataSize() - sizeof(flat_binder_object)); ++ unique_fd a, b; ++ EXPECT_EQ(OK, reply.readUniqueFileDescriptor(&a)); ++ EXPECT_EQ(BAD_TYPE, reply.readUniqueFileDescriptor(&b)); ++} ++ + TEST_F(BinderLibTest, PromoteLocal) { + sp strong = new BBinder(); + wp weak = strong; +@@ -1600,6 +1728,40 @@ public: + if (ret != size) return UNKNOWN_ERROR; + return NO_ERROR; + } ++ case BINDER_LIB_TEST_GET_FILE_DESCRIPTORS_OWNED_TRANSACTION: { ++ unique_fd fd1(memfd_create("memfd1", MFD_CLOEXEC)); ++ if (!fd1.ok()) { ++ PLOG(ERROR) << "memfd_create failed"; ++ return UNKNOWN_ERROR; ++ } ++ unique_fd fd2(memfd_create("memfd2", MFD_CLOEXEC)); ++ if (!fd2.ok()) { ++ PLOG(ERROR) << "memfd_create failed"; ++ return UNKNOWN_ERROR; ++ } ++ status_t ret; ++ ret = reply->writeFileDescriptor(fd1.release(), true); ++ if (ret != NO_ERROR) { ++ return ret; ++ } ++ ret = reply->writeFileDescriptor(fd2.release(), true); ++ if (ret != NO_ERROR) { ++ return ret; ++ } ++ return NO_ERROR; ++ } ++ case BINDER_LIB_TEST_GET_FILE_DESCRIPTORS_UNOWNED_TRANSACTION: { ++ status_t ret; ++ ret = reply->writeFileDescriptor(STDOUT_FILENO, false); ++ if (ret != NO_ERROR) { ++ return ret; ++ } ++ ret = reply->writeFileDescriptor(STDERR_FILENO, false); ++ if (ret != NO_ERROR) { ++ return ret; ++ } ++ return NO_ERROR; ++ } + case BINDER_LIB_TEST_DELAYED_EXIT_TRANSACTION: + alarm(10); + return NO_ERROR; +-- +2.46.1.824.gd892dcdcdd-goog + diff --git a/aosp_diff/preliminary/frameworks/native/12_0012-libbinder-Parcel-grow-rejects-large-data-pos.bulletin.patch b/aosp_diff/preliminary/frameworks/native/12_0012-libbinder-Parcel-grow-rejects-large-data-pos.bulletin.patch new file mode 100644 index 0000000000..91c9cea044 --- /dev/null +++ b/aosp_diff/preliminary/frameworks/native/12_0012-libbinder-Parcel-grow-rejects-large-data-pos.bulletin.patch @@ -0,0 +1,41 @@ +From 24a26f39f3faac82732d905857da825eeb6d87f4 Mon Sep 17 00:00:00 2001 +From: Steven Moreland +Date: Wed, 2 Oct 2024 01:00:23 +0000 +Subject: [PATCH] libbinder: Parcel: grow rejects large data pos + +This is unexpected behavior so throw an error. +Allocating this much memory may cause OOM or +other issues. + +Bug: 370831157 +Test: fuzzer +(cherry picked from commit 608524d462278c2c9f6716cd94f126c85e9f2e91) +(cherry picked from https://googleplex-android-review.googlesource.com/q/commit:e4f04ab7dff943d91240e02ebb2287278d1c40c1) +Merged-In: Iea0884ca61b08e52e6a6e9c66693e427cb5536f4 +Change-Id: Iea0884ca61b08e52e6a6e9c66693e427cb5536f4 +--- + libs/binder/Parcel.cpp | 8 ++++++++ + 1 file changed, 8 insertions(+) + +diff --git a/libs/binder/Parcel.cpp b/libs/binder/Parcel.cpp +index dd370caf1f..0264b1cfad 100644 +--- a/libs/binder/Parcel.cpp ++++ b/libs/binder/Parcel.cpp +@@ -2375,6 +2375,14 @@ status_t Parcel::growData(size_t len) + return BAD_VALUE; + } + ++ if (mDataPos > mDataSize) { ++ // b/370831157 - this case used to abort. We also don't expect mDataPos < mDataSize, but ++ // this would only waste a bit of memory, so it's okay. ++ ALOGE("growData only expected at the end of a Parcel. pos: %zu, size: %zu, capacity: %zu", ++ mDataPos, len, mDataCapacity); ++ return BAD_VALUE; ++ } ++ + if (len > SIZE_MAX - mDataSize) return NO_MEMORY; // overflow + if (mDataSize + len > SIZE_MAX / 3) return NO_MEMORY; // overflow + size_t newSize = ((mDataSize+len)*3)/2; +-- +2.46.1.824.gd892dcdcdd-goog + diff --git a/aosp_diff/preliminary/frameworks/native/13_0013-libbinder-Parcel-validate-read-data-before-write.bulletin.patch b/aosp_diff/preliminary/frameworks/native/13_0013-libbinder-Parcel-validate-read-data-before-write.bulletin.patch new file mode 100644 index 0000000000..5d74adb0ce --- /dev/null +++ b/aosp_diff/preliminary/frameworks/native/13_0013-libbinder-Parcel-validate-read-data-before-write.bulletin.patch @@ -0,0 +1,59 @@ +From 0b4de68bd7e05228755f01777fb9f1d19e93fe5f Mon Sep 17 00:00:00 2001 +From: Steven Moreland +Date: Wed, 2 Oct 2024 00:37:59 +0000 +Subject: [PATCH] libbinder: Parcel: validate read data before write + +This is slow, but it's required to prevent memory +corruption. + +Ignore-AOSP-First: security +Bug: 370840874 +Test: fuzzer +(cherry picked from commit c54dad65317f851ce9d016bd90ec6a7a04da09fc) +(cherry picked from https://googleplex-android-review.googlesource.com/q/commit:f946e3c5e34539ba1fe9f03025005e5b248ebbc6) +Merged-In: Ibc5566ade0389221690dc90324f93394cf7fc9a5 +Change-Id: Ibc5566ade0389221690dc90324f93394cf7fc9a5 +--- + libs/binder/Parcel.cpp | 12 ++++++++++++ + 1 file changed, 12 insertions(+) + +diff --git a/libs/binder/Parcel.cpp b/libs/binder/Parcel.cpp +index 0264b1cfad..acf262aa4a 100644 +--- a/libs/binder/Parcel.cpp ++++ b/libs/binder/Parcel.cpp +@@ -888,6 +888,10 @@ restart_write: + //printf("Writing %ld bytes, padded to %ld\n", len, padded); + uint8_t* const data = mData+mDataPos; + ++ if (status_t status = validateReadData(mDataPos + padded); status != OK) { ++ return nullptr; // drops status ++ } ++ + // Need to pad at end? + if (padded != len) { + #if BYTE_ORDER == BIG_ENDIAN +@@ -1405,6 +1409,10 @@ status_t Parcel::writeObject(const flat_binder_object& val, bool nullMetaData) + const bool enoughObjects = mObjectsSize < mObjectsCapacity; + if (enoughData && enoughObjects) { + restart_write: ++ if (status_t status = validateReadData(mDataPos + sizeof(val)); status != OK) { ++ return status; ++ } ++ + *reinterpret_cast(mData+mDataPos) = val; + + // remember if it's a file descriptor +@@ -1621,6 +1629,10 @@ status_t Parcel::writeAligned(T val) { + + if ((mDataPos+sizeof(val)) <= mDataCapacity) { + restart_write: ++ if (status_t status = validateReadData(mDataPos + sizeof(val)); status != OK) { ++ return status; ++ } ++ + memcpy(mData + mDataPos, &val, sizeof(val)); + return finishWrite(sizeof(val)); + } +-- +2.46.1.824.gd892dcdcdd-goog + diff --git a/aosp_diff/preliminary/packages/apps/DocumentsUI/0001-Prevent-clickjacking-attack-in-DocsUi.patch b/aosp_diff/preliminary/packages/apps/DocumentsUI/0001-Prevent-clickjacking-attack-in-DocsUi.patch new file mode 100644 index 0000000000..c8c1f00bd1 --- /dev/null +++ b/aosp_diff/preliminary/packages/apps/DocumentsUI/0001-Prevent-clickjacking-attack-in-DocsUi.patch @@ -0,0 +1,84 @@ +From aeb3c958df9ca704dac3f5795694e723b049e23b Mon Sep 17 00:00:00 2001 +From: Aditya Singh +Date: Mon, 9 Sep 2024 15:31:55 +0000 +Subject: [PATCH] Prevent clickjacking attack in DocsUi. + +* Added permission `HIDE_OVERLAY_WINDOWS` in the Manifest. +* Set the flag to hide overlay windows to true in BaseActivity and + ConfirmFragment. + +Bug: 233605527 +Test: Manually, see http://b/233605527#comment4 +Flag: EXEMPT bugfix +(cherry picked from https://googleplex-android-review.googlesource.com/q/commit:439685c0fdaf8e30a743591fd64ef0cd25877ac8) +Merged-In: I511730856be58cad3e13fa50bfac1e1ee2f5fee0 +Change-Id: I511730856be58cad3e13fa50bfac1e1ee2f5fee0 +--- + AndroidManifest.xml | 1 + + src/com/android/documentsui/BaseActivity.java | 5 +++++ + src/com/android/documentsui/picker/ConfirmFragment.java | 7 ++++++- + 3 files changed, 12 insertions(+), 1 deletion(-) + +diff --git a/AndroidManifest.xml b/AndroidManifest.xml +index e15805e28..9dbbb70c6 100644 +--- a/AndroidManifest.xml ++++ b/AndroidManifest.xml +@@ -32,6 +32,7 @@ + + + ++ + + + +diff --git a/src/com/android/documentsui/BaseActivity.java b/src/com/android/documentsui/BaseActivity.java +index c6cbc1936..5af331439 100644 +--- a/src/com/android/documentsui/BaseActivity.java ++++ b/src/com/android/documentsui/BaseActivity.java +@@ -74,6 +74,7 @@ import com.android.documentsui.roots.ProvidersCache; + import com.android.documentsui.sidebar.RootsFragment; + import com.android.documentsui.sorting.SortController; + import com.android.documentsui.sorting.SortModel; ++import com.android.modules.utils.build.SdkLevel; + + import com.android.documentsui.util.VersionUtils; + import com.google.android.material.appbar.AppBarLayout; +@@ -135,6 +136,10 @@ public abstract class BaseActivity + // Record the time when onCreate is invoked for metric. + mStartTime = new Date().getTime(); + ++ if (SdkLevel.isAtLeastS()) { ++ getWindow().setHideOverlayWindows(true); ++ } ++ + // ToDo Create tool to check resource version before applyStyle for the theme + // If version code is not match, we should reset overlay package to default, + // in case Activity continueusly encounter resource not found exception +diff --git a/src/com/android/documentsui/picker/ConfirmFragment.java b/src/com/android/documentsui/picker/ConfirmFragment.java +index 94015e930..e1af281bc 100644 +--- a/src/com/android/documentsui/picker/ConfirmFragment.java ++++ b/src/com/android/documentsui/picker/ConfirmFragment.java +@@ -32,6 +32,7 @@ import com.android.documentsui.BaseActivity; + import com.android.documentsui.R; + import com.android.documentsui.base.DocumentInfo; + import com.android.documentsui.base.Shared; ++import com.android.modules.utils.build.SdkLevel; + + import com.google.android.material.dialog.MaterialAlertDialogBuilder; + +@@ -102,7 +103,11 @@ public class ConfirmFragment extends DialogFragment { + builder.setNegativeButton(android.R.string.cancel, + (DialogInterface dialog, int id) -> pickResult.increaseActionCount()); + +- return builder.create(); ++ Dialog dialog = builder.create(); ++ if (SdkLevel.isAtLeastS()) { ++ dialog.getWindow().setHideOverlayWindows(true); ++ } ++ return dialog; + } + + @Override +-- +2.34.1 + diff --git a/aosp_diff/preliminary/packages/apps/Settings/0035-CDM-NLS-Check-if-the-NLS-service-has-an-intent-filter.bulletin.patch b/aosp_diff/preliminary/packages/apps/Settings/0035-CDM-NLS-Check-if-the-NLS-service-has-an-intent-filter.bulletin.patch new file mode 100644 index 0000000000..a7a20ef91f --- /dev/null +++ b/aosp_diff/preliminary/packages/apps/Settings/0035-CDM-NLS-Check-if-the-NLS-service-has-an-intent-filter.bulletin.patch @@ -0,0 +1,114 @@ +From 7fd48b9dad92f63d1d4acdcb3c9dcfe353029207 Mon Sep 17 00:00:00 2001 +From: Guojing Yuan +Date: Tue, 1 Oct 2024 21:59:31 +0000 +Subject: [PATCH] [CDM][NLS] Check if the NLS service has an intent-filter + +Bug: 363248394 +Test: CTS +Flag: EXEMPT bugfix +(cherry picked from commit 7ae59a42eb13f643d842525208619037c074371a) +(cherry picked from https://googleplex-android-review.googlesource.com/q/commit:19914088c3f437c504988f64261e62a99c9d113b) +Merged-In: Ib79c219cde8d73a218ceb7911f4552d43e384d8e +Change-Id: Ib79c219cde8d73a218ceb7911f4552d43e384d8e +--- + ...otificationAccessConfirmationActivity.java | 50 +++++++++++-------- + 1 file changed, 30 insertions(+), 20 deletions(-) + +diff --git a/src/com/android/settings/notification/NotificationAccessConfirmationActivity.java b/src/com/android/settings/notification/NotificationAccessConfirmationActivity.java +index 9ea8c58024..74b8102ee2 100644 +--- a/src/com/android/settings/notification/NotificationAccessConfirmationActivity.java ++++ b/src/com/android/settings/notification/NotificationAccessConfirmationActivity.java +@@ -31,13 +31,15 @@ import android.app.admin.DevicePolicyManager; + import android.content.ComponentName; + import android.content.Context; + import android.content.DialogInterface; ++import android.content.Intent; + import android.content.pm.ApplicationInfo; + import android.content.pm.PackageItemInfo; + import android.content.pm.PackageManager; +-import android.content.pm.ServiceInfo; ++import android.content.pm.ResolveInfo; + import android.os.Bundle; + import android.os.UserHandle; + import android.os.UserManager; ++import android.service.notification.NotificationListenerService; + import android.text.TextUtils; + import android.util.Slog; + import android.view.WindowManager; +@@ -48,6 +50,8 @@ import com.android.internal.app.AlertActivity; + import com.android.internal.app.AlertController; + import com.android.settings.R; + ++import java.util.List; ++ + /** @hide */ + public class NotificationAccessConfirmationActivity extends Activity + implements DialogInterface { +@@ -112,6 +116,31 @@ public class NotificationAccessConfirmationActivity extends Activity + return; + } + ++ // Check NLS service info. ++ String requiredPermission = Manifest.permission.BIND_NOTIFICATION_LISTENER_SERVICE; ++ Intent NLSIntent = new Intent(NotificationListenerService.SERVICE_INTERFACE); ++ List matchedServiceList = getPackageManager().queryIntentServicesAsUser( ++ NLSIntent, /* flags */ 0, mUserId); ++ boolean hasNLSIntentFilter = false; ++ for (ResolveInfo service : matchedServiceList) { ++ if (service.serviceInfo.packageName.equals(mComponentName.getPackageName())) { ++ if (!requiredPermission.equals(service.serviceInfo.permission)) { ++ Slog.e(LOG_TAG, "Service " + mComponentName + " lacks permission " ++ + requiredPermission); ++ finish(); ++ return; ++ } ++ hasNLSIntentFilter = true; ++ break; ++ } ++ } ++ if (!hasNLSIntentFilter) { ++ Slog.e(LOG_TAG, "Service " + mComponentName + " lacks an intent-filter action " ++ + "for android.service.notification.NotificationListenerService."); ++ finish(); ++ return; ++ } ++ + AlertController.AlertParams p = new AlertController.AlertParams(this); + p.mTitle = getString( + R.string.notification_listener_security_warning_title, +@@ -146,19 +175,6 @@ public class NotificationAccessConfirmationActivity extends Activity + } + + private void onAllow() { +- String requiredPermission = Manifest.permission.BIND_NOTIFICATION_LISTENER_SERVICE; +- try { +- ServiceInfo serviceInfo = getPackageManager().getServiceInfo(mComponentName, 0); +- if (!requiredPermission.equals(serviceInfo.permission)) { +- Slog.e(LOG_TAG, +- "Service " + mComponentName + " lacks permission " + requiredPermission); +- return; +- } +- } catch (PackageManager.NameNotFoundException e) { +- Slog.e(LOG_TAG, "Failed to get service info for " + mComponentName, e); +- return; +- } +- + mNm.setNotificationListenerAccessGranted(mComponentName, true); + + finish(); +@@ -169,12 +185,6 @@ public class NotificationAccessConfirmationActivity extends Activity + return AlertActivity.dispatchPopulateAccessibilityEvent(this, event); + } + +- @Override +- public void onBackPressed() { +- // Suppress finishing the activity on back button press, +- // consistently with the permission dialog behavior +- } +- + @Override + public void cancel() { + finish(); +-- +2.46.1.824.gd892dcdcdd-goog + diff --git a/aosp_diff/preliminary/packages/apps/Settings/0036-Disable-factory-reset-in-DSU-mode.bulletin.patch b/aosp_diff/preliminary/packages/apps/Settings/0036-Disable-factory-reset-in-DSU-mode.bulletin.patch new file mode 100644 index 0000000000..06d1feb706 --- /dev/null +++ b/aosp_diff/preliminary/packages/apps/Settings/0036-Disable-factory-reset-in-DSU-mode.bulletin.patch @@ -0,0 +1,64 @@ +From 779372c34f0b2928107d6ebf9165f97b4a2edc4f Mon Sep 17 00:00:00 2001 +From: t +Date: Thu, 2 Nov 2023 08:06:59 +0000 +Subject: [PATCH] Disable factory reset in DSU mode + +Bug: 302317901 +Bug: 316578327 +Test: build +(cherry picked from https://googleplex-android-review.googlesource.com/q/commit:d079c39b8db394dece4726010f8f9aa2823b039a) +Merged-In: I485eb6ac7beec0893d91ca5fe8ad88ecd96a5cbe +Change-Id: I485eb6ac7beec0893d91ca5fe8ad88ecd96a5cbe +--- + src/com/android/settings/MainClear.java | 16 ++++++++++++++++ + 1 file changed, 16 insertions(+) + +diff --git a/src/com/android/settings/MainClear.java b/src/com/android/settings/MainClear.java +index 2b0f01036e..9fe11a4850 100644 +--- a/src/com/android/settings/MainClear.java ++++ b/src/com/android/settings/MainClear.java +@@ -26,11 +26,13 @@ import android.accounts.AccountManager; + import android.accounts.AuthenticatorDescription; + import android.app.ActionBar; + import android.app.Activity; ++import android.app.AlertDialog; + import android.app.admin.DevicePolicyManager; + import android.app.settings.SettingsEnums; + import android.content.ComponentName; + import android.content.ContentResolver; + import android.content.Context; ++import android.content.DialogInterface; + import android.content.Intent; + import android.content.pm.PackageManager; + import android.content.pm.ResolveInfo; +@@ -43,6 +45,7 @@ import android.os.Environment; + import android.os.SystemProperties; + import android.os.UserHandle; + import android.os.UserManager; ++import android.os.image.DynamicSystemManager; + import android.provider.Settings; + import android.sysprop.VoldProperties; + import android.telephony.euicc.EuiccManager; +@@ -266,6 +269,19 @@ public class MainClear extends InstrumentedFragment implements OnGlobalLayoutLis + return; + } + ++ final DynamicSystemManager dsuManager = (DynamicSystemManager) ++ getActivity().getSystemService(Context.DYNAMIC_SYSTEM_SERVICE); ++ if (dsuManager.isInUse()) { ++ AlertDialog.Builder builder = new AlertDialog.Builder(getActivity()); ++ builder.setTitle(R.string.dsu_is_running); ++ builder.setPositiveButton(R.string.okay, new DialogInterface.OnClickListener() { ++ public void onClick(DialogInterface dialog, int id) {} ++ }); ++ AlertDialog dsuAlertdialog = builder.create(); ++ dsuAlertdialog.show(); ++ return; ++ } ++ + if (runKeyguardConfirmation(KEYGUARD_REQUEST)) { + return; + } +-- +2.46.1.824.gd892dcdcdd-goog + diff --git a/aosp_diff/preliminary/packages/modules/Bluetooth/64_0064-RESTRICT-AUTOMERGE-Disallow-unexpected-incoming-HID-.patch b/aosp_diff/preliminary/packages/modules/Bluetooth/64_0064-RESTRICT-AUTOMERGE-Disallow-unexpected-incoming-HID-.patch new file mode 100644 index 0000000000..8a7a676eb2 --- /dev/null +++ b/aosp_diff/preliminary/packages/modules/Bluetooth/64_0064-RESTRICT-AUTOMERGE-Disallow-unexpected-incoming-HID-.patch @@ -0,0 +1,453 @@ +From 66b07f3f69f0020d06fe97188059e3fe600af12e Mon Sep 17 00:00:00 2001 +From: Himanshu Rawat +Date: Mon, 8 Apr 2024 17:46:18 +0000 +Subject: [PATCH] RESTRICT AUTOMERGE Disallow unexpected incoming HID + connections + +HID profile accepted any new incoming HID connection. Even when the +connection policy disabled HID connection, remote devices could initiate +HID connection. +This change ensures that incoming HID connection are accepted only if +application was interested in that HID connection. +This vulnerarbility no longer exists on the main because of feature +request b/324093729. + +Test: mmm packages/modules/Bluetooth +Test: Manual | Pair and connect a HID device, disable HID connection +from Bluetooth device setting, attempt to connect from the HID device. +Bug: 308429049 +Ignore-AOSP-First: security +(cherry picked from https://googleplex-android-review.googlesource.com/q/commit:03dca3305311096f157da3ab9cfed5cc30f2c135) +(cherry picked from https://googleplex-android-review.googlesource.com/q/commit:431ef0346302dec8fa8c7d89c4696931e2bbac9a) +Merged-In: I013d0528fb18ee87195fb3c8aab553c6a8da5ae4 +Change-Id: I013d0528fb18ee87195fb3c8aab553c6a8da5ae4 +--- + .../jni/com_android_bluetooth_hid_host.cpp | 8 +- + .../android/bluetooth/hid/HidHostService.java | 7 +- + system/btif/include/btif_hh.h | 4 +- + system/btif/include/btif_storage.h | 23 +++++ + system/btif/src/btif_hh.cc | 86 +++++++++++++++++-- + system/btif/src/btif_storage.cc | 51 ++++++++++- + system/gd/rust/linux/stack/src/bluetooth.rs | 2 +- + .../gd/rust/topshim/src/profiles/hid_host.rs | 2 +- + system/include/hardware/bt_hh.h | 2 +- + 9 files changed, 170 insertions(+), 15 deletions(-) + +diff --git a/android/app/jni/com_android_bluetooth_hid_host.cpp b/android/app/jni/com_android_bluetooth_hid_host.cpp +index 7a164233bc..18ba315129 100644 +--- a/android/app/jni/com_android_bluetooth_hid_host.cpp ++++ b/android/app/jni/com_android_bluetooth_hid_host.cpp +@@ -282,7 +282,8 @@ static jboolean connectHidNative(JNIEnv* env, jobject object, + } + + static jboolean disconnectHidNative(JNIEnv* env, jobject object, +- jbyteArray address) { ++ jbyteArray address, ++ jboolean reconnect_allowed) { + jbyte* addr; + jboolean ret = JNI_TRUE; + if (!sBluetoothHidInterface) return JNI_FALSE; +@@ -293,7 +294,8 @@ static jboolean disconnectHidNative(JNIEnv* env, jobject object, + return JNI_FALSE; + } + +- bt_status_t status = sBluetoothHidInterface->disconnect((RawAddress*)addr); ++ bt_status_t status = ++ sBluetoothHidInterface->disconnect((RawAddress*)addr, reconnect_allowed); + if (status != BT_STATUS_SUCCESS) { + ALOGE("Failed disconnect hid channel, status: %d", status); + ret = JNI_FALSE; +@@ -509,7 +511,7 @@ static JNINativeMethod sMethods[] = { + {"initializeNative", "()V", (void*)initializeNative}, + {"cleanupNative", "()V", (void*)cleanupNative}, + {"connectHidNative", "([B)Z", (void*)connectHidNative}, +- {"disconnectHidNative", "([B)Z", (void*)disconnectHidNative}, ++ {"disconnectHidNative", "([BZ)Z", (void*)disconnectHidNative}, + {"getProtocolModeNative", "([B)Z", (void*)getProtocolModeNative}, + {"virtualUnPlugNative", "([B)Z", (void*)virtualUnPlugNative}, + {"setProtocolModeNative", "([BB)Z", (void*)setProtocolModeNative}, +diff --git a/android/app/src/com/android/bluetooth/hid/HidHostService.java b/android/app/src/com/android/bluetooth/hid/HidHostService.java +index 7352d5833b..ffd6b0a70b 100644 +--- a/android/app/src/com/android/bluetooth/hid/HidHostService.java ++++ b/android/app/src/com/android/bluetooth/hid/HidHostService.java +@@ -186,7 +186,10 @@ public class HidHostService extends ProfileService { + break; + case MESSAGE_DISCONNECT: { + BluetoothDevice device = (BluetoothDevice) msg.obj; +- if (!disconnectHidNative(getByteAddress(device))) { ++ int connectionPolicy = getConnectionPolicy(device); ++ boolean reconnectAllowed = ++ connectionPolicy == BluetoothProfile.CONNECTION_POLICY_ALLOWED; ++ if (!disconnectHidNative(getByteAddress(device), reconnectAllowed)) { + broadcastConnectionState(device, BluetoothProfile.STATE_DISCONNECTING); + broadcastConnectionState(device, BluetoothProfile.STATE_DISCONNECTED); + break; +@@ -1023,7 +1026,7 @@ public class HidHostService extends ProfileService { + + private native boolean connectHidNative(byte[] btAddress); + +- private native boolean disconnectHidNative(byte[] btAddress); ++ private native boolean disconnectHidNative(byte[] btAddress, boolean reconnectAllowed); + + private native boolean getProtocolModeNative(byte[] btAddress); + +diff --git a/system/btif/include/btif_hh.h b/system/btif/include/btif_hh.h +index b2e125053b..6e5fc80679 100644 +--- a/system/btif/include/btif_hh.h ++++ b/system/btif/include/btif_hh.h +@@ -106,6 +106,7 @@ typedef struct { + uint8_t dev_handle; + RawAddress bd_addr; + tBTA_HH_ATTR_MASK attr_mask; ++ bool reconnect_allowed; + } btif_hh_added_device_t; + + /** +@@ -130,7 +131,8 @@ extern btif_hh_cb_t btif_hh_cb; + extern btif_hh_device_t* btif_hh_find_connected_dev_by_handle(uint8_t handle); + extern void btif_hh_remove_device(RawAddress bd_addr); + extern bool btif_hh_add_added_dev(const RawAddress& bda, +- tBTA_HH_ATTR_MASK attr_mask); ++ tBTA_HH_ATTR_MASK attr_mask, ++ bool reconnect_allowed); + extern bt_status_t btif_hh_virtual_unplug(const RawAddress* bd_addr); + extern void btif_hh_disconnect(RawAddress* bd_addr); + extern void btif_hh_setreport(btif_hh_device_t* p_dev, +diff --git a/system/btif/include/btif_storage.h b/system/btif/include/btif_storage.h +index 5ffb9daf4a..fd76d581f5 100644 +--- a/system/btif/include/btif_storage.h ++++ b/system/btif/include/btif_storage.h +@@ -198,6 +198,29 @@ void btif_storage_load_le_devices(void); + ******************************************************************************/ + bt_status_t btif_storage_load_bonded_devices(void); + ++/******************************************************************************* ++ * ++ * Function btif_storage_set_hid_connection_policy ++ * ++ * Description Stores connection policy info in nvram ++ * ++ * Returns BT_STATUS_SUCCESS ++ * ++ ******************************************************************************/ ++bt_status_t btif_storage_set_hid_connection_policy(const RawAddress& addr, ++ bool reconnect_allowed); ++/******************************************************************************* ++ * ++ * Function btif_storage_get_hid_connection_policy ++ * ++ * Description get connection policy info from nvram ++ * ++ * Returns BT_STATUS_SUCCESS ++ * ++ ******************************************************************************/ ++bt_status_t btif_storage_get_hid_connection_policy(const RawAddress& addr, ++ bool* reconnect_allowed); ++ + /******************************************************************************* + * + * Function btif_storage_add_hid_device_info +diff --git a/system/btif/src/btif_hh.cc b/system/btif/src/btif_hh.cc +index 3280a957d2..f7e37692df 100644 +--- a/system/btif/src/btif_hh.cc ++++ b/system/btif/src/btif_hh.cc +@@ -308,6 +308,24 @@ btif_hh_device_t* btif_hh_find_connected_dev_by_handle(uint8_t handle) { + return &btif_hh_cb.devices[i]; + } + } ++ return nullptr; ++} ++ ++/******************************************************************************* ++ * ++ * Function btif_hh_find_added_dev ++ * ++ * Description Return the added device pointer of the specified address ++ * ++ * Returns Added device entry ++ ******************************************************************************/ ++btif_hh_added_device_t* btif_hh_find_added_dev(const RawAddress& addr) { ++ for (int i = 0; i < BTIF_HH_MAX_ADDED_DEV; i++) { ++ btif_hh_added_device_t* added_dev = &btif_hh_cb.added_devices[i]; ++ if (added_dev->bd_addr == addr) { ++ return added_dev; ++ } ++ } + return NULL; + } + +@@ -351,6 +369,23 @@ static btif_hh_device_t* btif_hh_find_connected_dev_by_bda( + return NULL; + } + ++static bool btif_hh_connection_allowed(const RawAddress& bda) { ++ /* Accept connection only if reconnection is allowed for the known device, or ++ * outgoing connection was requested */ ++ btif_hh_added_device_t* added_dev = btif_hh_find_added_dev(bda); ++ if (added_dev != nullptr && added_dev->reconnect_allowed) { ++ LOG_VERBOSE("Connection allowed %s", PRIVATE_ADDRESS(bda)); ++ return true; ++ } else if (btif_hh_cb.pending_conn_address == bda) { ++ LOG_VERBOSE("Device connection was pending for: %s, status: %s", ++ PRIVATE_ADDRESS(bda), ++ btif_hh_status_text(btif_hh_cb.status).c_str()); ++ return true; ++ } ++ ++ return false; ++} ++ + /******************************************************************************* + * + * Function btif_hh_stop_vup_timer +@@ -396,7 +431,8 @@ void btif_hh_start_vup_timer(const RawAddress* bd_addr) { + * + * Returns true if add successfully, otherwise false. + ******************************************************************************/ +-bool btif_hh_add_added_dev(const RawAddress& bda, tBTA_HH_ATTR_MASK attr_mask) { ++bool btif_hh_add_added_dev(const RawAddress& bda, tBTA_HH_ATTR_MASK attr_mask, ++ bool reconnect_allowed) { + int i; + for (i = 0; i < BTIF_HH_MAX_ADDED_DEV; i++) { + if (btif_hh_cb.added_devices[i].bd_addr == bda) { +@@ -410,6 +446,7 @@ bool btif_hh_add_added_dev(const RawAddress& bda, tBTA_HH_ATTR_MASK attr_mask) { + btif_hh_cb.added_devices[i].bd_addr = bda; + btif_hh_cb.added_devices[i].dev_handle = BTA_HH_INVALID_HANDLE; + btif_hh_cb.added_devices[i].attr_mask = attr_mask; ++ btif_hh_cb.added_devices[i].reconnect_allowed = reconnect_allowed; + return true; + } + } +@@ -775,9 +812,26 @@ static void btif_hh_upstreams_evt(uint16_t event, char* p_param) { + p_data->status); + break; + +- case BTA_HH_OPEN_EVT: ++ case BTA_HH_OPEN_EVT: { + BTIF_TRACE_WARNING("%s: BTA_HH_OPN_EVT: handle=%d, status =%d", __func__, + p_data->conn.handle, p_data->conn.status); ++ ++ if (!btif_hh_connection_allowed(p_data->conn.bda)) { ++ LOG_WARN("Reject incoming HID Connection, device: %s", ++ PRIVATE_ADDRESS(p_data->conn.bda)); ++ btif_hh_device_t* p_dev = ++ btif_hh_find_connected_dev_by_handle(p_data->conn.handle); ++ if (p_dev != nullptr) { ++ p_dev->dev_status = BTHH_CONN_STATE_DISCONNECTED; ++ } ++ ++ btif_hh_cb.status = (BTIF_HH_STATUS)BTIF_HH_DEV_DISCONNECTED; ++ BTA_HhClose(p_data->conn.handle); ++ HAL_CBACK(bt_hh_callbacks, connection_state_cb, &p_data->conn.bda, ++ BTHH_CONN_STATE_DISCONNECTED); ++ return; ++ } ++ + btif_hh_cb.pending_conn_address = RawAddress::kEmpty; + if (p_data->conn.status == BTA_HH_OK) { + p_dev = btif_hh_find_connected_dev_by_handle(p_data->conn.handle); +@@ -836,6 +890,7 @@ static void btif_hh_upstreams_evt(uint16_t event, char* p_param) { + btif_hh_cb.status = (BTIF_HH_STATUS)BTIF_HH_DEV_DISCONNECTED; + } + break; ++ } + + case BTA_HH_CLOSE_EVT: + BTIF_TRACE_DEBUG("BTA_HH_CLOSE_EVT: status = %d, handle = %d", +@@ -1009,7 +1064,7 @@ static void btif_hh_upstreams_evt(uint16_t event, char* p_param) { + p_data->dscp_info.version, + p_data->dscp_info.ctry_code, len, + p_data->dscp_info.descriptor.dsc_list); +- if (btif_hh_add_added_dev(p_dev->bd_addr, p_dev->attr_mask)) { ++ if (btif_hh_add_added_dev(p_dev->bd_addr, p_dev->attr_mask, true)) { + tBTA_HH_DEV_DSCP_INFO dscp_info; + bt_status_t ret; + btif_hh_copy_hid_info(&dscp_info, &p_data->dscp_info); +@@ -1025,6 +1080,8 @@ static void btif_hh_upstreams_evt(uint16_t event, char* p_param) { + p_data->dscp_info.ssr_min_tout, len, + p_data->dscp_info.descriptor.dsc_list); + ++ btif_storage_set_hid_connection_policy(p_dev->bd_addr, true); ++ + ASSERTC(ret == BT_STATUS_SUCCESS, "storing hid info failed", ret); + BTIF_TRACE_WARNING("BTA_HH_GET_DSCP_EVT: Called add device"); + +@@ -1312,6 +1369,13 @@ static bt_status_t init(bthh_callbacks_t* callbacks) { + ******************************************************************************/ + static bt_status_t connect(RawAddress* bd_addr) { + if (btif_hh_cb.status != BTIF_HH_DEV_CONNECTING) { ++ /* If the device was already added, ensure that reconnections are allowed */ ++ btif_hh_added_device_t* added_dev = btif_hh_find_added_dev(*bd_addr); ++ if (added_dev != nullptr && !added_dev->reconnect_allowed) { ++ added_dev->reconnect_allowed = true; ++ btif_storage_set_hid_connection_policy(*bd_addr, true); ++ } ++ + btif_transfer_context(btif_hh_handle_evt, BTIF_HH_CONNECT_REQ_EVT, + (char*)bd_addr, sizeof(RawAddress), NULL); + return BT_STATUS_SUCCESS; +@@ -1332,7 +1396,7 @@ static bt_status_t connect(RawAddress* bd_addr) { + * Returns bt_status_t + * + ******************************************************************************/ +-static bt_status_t disconnect(RawAddress* bd_addr) { ++static bt_status_t disconnect(RawAddress* bd_addr, bool reconnect_allowed) { + CHECK_BTHH_INIT(); + BTIF_TRACE_EVENT("BTHH: %s", __func__); + btif_hh_device_t* p_dev; +@@ -1342,6 +1406,17 @@ static bt_status_t disconnect(RawAddress* bd_addr) { + btif_hh_cb.status); + return BT_STATUS_FAIL; + } ++ ++ if (!reconnect_allowed) { ++ LOG_INFO("Incoming reconnections disabled for device %s", ++ PRIVATE_ADDRESS((*bd_addr))); ++ btif_hh_added_device_t* added_dev = btif_hh_find_added_dev(*bd_addr); ++ if (added_dev != nullptr && added_dev->reconnect_allowed) { ++ added_dev->reconnect_allowed = false; ++ btif_storage_set_hid_connection_policy(added_dev->bd_addr, false); ++ } ++ } ++ + p_dev = btif_hh_find_connected_dev_by_bda(*bd_addr); + if (p_dev != NULL) { + return btif_transfer_context(btif_hh_handle_evt, BTIF_HH_DISCONNECT_REQ_EVT, +@@ -1473,9 +1548,10 @@ static bt_status_t set_info(RawAddress* bd_addr, bthh_hid_info_t hid_info) { + (uint8_t*)osi_malloc(dscp_info.descriptor.dl_len); + memcpy(dscp_info.descriptor.dsc_list, &(hid_info.dsc_list), hid_info.dl_len); + +- if (btif_hh_add_added_dev(*bd_addr, hid_info.attr_mask)) { ++ if (btif_hh_add_added_dev(*bd_addr, hid_info.attr_mask, true)) { + BTA_HhAddDev(*bd_addr, hid_info.attr_mask, hid_info.sub_class, + hid_info.app_id, dscp_info); ++ btif_storage_set_hid_connection_policy(*bd_addr, true); + } + + osi_free_and_reset((void**)&dscp_info.descriptor.dsc_list); +diff --git a/system/btif/src/btif_storage.cc b/system/btif/src/btif_storage.cc +index 7be5f37e66..bcc644bf10 100644 +--- a/system/btif/src/btif_storage.cc ++++ b/system/btif/src/btif_storage.cc +@@ -111,6 +111,8 @@ using bluetooth::groups::DeviceGroups; + #define BTIF_STORAGE_LEAUDIO_SOURCE_SUPPORTED_CONTEXT_TYPE \ + "SourceSupportedContextType" + ++#define BTIF_STORAGE_KEY_HID_RECONNECT_ALLOWED "HidReConnectAllowed" ++ + /* This is a local property to add a device found */ + #define BT_PROPERTY_REMOTE_DEVICE_TIMESTAMP 0xFF + +@@ -1482,6 +1484,49 @@ bool btif_storage_get_remote_device_type(const RawAddress& remote_bd_addr, + return ret; + } + ++/******************************************************************************* ++ * ++ * Function btif_storage_set_hid_connection_policy ++ * ++ * Description Stores connection policy info in nvram ++ * ++ * Returns BT_STATUS_SUCCESS ++ * ++ ******************************************************************************/ ++bt_status_t btif_storage_set_hid_connection_policy(const RawAddress& addr, ++ bool reconnect_allowed) { ++ std::string bdstr = addr.ToString(); ++ ++ if (btif_config_set_int(bdstr, BTIF_STORAGE_KEY_HID_RECONNECT_ALLOWED, ++ reconnect_allowed)) { ++ return BT_STATUS_SUCCESS; ++ } else { ++ return BT_STATUS_FAIL; ++ } ++} ++ ++/******************************************************************************* ++ * ++ * Function btif_storage_get_hid_connection_policy ++ * ++ * Description get connection policy info from nvram ++ * ++ * Returns BT_STATUS_SUCCESS ++ * ++ ******************************************************************************/ ++bt_status_t btif_storage_get_hid_connection_policy(const RawAddress& addr, ++ bool* reconnect_allowed) { ++ std::string bdstr = addr.ToString(); ++ ++ // For backward compatibility, assume that the reconnection is allowed in the ++ // absence of the key ++ int value = 1; ++ btif_config_get_int(bdstr, BTIF_STORAGE_KEY_HID_RECONNECT_ALLOWED, &value); ++ *reconnect_allowed = (value != 0); ++ ++ return BT_STATUS_SUCCESS; ++} ++ + /******************************************************************************* + * + * Function btif_storage_add_hid_device_info +@@ -1577,8 +1622,11 @@ bt_status_t btif_storage_load_bonded_hid_info(void) { + (uint8_t*)dscp_info.descriptor.dsc_list, &len); + } + ++ bool reconnect_allowed = false; ++ btif_storage_get_hid_connection_policy(bd_addr, &reconnect_allowed); ++ + // add extracted information to BTA HH +- if (btif_hh_add_added_dev(bd_addr, attr_mask)) { ++ if (btif_hh_add_added_dev(bd_addr, attr_mask, reconnect_allowed)) { + BTA_HhAddDev(bd_addr, attr_mask, sub_class, app_id, dscp_info); + } + } +@@ -1610,6 +1658,7 @@ bt_status_t btif_storage_remove_hid_info(const RawAddress& remote_bd_addr) { + btif_config_remove(bdstr, "HidSSRMaxLatency"); + btif_config_remove(bdstr, "HidSSRMinTimeout"); + btif_config_remove(bdstr, "HidDescriptor"); ++ btif_config_remove(bdstr, BTIF_STORAGE_KEY_HID_RECONNECT_ALLOWED); + btif_config_save(); + return BT_STATUS_SUCCESS; + } +diff --git a/system/gd/rust/linux/stack/src/bluetooth.rs b/system/gd/rust/linux/stack/src/bluetooth.rs +index 28216ff39a..a58e68e7d8 100644 +--- a/system/gd/rust/linux/stack/src/bluetooth.rs ++++ b/system/gd/rust/linux/stack/src/bluetooth.rs +@@ -1412,7 +1412,7 @@ impl IBluetooth for Bluetooth { + if self.uuid_helper.is_profile_enabled(&p) { + match p { + Profile::Hid | Profile::Hogp => { +- self.hh.as_ref().unwrap().disconnect(&mut addr.unwrap()); ++ self.hh.as_ref().unwrap().disconnect(&mut addr.unwrap(), true); + } + + Profile::A2dpSink | Profile::A2dpSource => { +diff --git a/system/gd/rust/topshim/src/profiles/hid_host.rs b/system/gd/rust/topshim/src/profiles/hid_host.rs +index db447be9d2..15f1f27a1b 100644 +--- a/system/gd/rust/topshim/src/profiles/hid_host.rs ++++ b/system/gd/rust/topshim/src/profiles/hid_host.rs +@@ -208,7 +208,7 @@ impl HidHost { + + pub fn disconnect(&self, addr: &mut RawAddress) -> BtStatus { + let ffi_addr = cast_to_ffi_address!(addr as *mut RawAddress); +- BtStatus::from(ccall!(self, disconnect, ffi_addr)) ++ BtStatus::from(ccall!(self, disconnect, ffi_addr, true)) + } + + pub fn virtual_unplug(&self, addr: &mut RawAddress) -> BtStatus { +diff --git a/system/include/hardware/bt_hh.h b/system/include/hardware/bt_hh.h +index dfe47f778a..c64e465e37 100644 +--- a/system/include/hardware/bt_hh.h ++++ b/system/include/hardware/bt_hh.h +@@ -173,7 +173,7 @@ typedef struct { + bt_status_t (*connect)(RawAddress* bd_addr); + + /** dis-connect from hid device */ +- bt_status_t (*disconnect)(RawAddress* bd_addr); ++ bt_status_t (*disconnect)(RawAddress* bd_addr, bool reconnect_allowed); + + /** Virtual UnPlug (VUP) the specified HID device */ + bt_status_t (*virtual_unplug)(RawAddress* bd_addr); +-- +2.34.1 + diff --git a/aosp_diff/preliminary/packages/modules/Bluetooth/65_0065-Resolve-incomplete-fix-for-SMP-authentication-bypass.bulletin.patch b/aosp_diff/preliminary/packages/modules/Bluetooth/65_0065-Resolve-incomplete-fix-for-SMP-authentication-bypass.bulletin.patch new file mode 100644 index 0000000000..9d5fdb024e --- /dev/null +++ b/aosp_diff/preliminary/packages/modules/Bluetooth/65_0065-Resolve-incomplete-fix-for-SMP-authentication-bypass.bulletin.patch @@ -0,0 +1,49 @@ +From 2f0cf867e27af9048821d332ecbd21ddf234888c Mon Sep 17 00:00:00 2001 +From: Brian Delwiche +Date: Mon, 14 Oct 2024 22:50:55 +0000 +Subject: [PATCH] Resolve incomplete fix for SMP authentication bypass + +Fix for b/251514170 was landed correctly on main, but in older branches +SMP contains identical functions smp_proc_init and smp_proc_rand, both +of which exhibit the problem, and only the former of which was patched. +This allows the problem to still appear on branches from sc-dev to +udc-dev. + +Add the logic to smp_proc_rand. + +Bug: 251514170 +Test: m com.android.btservices +Tag: #security +Ignore-AOSP-First: security +(cherry picked from https://googleplex-android-review.googlesource.com/q/commit:9b6737a08f5718b6400ffe78b494cb5f0779e56e) +Merged-In: I51e99c18a322a29632a6cac09ddb2b07bea482fc +Change-Id: I51e99c18a322a29632a6cac09ddb2b07bea482fc +--- + system/stack/smp/smp_act.cc | 11 +++++++++++ + 1 file changed, 11 insertions(+) + +diff --git a/system/stack/smp/smp_act.cc b/system/stack/smp/smp_act.cc +index bbbf3dc29b..5dbef5854c 100644 +--- a/system/stack/smp/smp_act.cc ++++ b/system/stack/smp/smp_act.cc +@@ -686,6 +686,17 @@ void smp_proc_rand(tSMP_CB* p_cb, tSMP_INT_DATA* p_data) { + return; + } + ++ if (!((p_cb->loc_auth_req & SMP_SC_SUPPORT_BIT) && ++ (p_cb->peer_auth_req & SMP_SC_SUPPORT_BIT)) && ++ !(p_cb->flags & SMP_PAIR_FLAGS_CMD_CONFIRM_SENT)) { ++ // in legacy pairing, the peer should send its rand after ++ // we send our confirm ++ tSMP_INT_DATA smp_int_data{}; ++ smp_int_data.status = SMP_INVALID_PARAMETERS; ++ smp_sm_event(p_cb, SMP_AUTH_CMPL_EVT, &smp_int_data); ++ return; ++ } ++ + /* save the SRand for comparison */ + STREAM_TO_ARRAY(p_cb->rrand.data(), p, OCTET16_LEN); + } +-- +2.46.1.824.gd892dcdcdd-goog + diff --git a/aosp_diff/preliminary/packages/modules/Permission/0004-Fix-Dynamic-Permission-group-auto-grant-behaivor.bulletin.patch b/aosp_diff/preliminary/packages/modules/Permission/0004-Fix-Dynamic-Permission-group-auto-grant-behaivor.bulletin.patch new file mode 100644 index 0000000000..125e79bc46 --- /dev/null +++ b/aosp_diff/preliminary/packages/modules/Permission/0004-Fix-Dynamic-Permission-group-auto-grant-behaivor.bulletin.patch @@ -0,0 +1,696 @@ +From 5ba5d56c45b95973f2d36ea18429129d7a816af6 Mon Sep 17 00:00:00 2001 +From: Yi-an Chen +Date: Thu, 8 Aug 2024 01:15:57 +0000 +Subject: [PATCH] Fix Dynamic Permission group auto grant behaivor + +Fix the Dynamic Permission group auto grant behaivor so that a +permission group is only considered granted when (1) all permissions +were auto-granted or (2) a platform permission in the same group is +granted. + +Bug: 340480881 +Test: DynamicPermissionsTest +(cherry picked from https://googleplex-android-review.googlesource.com/q/commit:cbe1a7f85d3d11825381c7f52b6ed144e575b861) +Merged-In: I37b550f0c3933bc790c2917a14e917efbcccc4e8 +Change-Id: I37b550f0c3933bc790c2917a14e917efbcccc4e8 +--- + .../permission/data/LightPermInfoLiveData.kt | 2 +- + .../permission/data/PermGroupLiveData.kt | 40 +++++++++++-------- + .../model/livedatatypes/LightAppPermGroup.kt | 37 ++++++++++++----- + .../model/livedatatypes/LightPackageInfo.kt | 4 +- + .../model/livedatatypes/LightPermInfo.kt | 11 +++-- + .../model/livedatatypes/LightPermission.kt | 17 +++++--- + .../service/AutoRevokePermissions.kt | 2 +- + .../RuntimePermissionsUpgradeController.kt | 4 +- + .../handheld/ReviewPermissionsFragment.java | 4 +- + .../ui/model/AppPermissionViewModel.kt | 10 ++--- + .../ui/model/GrantPermissionsViewModel.kt | 36 +++++++++++------ + .../ui/model/ReviewPermissionsViewModel.kt | 2 +- + .../permission/utils/KotlinUtils.kt | 24 +++++------ + .../permission/utils/SafetyNetLogger.java | 2 +- + .../model/ReviewPermissionsViewModelTest.kt | 2 +- + .../permission/utils/GrantRevokeTests.kt | 6 ++- + 16 files changed, 125 insertions(+), 78 deletions(-) + +diff --git a/PermissionController/src/com/android/permissioncontroller/permission/data/LightPermInfoLiveData.kt b/PermissionController/src/com/android/permissioncontroller/permission/data/LightPermInfoLiveData.kt +index 97389b098..bf43f15c3 100644 +--- a/PermissionController/src/com/android/permissioncontroller/permission/data/LightPermInfoLiveData.kt ++++ b/PermissionController/src/com/android/permissioncontroller/permission/data/LightPermInfoLiveData.kt +@@ -66,7 +66,7 @@ class LightPermInfoLiveData private constructor( + } + + val newValue = try { +- LightPermInfo(app.packageManager.getPermissionInfo(permissionName, 0)) ++ LightPermInfo(app.packageManager.getPermissionInfo(permissionName, 0), null) + } catch (e: PackageManager.NameNotFoundException) { + Log.w(LOG_TAG, "Permission \"$permissionName\" not found") + invalidateSingle(permissionName) +diff --git a/PermissionController/src/com/android/permissioncontroller/permission/data/PermGroupLiveData.kt b/PermissionController/src/com/android/permissioncontroller/permission/data/PermGroupLiveData.kt +index 78f2f72c6..948815646 100644 +--- a/PermissionController/src/com/android/permissioncontroller/permission/data/PermGroupLiveData.kt ++++ b/PermissionController/src/com/android/permissioncontroller/permission/data/PermGroupLiveData.kt +@@ -17,6 +17,7 @@ + package com.android.permissioncontroller.permission.data + + import android.app.Application ++import android.content.pm.ApplicationInfo + import android.content.pm.PackageItemInfo + import android.content.pm.PackageManager + import android.content.pm.PermissionGroupInfo +@@ -68,32 +69,31 @@ class PermGroupLiveData private constructor( + */ + override fun onUpdate() { + val permissionInfos = mutableMapOf() +- + groupInfo = Utils.getGroupInfo(groupName, context) ?: run { + Log.e(LOG_TAG, "Invalid permission group $groupName") + invalidateSingle(groupName) + value = null + return + } +- ++ val permInfos = mutableListOf() + when (groupInfo) { + is PermissionGroupInfo -> { +- val permInfos = try { +- Utils.getInstalledRuntimePermissionInfosForGroup(context.packageManager, +- groupName) ++ try { ++ permInfos.addAll( ++ Utils.getInstalledRuntimePermissionInfosForGroup( ++ context.packageManager, ++ groupName ++ ) ++ ) + } catch (e: PackageManager.NameNotFoundException) { + Log.e(LOG_TAG, "Invalid permission group $groupName") + invalidateSingle(groupName) + value = null + return + } +- +- for (permInfo in permInfos) { +- permissionInfos[permInfo.name] = LightPermInfo(permInfo) +- } + } + is PermissionInfo -> { +- permissionInfos[groupInfo.name] = LightPermInfo(groupInfo as PermissionInfo) ++ permInfos.add(groupInfo as PermissionInfo) + } + else -> { + value = null +@@ -101,19 +101,25 @@ class PermGroupLiveData private constructor( + } + } + +- val permGroup = PermGroup(LightPermGroupInfo(groupInfo), permissionInfos) +- +- value = permGroup +- +- val packageNames = permissionInfos.values.map { permInfo -> permInfo.packageName } +- .toMutableSet() ++ val packageNames = permInfos.map { permInfo -> permInfo.packageName }.toMutableSet() + packageNames.add(groupInfo.packageName) +- + // TODO ntmyren: What if the package isn't installed for the system user? + val getLiveData = { packageName: String -> + LightPackageInfoLiveData[packageName, UserHandle.SYSTEM] + } + setSourcesToDifference(packageNames, packageLiveDatas, getLiveData) ++ if (!packageLiveDatas.all { it.value.isInitialized }) { ++ return ++ } ++ for (permInfo in permInfos) { ++ val lightPackageInfo = packageLiveDatas[permInfo.packageName]?.value ++ val isSystem = ++ lightPackageInfo?.let { it.appFlags and ApplicationInfo.FLAG_SYSTEM != 0 } ++ permissionInfos[permInfo.name] = LightPermInfo(permInfo, isSystem) ++ } ++ ++ val permGroup = PermGroup(LightPermGroupInfo(groupInfo), permissionInfos) ++ value = permGroup + } + + override fun onInactive() { +diff --git a/PermissionController/src/com/android/permissioncontroller/permission/model/livedatatypes/LightAppPermGroup.kt b/PermissionController/src/com/android/permissioncontroller/permission/model/livedatatypes/LightAppPermGroup.kt +index 64d63bd1a..2e7d3b4a0 100644 +--- a/PermissionController/src/com/android/permissioncontroller/permission/model/livedatatypes/LightAppPermGroup.kt ++++ b/PermissionController/src/com/android/permissioncontroller/permission/model/livedatatypes/LightAppPermGroup.kt +@@ -20,6 +20,7 @@ import android.Manifest + import android.Manifest.permission.ACCESS_COARSE_LOCATION + import android.os.Build + import android.os.UserHandle ++import com.android.permissioncontroller.permission.utils.Utils + + /** + * A lightweight version of the AppPermissionGroup data structure. Represents information about a +@@ -79,11 +80,13 @@ data class LightAppPermGroup( + if (name !in backgroundPermNames) name else null + } + ++ val isPlatformPermissionGroup = permGroupInfo.packageName == Utils.OS_PKG ++ + val foreground = AppPermSubGroup(permissions.filter { it.key in foregroundPermNames }, +- packageInfo, specialLocationGrant) ++ packageInfo, isPlatformPermissionGroup, specialLocationGrant) + + val background = AppPermSubGroup(permissions.filter { it.key in backgroundPermNames }, +- packageInfo, specialLocationGrant) ++ packageInfo, isPlatformPermissionGroup, specialLocationGrant) + + /** + * Whether or not this App Permission Group has a permission which has a background mode +@@ -124,7 +127,7 @@ data class LightAppPermGroup( + */ + val isOneTime = (permGroupName != Manifest.permission_group.LOCATION && + permissions.any { it.value.isOneTime } && +- permissions.none { !it.value.isOneTime && it.value.isGrantedIncludingAppOp }) || ++ permissions.none { !it.value.isOneTime && it.value.isGranted }) || + (permGroupName == Manifest.permission_group.LOCATION && + permissions[ACCESS_COARSE_LOCATION]?.isOneTime == true) + +@@ -176,17 +179,23 @@ data class LightAppPermGroup( + * + * @param permissions The permissions contained within this subgroup, a subset of those contained + * in the full group ++ * @param isPlatformPermissionGroup Whether this is a platform permission group + * @param specialLocationGrant Whether this is a special location package + */ + data class AppPermSubGroup internal constructor( + private val permissions: Map, + private val packageInfo: LightPackageInfo, ++ private val isPlatformPermissionGroup: Boolean, + private val specialLocationGrant: Boolean? + ) { +- /** +- * Whether any of this App Permission SubGroup's permissions are granted +- */ +- val isGranted = specialLocationGrant ?: permissions.any { it.value.isGrantedIncludingAppOp } ++ /** Whether any of this App Permission SubGroup's permissions are granted */ ++ val isGranted = ++ specialLocationGrant ++ ?: permissions.any { ++ val mayGrantByPlatformOrSystem = ++ !isPlatformPermissionGroup || it.value.isPlatformOrSystem ++ it.value.isGranted && mayGrantByPlatformOrSystem ++ } + + /** + * Whether this App Permission SubGroup should be treated as granted. This means either: +@@ -195,9 +204,15 @@ data class LightAppPermGroup( + * 2) All permissions were auto-granted (all permissions are all granted and all + * RevokeWhenRequested.) + */ +- val isGrantedExcludingRWROrAllRWR = specialLocationGrant ?: (permissions +- .any { it.value.isGrantedIncludingAppOp && !it.value.isRevokeWhenRequested } || +- permissions.all { it.value.isGrantedIncludingAppOp && it.value.isRevokeWhenRequested }) ++ val allowFullGroupGrant = ++ specialLocationGrant ++ ?: (permissions.any { ++ val mayGrantByPlatformOrSystem = ++ !isPlatformPermissionGroup || it.value.isPlatformOrSystem ++ it.value.allowFullGroupGrant && mayGrantByPlatformOrSystem ++ } || permissions.all { ++ it.value.isGranted && it.value.isRevokeWhenRequested ++ }) + + /** + * Whether any of this App Permission SubGroup's permissions are granted by default +@@ -209,7 +224,7 @@ data class LightAppPermGroup( + * none of the granted permissions are not one-time. + */ + val isOneTime = permissions.any { it.value.isOneTime } && +- permissions.none { it.value.isGrantedIncludingAppOp && !it.value.isOneTime } ++ permissions.none { it.value.isGranted && !it.value.isOneTime } + + /** + * Whether any of this App Permission Subgroup's foreground permissions are fixed by policy +diff --git a/PermissionController/src/com/android/permissioncontroller/permission/model/livedatatypes/LightPackageInfo.kt b/PermissionController/src/com/android/permissioncontroller/permission/model/livedatatypes/LightPackageInfo.kt +index 182de1a59..a37a47163 100644 +--- a/PermissionController/src/com/android/permissioncontroller/permission/model/livedatatypes/LightPackageInfo.kt ++++ b/PermissionController/src/com/android/permissioncontroller/permission/model/livedatatypes/LightPackageInfo.kt +@@ -49,7 +49,9 @@ data class LightPackageInfo( + val firstInstallTime: Long + ) { + constructor(pI: PackageInfo) : this(pI.packageName, +- pI.permissions?.map { perm -> LightPermInfo(perm) } ?: emptyList(), ++ pI.permissions?.map { perm -> ++ LightPermInfo(perm, pI.applicationInfo!!.flags and ApplicationInfo.FLAG_SYSTEM != 0) ++ } ?: emptyList(), + pI.requestedPermissions?.toList() ?: emptyList(), + pI.requestedPermissionsFlags?.toList() ?: emptyList(), + pI.applicationInfo.uid, pI.applicationInfo.targetSdkVersion, +diff --git a/PermissionController/src/com/android/permissioncontroller/permission/model/livedatatypes/LightPermInfo.kt b/PermissionController/src/com/android/permissioncontroller/permission/model/livedatatypes/LightPermInfo.kt +index 3954b7472..582742da4 100644 +--- a/PermissionController/src/com/android/permissioncontroller/permission/model/livedatatypes/LightPermInfo.kt ++++ b/PermissionController/src/com/android/permissioncontroller/permission/model/livedatatypes/LightPermInfo.kt +@@ -30,6 +30,7 @@ import android.content.pm.PermissionInfo + * @param protection The protection level of this permission + * @param protection Extra information about the protection of this permission + * @param flags The system flags of this permission ++ * @param isSystem Whether this permission is defined by a system app + */ + data class LightPermInfo( + val name: String, +@@ -38,11 +39,13 @@ data class LightPermInfo( + val backgroundPermission: String?, + val protection: Int, + val protectionFlags: Int, +- val flags: Int ++ val flags: Int, ++ val isSystem: Boolean? + ) { +- constructor (permInfo: PermissionInfo): this(permInfo.name, permInfo.packageName, +- permInfo.group, permInfo.backgroundPermission, permInfo.protection, +- permInfo.protectionFlags, permInfo.flags) ++ constructor (permInfo: PermissionInfo, isSystem: Boolean?) : this( ++ permInfo.name, permInfo.packageName, permInfo.group, permInfo.backgroundPermission, ++ permInfo.protection, permInfo.protectionFlags, permInfo.flags, isSystem ++ ) + + /** + * Gets the PermissionInfo for this permission from the system. +diff --git a/PermissionController/src/com/android/permissioncontroller/permission/model/livedatatypes/LightPermission.kt b/PermissionController/src/com/android/permissioncontroller/permission/model/livedatatypes/LightPermission.kt +index c3d087fd2..45c3b1e92 100644 +--- a/PermissionController/src/com/android/permissioncontroller/permission/model/livedatatypes/LightPermission.kt ++++ b/PermissionController/src/com/android/permissioncontroller/permission/model/livedatatypes/LightPermission.kt +@@ -27,7 +27,7 @@ import com.android.permissioncontroller.permission.utils.Utils.isRuntimePlatform + * + * @param pkgInfo The package requesting the permission + * @param permInfo The permissionInfo this represents +- * @param isGrantedIncludingAppOp Whether or not this permission is functionally granted. ++ * @param isGranted Whether or not this permission is functionally granted. + * A non-granted app op but granted permission is counted as not granted + * @param flags The PermissionController flags for this permission + * @param foregroundPerms The foreground permission names corresponding to this permission, if this +@@ -36,7 +36,7 @@ import com.android.permissioncontroller.permission.utils.Utils.isRuntimePlatform + data class LightPermission( + val pkgInfo: LightPackageInfo, + val permInfo: LightPermInfo, +- val isGrantedIncludingAppOp: Boolean, ++ val isGranted: Boolean, + val flags: Int, + val foregroundPerms: List? + ) { +@@ -82,9 +82,9 @@ data class LightPermission( + val isRevokeWhenRequested = flags and PackageManager.FLAG_PERMISSION_REVOKE_WHEN_REQUESTED != 0 + /** Whether this permission is user sensitive in its current grant state */ + val isUserSensitive = !isRuntimePlatformPermission(permInfo.name) || +- (isGrantedIncludingAppOp && ++ (isGranted && + (flags and PackageManager.FLAG_PERMISSION_USER_SENSITIVE_WHEN_GRANTED) != 0) || +- (!isGrantedIncludingAppOp && ++ (!isGranted && + (flags and PackageManager.FLAG_PERMISSION_USER_SENSITIVE_WHEN_DENIED) != 0) + /** Whether the permission is restricted */ + val isRestricted = when { +@@ -105,10 +105,17 @@ data class LightPermission( + */ + val isSelectedLocationAccuracy = + flags and PackageManager.FLAG_PERMISSION_SELECTED_LOCATION_ACCURACY != 0 ++ /** Whether this permission is defined by platform or a system app */ ++ val isPlatformOrSystem = permInfo.packageName == Utils.OS_PKG || permInfo.isSystem == true ++ /** ++ * Whether this permission is granted including app op and does not hold the ++ * PackageManager.FLAG_PERMISSION_REVOKE_WHEN_REQUESTED flag. ++ */ ++ val allowFullGroupGrant = isGranted && !isRevokeWhenRequested + + override fun toString() = buildString { + append(name) +- if (isGrantedIncludingAppOp) append(", Granted") else append(", NotGranted") ++ if (isGranted) append(", Granted") else append(", NotGranted") + if (isPolicyFixed) append(", PolicyFixed") + if (isSystemFixed) append(", SystemFixed") + if (isUserFixed) append(", UserFixed") +diff --git a/PermissionController/src/com/android/permissioncontroller/permission/service/AutoRevokePermissions.kt b/PermissionController/src/com/android/permissioncontroller/permission/service/AutoRevokePermissions.kt +index aed275d8a..436612d58 100644 +--- a/PermissionController/src/com/android/permissioncontroller/permission/service/AutoRevokePermissions.kt ++++ b/PermissionController/src/com/android/permissioncontroller/permission/service/AutoRevokePermissions.kt +@@ -102,7 +102,7 @@ suspend fun revokeAppPermissions( + .getInitializedValue() ?: continue + val fixed = group.isBackgroundFixed || group.isForegroundFixed + val granted = group.permissions.any { (_, perm) -> +- perm.isGrantedIncludingAppOp && perm.name !in EXEMPT_PERMISSIONS ++ perm.isGranted && perm.name !in EXEMPT_PERMISSIONS + } + if (!fixed && granted && + !group.isGrantedByDefault && +diff --git a/PermissionController/src/com/android/permissioncontroller/permission/service/RuntimePermissionsUpgradeController.kt b/PermissionController/src/com/android/permissioncontroller/permission/service/RuntimePermissionsUpgradeController.kt +index e105a69bc..12d7d45ba 100644 +--- a/PermissionController/src/com/android/permissioncontroller/permission/service/RuntimePermissionsUpgradeController.kt ++++ b/PermissionController/src/com/android/permissioncontroller/permission/service/RuntimePermissionsUpgradeController.kt +@@ -389,7 +389,7 @@ internal object RuntimePermissionsUpgradeController { + + val allPermissionsWithxemption = bgApp.allPermissions.toMutableMap() + allPermissionsWithxemption[permission.ACCESS_BACKGROUND_LOCATION] = +- LightPermission(perm.pkgInfo, perm.permInfo, perm.isGrantedIncludingAppOp, ++ LightPermission(perm.pkgInfo, perm.permInfo, perm.isGranted, + perm.flags or FLAG_PERMISSION_RESTRICTION_UPGRADE_EXEMPT, + perm.foregroundPerms) + +@@ -451,7 +451,7 @@ internal object RuntimePermissionsUpgradeController { + ?: continue + + if (!perm.isUserSet && !perm.isSystemFixed && !perm.isPolicyFixed && +- !perm.isGrantedIncludingAppOp) { ++ !perm.isGranted) { + grants.add(Grant(false, appPermGroup, + listOf(permission.ACCESS_MEDIA_LOCATION))) + } +diff --git a/PermissionController/src/com/android/permissioncontroller/permission/ui/handheld/ReviewPermissionsFragment.java b/PermissionController/src/com/android/permissioncontroller/permission/ui/handheld/ReviewPermissionsFragment.java +index a6f74c822..3bfe7ee69 100644 +--- a/PermissionController/src/com/android/permissioncontroller/permission/ui/handheld/ReviewPermissionsFragment.java ++++ b/PermissionController/src/com/android/permissioncontroller/permission/ui/handheld/ReviewPermissionsFragment.java +@@ -257,11 +257,11 @@ public final class ReviewPermissionsFragment extends PreferenceFragmentCompat + PermissionControllerStatsLog.write(REVIEW_PERMISSIONS_FRAGMENT_RESULT_REPORTED, + changeId, mViewModel.getPackageInfo().applicationInfo.uid, + group.getPackageName(), +- permission.getName(), permission.isGrantedIncludingAppOp()); ++ permission.getName(), permission.isGranted()); + Log.v(LOG_TAG, "Permission grant via permission review changeId=" + changeId + " uid=" + + mViewModel.getPackageInfo().applicationInfo.uid + " packageName=" + + group.getPackageName() + " permission=" +- + permission.getName() + " granted=" + permission.isGrantedIncludingAppOp()); ++ + permission.getName() + " granted=" + permission.isGranted()); + } + } + +diff --git a/PermissionController/src/com/android/permissioncontroller/permission/ui/model/AppPermissionViewModel.kt b/PermissionController/src/com/android/permissioncontroller/permission/ui/model/AppPermissionViewModel.kt +index e743d7c81..d4d63e31f 100644 +--- a/PermissionController/src/com/android/permissioncontroller/permission/ui/model/AppPermissionViewModel.kt ++++ b/PermissionController/src/com/android/permissioncontroller/permission/ui/model/AppPermissionViewModel.kt +@@ -868,7 +868,7 @@ class AppPermissionViewModel( + + private fun getIndividualPermissionDetailResId(group: LightAppPermGroup): Pair { + return when (val numRevoked = +- group.permissions.filter { !it.value.isGrantedIncludingAppOp }.size) { ++ group.permissions.filter { !it.value.isGranted }.size) { + 0 -> R.string.permission_revoked_none to numRevoked + group.permissions.size -> R.string.permission_revoked_all to numRevoked + else -> R.string.permission_revoked_count to numRevoked +@@ -938,11 +938,11 @@ class AppPermissionViewModel( + for ((permName, permission) in oldGroup.permissions) { + val newPermission = newGroup.permissions[permName] ?: continue + +- if (permission.isGrantedIncludingAppOp != newPermission.isGrantedIncludingAppOp || ++ if (permission.isGranted != newPermission.isGranted || + permission.flags != newPermission.flags) { + logAppPermissionFragmentActionReported(changeId, newPermission, buttonPressed) + PermissionDecisionStorageImpl.recordPermissionDecision(app.applicationContext, +- packageName, permGroupName, newPermission.isGrantedIncludingAppOp) ++ packageName, permGroupName, newPermission.isGranted) + } + } + } +@@ -955,10 +955,10 @@ class AppPermissionViewModel( + val uid = KotlinUtils.getPackageUid(app, packageName, user) ?: return + PermissionControllerStatsLog.write(APP_PERMISSION_FRAGMENT_ACTION_REPORTED, sessionId, + changeId, uid, packageName, permission.permInfo.name, +- permission.isGrantedIncludingAppOp, permission.flags, buttonPressed) ++ permission.isGranted, permission.flags, buttonPressed) + Log.v(LOG_TAG, "Permission changed via UI with sessionId=$sessionId changeId=" + + "$changeId uid=$uid packageName=$packageName permission=" + permission.permInfo.name + +- " isGranted=" + permission.isGrantedIncludingAppOp + " permissionFlags=" + ++ " isGranted=" + permission.isGranted + " permissionFlags=" + + permission.flags + " buttonPressed=$buttonPressed") + } + +diff --git a/PermissionController/src/com/android/permissioncontroller/permission/ui/model/GrantPermissionsViewModel.kt b/PermissionController/src/com/android/permissioncontroller/permission/ui/model/GrantPermissionsViewModel.kt +index 71f729dc4..1fcd087ff 100644 +--- a/PermissionController/src/com/android/permissioncontroller/permission/ui/model/GrantPermissionsViewModel.kt ++++ b/PermissionController/src/com/android/permissioncontroller/permission/ui/model/GrantPermissionsViewModel.kt +@@ -261,7 +261,7 @@ class GrantPermissionsViewModel( + // REVIEW_REQUIRED flag setting + for ((key, state) in states) { + val allAffectedGranted = state.affectedPermissions.all { perm -> +- appPermGroup.permissions[perm]?.isGrantedIncludingAppOp == true && ++ appPermGroup.permissions[perm]?.isGranted == true && + appPermGroup.permissions[perm]?.isRevokeWhenRequested == false + } && !appPermGroup.isRuntimePermReviewRequired + if (allAffectedGranted) { +@@ -300,7 +300,7 @@ class GrantPermissionsViewModel( + for (perm in fgState.affectedPermissions) { + minSdkForOrderedSplitPermissions = maxOf(minSdkForOrderedSplitPermissions, + splitPermissionTargetSdkMap.getOrDefault(perm, 0)) +- if (fgGroup.permissions[perm]?.isGrantedIncludingAppOp == false) { ++ if (fgGroup.permissions[perm]?.isGranted == false) { + // If any of the requested permissions is not granted, + // needFgPermissions = true + needFgPermissions = true +@@ -450,7 +450,7 @@ class GrantPermissionsViewModel( + fgState.affectedPermissions.contains(ACCESS_FINE_LOCATION)) { + val coarseLocationPerm = + groupState.group.allPermissions[ACCESS_COARSE_LOCATION] +- if (coarseLocationPerm?.isGrantedIncludingAppOp == true) { ++ if (coarseLocationPerm?.isGranted == true) { + // Upgrade flow + locationVisibilities[DIALOG_WITH_FINE_LOCATION_ONLY] = true + message = RequestMessage.FG_FINE_LOCATION_MESSAGE +@@ -710,7 +710,7 @@ class GrantPermissionsViewModel( + + // Do not attempt to grant background access if foreground access is not either already + // granted or requested +- if (isBackground && !group.foreground.isGrantedExcludingRWROrAllRWR && ++ if (isBackground && !group.foreground.allowFullGroupGrant && + !hasForegroundRequest) { + Log.w(LOG_TAG, "Cannot grant $perm as the matching foreground permission is not " + + "already granted.") +@@ -724,18 +724,18 @@ class GrantPermissionsViewModel( + + // TODO(b/205888750): remove isRuntimePermReview line once confident in + // REVIEW_REQUIRED flag setting +- if ((isBackground && group.background.isGrantedExcludingRWROrAllRWR || +- !isBackground && group.foreground.isGrantedExcludingRWROrAllRWR) && ++ if ((isBackground && group.background.allowFullGroupGrant || ++ !isBackground && group.foreground.allowFullGroupGrant) && + !group.isRuntimePermReviewRequired) { + // If FINE location is not granted, do not grant it automatically when COARSE + // location is already granted. + if (group.permGroupName == LOCATION && +- group.allPermissions[ACCESS_FINE_LOCATION]?.isGrantedIncludingAppOp ++ group.allPermissions[ACCESS_FINE_LOCATION]?.isGranted + == false) { + return STATE_UNKNOWN + } + +- if (group.permissions[perm]?.isGrantedIncludingAppOp == false) { ++ if (group.permissions[perm]?.isGranted == false) { + if (isBackground) { + KotlinUtils.grantBackgroundRuntimePermissions(app, group, listOf(perm)) + } else { +@@ -939,28 +939,40 @@ class GrantPermissionsViewModel( + } else { + PERMISSION_GRANT_REQUEST_RESULT_REPORTED__RESULT__USER_GRANTED + } ++ var affectedPermissions: List = groupState.affectedPermissions + if (groupState.isBackground) { + KotlinUtils.grantBackgroundRuntimePermissions(app, groupState.group, +- groupState.affectedPermissions) ++ affectedPermissions) + } else { + if (affectedForegroundPermissions == null) { + KotlinUtils.grantForegroundRuntimePermissions(app, groupState.group, +- groupState.affectedPermissions, isOneTime) ++ affectedPermissions, isOneTime) + // This prevents weird flag state when app targetSDK switches from S+ to R- + if (groupState.affectedPermissions.contains(ACCESS_FINE_LOCATION)) { + KotlinUtils.setFlagsWhenLocationAccuracyChanged( + app, groupState.group, true) + } + } else { ++ affectedPermissions = affectedForegroundPermissions + val newGroup = KotlinUtils.grantForegroundRuntimePermissions(app, +- groupState.group, affectedForegroundPermissions, isOneTime) ++ groupState.group, affectedPermissions, isOneTime) + if (!isOneTime || newGroup.isOneTime) { + KotlinUtils.setFlagsWhenLocationAccuracyChanged(app, newGroup, + affectedForegroundPermissions.contains(ACCESS_FINE_LOCATION)) + } + } + } +- groupState.state = STATE_ALLOWED ++ val shouldDenyFullGroupGrant = ++ groupState.group.isPlatformPermissionGroup && ++ affectedPermissions.none { ++ groupState.group.permissions[it]?.isPlatformOrSystem == true ++ } ++ groupState.state = ++ if (shouldDenyFullGroupGrant) { ++ STATE_UNKNOWN ++ } else { ++ STATE_ALLOWED ++ } + } else { + if (groupState.isBackground) { + KotlinUtils.revokeBackgroundRuntimePermissions(app, groupState.group, +diff --git a/PermissionController/src/com/android/permissioncontroller/permission/ui/model/ReviewPermissionsViewModel.kt b/PermissionController/src/com/android/permissioncontroller/permission/ui/model/ReviewPermissionsViewModel.kt +index 94d0df0d5..79b2c87d8 100644 +--- a/PermissionController/src/com/android/permissioncontroller/permission/ui/model/ReviewPermissionsViewModel.kt ++++ b/PermissionController/src/com/android/permissioncontroller/permission/ui/model/ReviewPermissionsViewModel.kt +@@ -133,7 +133,7 @@ class ReviewPermissionsViewModel( + val lightPerms = permGroup.allPermissions.values.toList() + val permissionCount = lightPerms.size + for (i in 0 until permissionCount) { +- if (!lightPerms[i].isGrantedIncludingAppOp) { ++ if (!lightPerms[i].isGranted) { + revokedCount++ + } + } +diff --git a/PermissionController/src/com/android/permissioncontroller/permission/utils/KotlinUtils.kt b/PermissionController/src/com/android/permissioncontroller/permission/utils/KotlinUtils.kt +index 2216802f3..e7f4874e4 100644 +--- a/PermissionController/src/com/android/permissioncontroller/permission/utils/KotlinUtils.kt ++++ b/PermissionController/src/com/android/permissioncontroller/permission/utils/KotlinUtils.kt +@@ -470,7 +470,7 @@ object KotlinUtils { + group.userHandle, *flags) + } + newPerms[permName] = LightPermission(group.packageInfo, perm.permInfo, +- perm.isGrantedIncludingAppOp, perm.flags or flagsToSet, perm.foregroundPerms) ++ perm.isGranted, perm.flags or flagsToSet, perm.foregroundPerms) + } + return LightAppPermGroup(group.packageInfo, group.permGroupInfo, newPerms, + group.hasInstallToRuntimeSplit, group.specialLocationGrant) +@@ -560,7 +560,7 @@ object KotlinUtils { + val newGroup = LightAppPermGroup(group.packageInfo, group.permGroupInfo, newPerms, + group.hasInstallToRuntimeSplit, group.specialLocationGrant) + // If any permission in the group is one time granted, start one time permission session. +- if (newGroup.permissions.any { it.value.isOneTime && it.value.isGrantedIncludingAppOp }) { ++ if (newGroup.permissions.any { it.value.isOneTime && it.value.isGranted }) { + if (SdkLevel.isAtLeastT()) { + app.getSystemService(PermissionManager::class.java)!!.startOneTimePermissionSession( + group.packageName, Utils.getOneTimePermissionsTimeout(), +@@ -605,11 +605,11 @@ object KotlinUtils { + } + + var newFlags = perm.flags +- var isGranted = perm.isGrantedIncludingAppOp ++ var isGranted = perm.isGranted + var shouldKill = false + + // Grant the permission if needed. +- if (!perm.isGrantedIncludingAppOp) { ++ if (!perm.isGranted) { + val affectsAppOp = permissionToOp(perm.name) != null || perm.isBackgroundPermission + + // TODO 195016052: investigate adding split permission handling +@@ -653,14 +653,14 @@ object KotlinUtils { + + // If we newly grant background access to the fine location, double-guess the user some + // time later if this was really the right choice. +- if (!perm.isGrantedIncludingAppOp && isGranted) { ++ if (!perm.isGranted && isGranted) { + var triggerLocationAccessCheck = false + if (perm.name == ACCESS_FINE_LOCATION) { + val bgPerm = group.permissions[perm.backgroundPermission] +- triggerLocationAccessCheck = bgPerm?.isGrantedIncludingAppOp == true ++ triggerLocationAccessCheck = bgPerm?.isGranted == true + } else if (perm.name == ACCESS_BACKGROUND_LOCATION) { + val fgPerm = group.permissions[ACCESS_FINE_LOCATION] +- triggerLocationAccessCheck = fgPerm?.isGrantedIncludingAppOp == true ++ triggerLocationAccessCheck = fgPerm?.isGranted == true + } + if (triggerLocationAccessCheck) { + // trigger location access check +@@ -825,13 +825,13 @@ object KotlinUtils { + + val user = UserHandle.getUserHandleForUid(group.packageInfo.uid) + var newFlags = perm.flags +- var isGranted = perm.isGrantedIncludingAppOp ++ var isGranted = perm.isGranted + val supportsRuntime = group.packageInfo.targetSdkVersion >= Build.VERSION_CODES.M + var shouldKill = false + + val affectsAppOp = permissionToOp(perm.name) != null || perm.isBackgroundPermission + +- if (perm.isGrantedIncludingAppOp) { ++ if (perm.isGranted) { + if (supportsRuntime && !isPermissionSplitFromNonRuntime(app, perm.name, + group.packageInfo.targetSdkVersion)) { + // Revoke the permission if needed. +@@ -926,7 +926,7 @@ object KotlinUtils { + val fgPerm = group.permissions[foregroundPermName] + val appOpName = permissionToOp(foregroundPermName) ?: continue + +- if (fgPerm != null && fgPerm.isGrantedIncludingAppOp) { ++ if (fgPerm != null && fgPerm.isGranted) { + wasChanged = wasChanged || setOpMode(appOpName, uid, packageName, MODE_ALLOWED, + appOpsManager) + } +@@ -936,7 +936,7 @@ object KotlinUtils { + if (perm.backgroundPermission != null) { + wasChanged = if (group.permissions.containsKey(perm.backgroundPermission)) { + val bgPerm = group.permissions[perm.backgroundPermission] +- val mode = if (bgPerm != null && bgPerm.isGrantedIncludingAppOp) MODE_ALLOWED ++ val mode = if (bgPerm != null && bgPerm.isGranted) MODE_ALLOWED + else MODE_FOREGROUND + + setOpMode(appOpName, uid, packageName, mode, appOpsManager) +@@ -987,7 +987,7 @@ object KotlinUtils { + if (perm.isBackgroundPermission && perm.foregroundPerms != null) { + for (foregroundPermName in perm.foregroundPerms) { + val fgPerm = group.permissions[foregroundPermName] +- if (fgPerm != null && fgPerm.isGrantedIncludingAppOp) { ++ if (fgPerm != null && fgPerm.isGranted) { + val appOpName = permissionToOp(foregroundPermName) ?: return false + wasChanged = wasChanged || setOpMode(appOpName, uid, packageName, + MODE_FOREGROUND, appOpsManager) +diff --git a/PermissionController/src/com/android/permissioncontroller/permission/utils/SafetyNetLogger.java b/PermissionController/src/com/android/permissioncontroller/permission/utils/SafetyNetLogger.java +index f0227cad5..96eccd0a8 100644 +--- a/PermissionController/src/com/android/permissioncontroller/permission/utils/SafetyNetLogger.java ++++ b/PermissionController/src/com/android/permissioncontroller/permission/utils/SafetyNetLogger.java +@@ -162,7 +162,7 @@ public final class SafetyNetLogger { + } + + builder.append(permission.getName()).append('|'); +- builder.append(permission.isGrantedIncludingAppOp()).append('|'); ++ builder.append(permission.isGranted()).append('|'); + builder.append(permission.getFlags()); + } + } +diff --git a/PermissionController/tests/mocking/src/com/android/permissioncontroller/tests/mocking/permission/ui/model/ReviewPermissionsViewModelTest.kt b/PermissionController/tests/mocking/src/com/android/permissioncontroller/tests/mocking/permission/ui/model/ReviewPermissionsViewModelTest.kt +index df6e92154..7e95fcef8 100644 +--- a/PermissionController/tests/mocking/src/com/android/permissioncontroller/tests/mocking/permission/ui/model/ReviewPermissionsViewModelTest.kt ++++ b/PermissionController/tests/mocking/src/com/android/permissioncontroller/tests/mocking/permission/ui/model/ReviewPermissionsViewModelTest.kt +@@ -110,7 +110,7 @@ class ReviewPermissionsViewModelTest { + permissionsMap["mockedPermission1"] = permission2 + + whenever(permGroup.allPermissions).thenReturn(permissionsMap) +- whenever(permission1.isGrantedIncludingAppOp).thenReturn(true) ++ whenever(permission1.isGranted).thenReturn(true) + + val summary = model.getSummaryForIndividuallyControlledPermGroup(permGroup) + assertEquals( +diff --git a/PermissionController/tests/mocking/src/com/android/permissioncontroller/tests/mocking/permission/utils/GrantRevokeTests.kt b/PermissionController/tests/mocking/src/com/android/permissioncontroller/tests/mocking/permission/utils/GrantRevokeTests.kt +index df4c4e80f..0d0edc866 100644 +--- a/PermissionController/tests/mocking/src/com/android/permissioncontroller/tests/mocking/permission/utils/GrantRevokeTests.kt ++++ b/PermissionController/tests/mocking/src/com/android/permissioncontroller/tests/mocking/permission/utils/GrantRevokeTests.kt +@@ -24,6 +24,7 @@ import android.app.AppOpsManager.MODE_FOREGROUND + import android.app.AppOpsManager.MODE_IGNORED + import android.app.AppOpsManager.permissionToOp + import android.app.Application ++import android.content.pm.ApplicationInfo + import android.content.pm.PackageManager + import android.content.pm.PackageManager.FLAG_PERMISSION_AUTO_REVOKED + import android.content.pm.PackageManager.FLAG_PERMISSION_ONE_TIME +@@ -181,7 +182,8 @@ class GrantRevokeTests { + permInfoProtectionFlags: Int = 0 + ): LightPermission { + val permInfo = LightPermInfo(permName, TEST_PACKAGE_NAME, PERM_GROUP_NAME, backgroundPerm, +- PermissionInfo.PROTECTION_DANGEROUS, permInfoProtectionFlags, 0) ++ PermissionInfo.PROTECTION_DANGEROUS, permInfoProtectionFlags, 0, ++ pkgInfo.appFlags and ApplicationInfo.FLAG_SYSTEM != 0) + return LightPermission(pkgInfo, permInfo, + pkgInfo.requestedPermissionsFlags[pkgInfo.requestedPermissions.indexOf(permName)] + == PERMISSION_GRANTED, flags, foregroundPerms) +@@ -252,7 +254,7 @@ class GrantRevokeTests { + val flags = state.second + + assertWithMessage("permission $permName grant state incorrect") +- .that(perms[permName]?.isGrantedIncludingAppOp).isEqualTo(granted) ++ .that(perms[permName]?.isGranted).isEqualTo(granted) + + val actualFlags = perms[permName]!!.flags + assertWithMessage("permission $permName flags incorrect, expected" + +-- +2.46.1.824.gd892dcdcdd-goog + diff --git a/aosp_diff/preliminary/packages/services/Telephony/05_0005-enforce-the-calling-package-is-the-default-dialer-for-VisualVoic.bulletin.patch b/aosp_diff/preliminary/packages/services/Telephony/05_0005-enforce-the-calling-package-is-the-default-dialer-for-VisualVoic.bulletin.patch new file mode 100644 index 0000000000..9ea0ad302a --- /dev/null +++ b/aosp_diff/preliminary/packages/services/Telephony/05_0005-enforce-the-calling-package-is-the-default-dialer-for-VisualVoic.bulletin.patch @@ -0,0 +1,46 @@ +From 49a83909c74db7a4d783926631e3ab7cc4ab7360 Mon Sep 17 00:00:00 2001 +From: Thomas Stuart +Date: Thu, 6 Jun 2024 22:34:33 +0000 +Subject: [PATCH] enforce the calling package is the default dialer for + VisualVoicemail + +In the docs for setVisualVoicemailSmsFilterSettings, they state that +the callingPackage needs to be the default dialer. However, server side, +there is no enforcement for this. Now, every client is verified to be +the default dialer, system dialer, or carrier visual voicemail app +before changing the settings for visual voicemail. + +Bug: 308932906 +Test: CTS +(cherry picked from https://googleplex-android-review.googlesource.com/q/commit:17a3adcb99b587027be2bebe9718457df689b4b4) +Merged-In: I951d7783f5c425c03768efb7aee7a38e299e6536 +Change-Id: I951d7783f5c425c03768efb7aee7a38e299e6536 +--- + src/com/android/phone/PhoneInterfaceManager.java | 4 ++-- + 1 file changed, 2 insertions(+), 2 deletions(-) + +diff --git a/src/com/android/phone/PhoneInterfaceManager.java b/src/com/android/phone/PhoneInterfaceManager.java +index 66cef648e..ad0a4e586 100755 +--- a/src/com/android/phone/PhoneInterfaceManager.java ++++ b/src/com/android/phone/PhoneInterfaceManager.java +@@ -3788,7 +3788,7 @@ public class PhoneInterfaceManager extends ITelephony.Stub { + public void enableVisualVoicemailSmsFilter(String callingPackage, int subId, + VisualVoicemailSmsFilterSettings settings) { + mAppOps.checkPackage(Binder.getCallingUid(), callingPackage); +- ++ enforceVisualVoicemailPackage(callingPackage, subId); + final long identity = Binder.clearCallingIdentity(); + try { + VisualVoicemailSmsFilterConfig.enableVisualVoicemailSmsFilter( +@@ -3801,7 +3801,7 @@ public class PhoneInterfaceManager extends ITelephony.Stub { + @Override + public void disableVisualVoicemailSmsFilter(String callingPackage, int subId) { + mAppOps.checkPackage(Binder.getCallingUid(), callingPackage); +- ++ enforceVisualVoicemailPackage(callingPackage, subId); + final long identity = Binder.clearCallingIdentity(); + try { + VisualVoicemailSmsFilterConfig.disableVisualVoicemailSmsFilter( +-- +2.46.1.824.gd892dcdcdd-goog +