Skip to content

Commit

Permalink
ASB-July 2024 Security Patches integration
Browse files Browse the repository at this point in the history
Integrating Google Android Security Bulletin Patches

Test done: STS r28 TCs Passed.

Tracked-On: OAM-121183
Signed-off-by: Alam, Sahibex <[email protected]>
  • Loading branch information
AlamIntel authored and sysopenci committed Jul 5, 2024
1 parent b49e538 commit 13ed7cb
Show file tree
Hide file tree
Showing 8 changed files with 997 additions and 1 deletion.
Original file line number Diff line number Diff line change
Expand Up @@ -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-06-01
+ PLATFORM_SECURITY_PATCH := 2024-07-01
endif

include $(BUILD_SYSTEM)/version_util.mk
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
From e6c33e169447cff7675cc6b8686f3f0e03f49c71 Mon Sep 17 00:00:00 2001
From: Ivan Chiang <[email protected]>
Date: Mon, 18 Mar 2024 02:46:56 +0000
Subject: [PATCH] [PM] Send ACTION_PACKAGE_CHANGED when mimeGroups are changed

Test: atest CtsPackageManagerTestCases:PackageManagerShellCommandMultiUserTest
Test: atest CtsPackageManagerTestCases:PackageManagerTest
Bug: 297517712
(cherry picked from https://googleplex-android-review.googlesource.com/q/commit:81eb9f8294645684ce1fad39d5d4a00ef11736e4)
(cherry picked from https://googleplex-android-review.googlesource.com/q/commit:c160424ef22bffd25a9cc9bc7b901ae1b9721a72)
(cherry picked from https://googleplex-android-review.googlesource.com/q/commit:6d9520bb9be2e31fd43bb08f0017838bbd389883)
Merged-In: I271a3526ea4555249e3a2797605269257330e0e9
Change-Id: I271a3526ea4555249e3a2797605269257330e0e9
---
.../server/pm/PackageManagerService.java | 23 ++++++++++++++++---
1 file changed, 20 insertions(+), 3 deletions(-)

diff --git a/services/core/java/com/android/server/pm/PackageManagerService.java b/services/core/java/com/android/server/pm/PackageManagerService.java
index 9c901e885fc8..a02bb469e40d 100644
--- a/services/core/java/com/android/server/pm/PackageManagerService.java
+++ b/services/core/java/com/android/server/pm/PackageManagerService.java
@@ -5827,9 +5827,26 @@ public class PackageManagerService implements PackageSender, TestUtilityService
packageStateWrite.setMimeGroup(mimeGroup, mimeTypesSet);
});
if (mComponentResolver.updateMimeGroup(snapshotComputer(), packageName, mimeGroup)) {
- Binder.withCleanCallingIdentity(() ->
- mPreferredActivityHelper.clearPackagePreferredActivities(packageName,
- UserHandle.USER_ALL));
+ Binder.withCleanCallingIdentity(() -> {
+ mPreferredActivityHelper.clearPackagePreferredActivities(packageName,
+ UserHandle.USER_ALL);
+ // Send the ACTION_PACKAGE_CHANGED when the mimeGroup has changes
+ final Computer snapShot = snapshotComputer();
+ final ArrayList<String> components = new ArrayList<>(
+ Collections.singletonList(packageName));
+ final int appId = packageState.getAppId();
+ final int[] userIds = resolveUserIds(UserHandle.USER_ALL);
+ final String reason = "The mimeGroup is changed";
+ for (int i = 0; i < userIds.length; i++) {
+ final PackageUserStateInternal pkgUserState =
+ packageState.getUserStates().get(userIds[i]);
+ if (pkgUserState != null && pkgUserState.isInstalled()) {
+ final int packageUid = UserHandle.getUid(userIds[i], appId);
+ sendPackageChangedBroadcast(snapShot, packageName,
+ true /* dontKillApp */, components, packageUid, reason);
+ }
+ }
+ });
}

scheduleWriteSettings();
--
2.45.0.rc1.225.g2a3ae87e7f-goog

Original file line number Diff line number Diff line change
@@ -0,0 +1,85 @@
From d6271c284c8ba301c97018baf65f4195d2a298aa Mon Sep 17 00:00:00 2001
From: Bishoy Gendy <[email protected]>
Date: Thu, 11 Apr 2024 16:37:10 +0000
Subject: [PATCH] Fix security vulnerability allowing apps to start from
background

Bug: 317048338
Test: Using the steps in b/317048338#comment12
(cherry picked from commit c5fc8ea92c0aabbb2fdccc23b743c18a8bf62e64)
(cherry picked from https://googleplex-android-review.googlesource.com/q/commit:df3584bb93ab89d7e174f7d39e42d4b22cb92fe0)
Merged-In: Ia91199fdb23beed27bde687fdca8fe5d3a5a4759
Change-Id: Ia91199fdb23beed27bde687fdca8fe5d3a5a4759
---
.../media/session/ParcelableListBinder.java | 13 +++++++++++--
.../android/server/media/MediaSessionRecord.java | 14 ++++++++------
2 files changed, 19 insertions(+), 8 deletions(-)

diff --git a/media/java/android/media/session/ParcelableListBinder.java b/media/java/android/media/session/ParcelableListBinder.java
index bbf1e0889b68..d78828462b1e 100644
--- a/media/java/android/media/session/ParcelableListBinder.java
+++ b/media/java/android/media/session/ParcelableListBinder.java
@@ -45,6 +45,7 @@ public class ParcelableListBinder<T extends Parcelable> extends Binder {
private static final int END_OF_PARCEL = 0;
private static final int ITEM_CONTINUED = 1;

+ private final Class<T> mListElementsClass;
private final Consumer<List<T>> mConsumer;

private final Object mLock = new Object();
@@ -61,9 +62,11 @@ public class ParcelableListBinder<T extends Parcelable> extends Binder {
/**
* Creates an instance.
*
+ * @param listElementsClass the class of the list elements.
* @param consumer a consumer that consumes the list received
*/
- public ParcelableListBinder(@NonNull Consumer<List<T>> consumer) {
+ public ParcelableListBinder(Class<T> listElementsClass, @NonNull Consumer<List<T>> consumer) {
+ mListElementsClass = listElementsClass;
mConsumer = consumer;
}

@@ -83,7 +86,13 @@ public class ParcelableListBinder<T extends Parcelable> extends Binder {
mCount = data.readInt();
}
while (i < mCount && data.readInt() != END_OF_PARCEL) {
- mList.add(data.readParcelable(null));
+ Object object = data.readParcelable(null);
+ if (mListElementsClass.isAssignableFrom(object.getClass())) {
+ // Checking list items are of compaitible types to validate against malicious
+ // apps calling it directly via reflection with non compilable items.
+ // See b/317048338 for more details
+ mList.add((T) object);
+ }
i++;
}
if (i >= mCount) {
diff --git a/services/core/java/com/android/server/media/MediaSessionRecord.java b/services/core/java/com/android/server/media/MediaSessionRecord.java
index b459cfe6b44e..8f07b3924da0 100644
--- a/services/core/java/com/android/server/media/MediaSessionRecord.java
+++ b/services/core/java/com/android/server/media/MediaSessionRecord.java
@@ -1100,12 +1100,14 @@ public class MediaSessionRecord implements IBinder.DeathRecipient, MediaSessionR

@Override
public IBinder getBinderForSetQueue() throws RemoteException {
- return new ParcelableListBinder<QueueItem>((list) -> {
- synchronized (mLock) {
- mQueue = list;
- }
- mHandler.post(MessageHandler.MSG_UPDATE_QUEUE);
- });
+ return new ParcelableListBinder<QueueItem>(
+ QueueItem.class,
+ (list) -> {
+ synchronized (mLock) {
+ mQueue = list;
+ }
+ mHandler.post(MessageHandler.MSG_UPDATE_QUEUE);
+ });
}

@Override
--
2.45.0.rc1.225.g2a3ae87e7f-goog

Original file line number Diff line number Diff line change
@@ -0,0 +1,179 @@
From a03884119ae09221831e8dd5bb64ec13f4d721d5 Mon Sep 17 00:00:00 2001
From: Martijn Coenen <[email protected]>
Date: Thu, 29 Feb 2024 12:03:05 +0000
Subject: [PATCH] Verify UID of incoming Zygote connections.

Only the system UID should be allowed to connect to the Zygote. While
for generic Zygotes this is also covered by SELinux policy, this is not
true for App Zygotes: the preload code running in an app zygote could
connect to another app zygote socket, if it had access to its (random)
socket address.

On the Java layer, simply check the UID when the connection is made. In
the native layer, this check was already present, but it actually didn't
work in the case where we receive a new incoming connection on the
socket, and receive a 'non-fork' command: in that case, we will simply
exit the native loop, and let the Java layer handle the command, without
any further UID checking.

Modified the native logic to drop new connections with a mismatching
UID, and to keep serving the existing connection (if it was still
there).

Bug: 319081336
Test: manual
(cherry picked from commit 2ffc7cb220e4220b7e108c4043a3f0f2a85b6508)
(cherry picked from https://googleplex-android-review.googlesource.com/q/commit:f1d4b34ad51b6ccb84ab042486923da8b2451e0f)
Merged-In: I3f85a17107849e2cd3e82d6ef15c90b9e2f26532
Change-Id: I3f85a17107849e2cd3e82d6ef15c90b9e2f26532
---
.../android/internal/os/ZygoteConnection.java | 3 +
...ndroid_internal_os_ZygoteCommandBuffer.cpp | 82 ++++++++++++-------
2 files changed, 56 insertions(+), 29 deletions(-)

diff --git a/core/java/com/android/internal/os/ZygoteConnection.java b/core/java/com/android/internal/os/ZygoteConnection.java
index 993e4e7b4b3d..765901a043a0 100644
--- a/core/java/com/android/internal/os/ZygoteConnection.java
+++ b/core/java/com/android/internal/os/ZygoteConnection.java
@@ -93,6 +93,9 @@ class ZygoteConnection {
throw ex;
}

+ if (peer.getUid() != Process.SYSTEM_UID) {
+ throw new ZygoteSecurityException("Only system UID is allowed to connect to Zygote.");
+ }
isEof = false;
}

diff --git a/core/jni/com_android_internal_os_ZygoteCommandBuffer.cpp b/core/jni/com_android_internal_os_ZygoteCommandBuffer.cpp
index add645dee718..b48fe419cca5 100644
--- a/core/jni/com_android_internal_os_ZygoteCommandBuffer.cpp
+++ b/core/jni/com_android_internal_os_ZygoteCommandBuffer.cpp
@@ -353,6 +353,18 @@ jstring com_android_internal_os_ZygoteCommandBuffer_nativeNextArg(JNIEnv* env, j
return result;
}

+static uid_t getSocketPeerUid(int socket, const std::function<void(const std::string&)>& fail_fn) {
+ struct ucred credentials;
+ socklen_t cred_size = sizeof credentials;
+ if (getsockopt(socket, SOL_SOCKET, SO_PEERCRED, &credentials, &cred_size) == -1
+ || cred_size != sizeof credentials) {
+ fail_fn(CREATE_ERROR("Failed to get socket credentials, %s",
+ strerror(errno)));
+ }
+
+ return credentials.uid;
+}
+
// Read all lines from the current command into the buffer, and then reset the buffer, so
// we will start reading again at the beginning of the command, starting with the argument
// count. And we don't need access to the fd to do so.
@@ -411,19 +423,12 @@ jboolean com_android_internal_os_ZygoteCommandBuffer_nativeForkRepeatedly(
fail_fn_z("Failed to retrieve session socket timeout");
}

- struct ucred credentials;
- socklen_t cred_size = sizeof credentials;
- if (getsockopt(n_buffer->getFd(), SOL_SOCKET, SO_PEERCRED, &credentials, &cred_size) == -1
- || cred_size != sizeof credentials) {
- fail_fn_1(CREATE_ERROR("ForkRepeatedly failed to get initial credentials, %s",
- strerror(errno)));
+ uid_t peerUid = getSocketPeerUid(session_socket, fail_fn_1);
+ if (peerUid != static_cast<uid_t>(expected_uid)) {
+ return JNI_FALSE;
}
-
bool first_time = true;
do {
- if (credentials.uid != expected_uid) {
- return JNI_FALSE;
- }
n_buffer->readAllLines(first_time ? fail_fn_1 : fail_fn_n);
n_buffer->reset();
int pid = zygote::forkApp(env, /* no pipe FDs */ -1, -1, session_socket_fds,
@@ -453,30 +458,56 @@ jboolean com_android_internal_os_ZygoteCommandBuffer_nativeForkRepeatedly(
// Clear buffer and get count from next command.
n_buffer->clear();
for (;;) {
+ bool valid_session_socket = true;
// Poll isn't strictly necessary for now. But without it, disconnect is hard to detect.
int poll_res = TEMP_FAILURE_RETRY(poll(fd_structs, 2, -1 /* infinite timeout */));
if ((fd_structs[SESSION_IDX].revents & POLLIN) != 0) {
if (n_buffer->getCount(fail_fn_z) != 0) {
break;
- } // else disconnected;
+ } else {
+ // Session socket was disconnected
+ valid_session_socket = false;
+ close(session_socket);
+ }
} else if (poll_res == 0 || (fd_structs[ZYGOTE_IDX].revents & POLLIN) == 0) {
fail_fn_z(
CREATE_ERROR("Poll returned with no descriptors ready! Poll returned %d", poll_res));
}
- // We've now seen either a disconnect or connect request.
- close(session_socket);
- int new_fd = TEMP_FAILURE_RETRY(accept(zygote_socket_fd, nullptr, nullptr));
+ int new_fd = -1;
+ do {
+ // We've now seen either a disconnect or connect request.
+ new_fd = TEMP_FAILURE_RETRY(accept(zygote_socket_fd, nullptr, nullptr));
+ if (new_fd == -1) {
+ fail_fn_z(CREATE_ERROR("Accept(%d) failed: %s", zygote_socket_fd, strerror(errno)));
+ }
+ uid_t newPeerUid = getSocketPeerUid(new_fd, fail_fn_1);
+ if (newPeerUid != static_cast<uid_t>(expected_uid)) {
+ ALOGW("Dropping new connection with a mismatched uid %d\n", newPeerUid);
+ close(new_fd);
+ new_fd = -1;
+ } else {
+ // If we still have a valid session socket, close it now
+ if (valid_session_socket) {
+ close(session_socket);
+ }
+ valid_session_socket = true;
+ }
+ } while (!valid_session_socket);
+
+ // At this point we either have a valid new connection (new_fd > 0), or
+ // an existing session socket we can poll on
if (new_fd == -1) {
- fail_fn_z(CREATE_ERROR("Accept(%d) failed: %s", zygote_socket_fd, strerror(errno)));
+ // The new connection wasn't valid, and we still have an old one; retry polling
+ continue;
}
if (new_fd != session_socket) {
- // Move new_fd back to the old value, so that we don't have to change Java-level data
- // structures to reflect a change. This implicitly closes the old one.
- if (TEMP_FAILURE_RETRY(dup2(new_fd, session_socket)) != session_socket) {
- fail_fn_z(CREATE_ERROR("Failed to move fd %d to %d: %s",
- new_fd, session_socket, strerror(errno)));
- }
- close(new_fd); // On Linux, fd is closed even if EINTR is returned.
+ // Move new_fd back to the old value, so that we don't have to change Java-level data
+ // structures to reflect a change. This implicitly closes the old one.
+ if (TEMP_FAILURE_RETRY(dup2(new_fd, session_socket)) != session_socket) {
+ fail_fn_z(CREATE_ERROR("Failed to move fd %d to %d: %s",
+ new_fd, session_socket, strerror(errno)));
+ }
+ close(new_fd); // On Linux, fd is closed even if EINTR is returned.
}
// If we ever return, we effectively reuse the old Java ZygoteConnection.
// None of its state needs to change.
@@ -488,13 +519,6 @@ jboolean com_android_internal_os_ZygoteCommandBuffer_nativeForkRepeatedly(
fail_fn_z(CREATE_ERROR("Failed to set send timeout for socket %d: %s",
session_socket, strerror(errno)));
}
- if (getsockopt(session_socket, SOL_SOCKET, SO_PEERCRED, &credentials, &cred_size) == -1) {
- fail_fn_z(CREATE_ERROR("ForkMany failed to get credentials: %s", strerror(errno)));
- }
- if (cred_size != sizeof credentials) {
- fail_fn_z(CREATE_ERROR("ForkMany credential size = %d, should be %d",
- cred_size, static_cast<int>(sizeof credentials)));
- }
}
first_time = false;
} while (n_buffer->isSimpleForkCommand(minUid, fail_fn_n));
--
2.45.0.rc1.225.g2a3ae87e7f-goog

Loading

0 comments on commit 13ed7cb

Please sign in to comment.