From 2fc07deafd257dcc934bb802887ef4297f49548f Mon Sep 17 00:00:00 2001 From: Austin Borger Date: Thu, 9 Feb 2023 16:00:02 -0800 Subject: [PATCH] Fix vulnerability in AttributionSource due to incorrect Binder call AttributionSource uses Binder.getCallingUid to verify the UID of the caller from another process. However, getCallingUid does not always behave as expected. If the AttributionSource is unparceled outside a transaction thread, which is quite possible, getCallingUid will return the UID of the current process instead. If this is a system process, the UID check gets bypassed entirely, meaning any uid can be provided. This patch fixes the vulnerability by emptying out the state of the AttributionSource, so that the service checking its credentials will fail to give permission to the app. Bug: 267231571 Test: v2/android-virtual-infra/test_mapping/presubmit-avd Merged-In: Ic301a8518b8e57e1c9a2c9f2f845e51dca145257 Change-Id: Ifdc53f87b7ef53b69cc01ec1955b4cb1dfd3345b Merged-In: Ic985afdb5e4495f2e051e6d0d62d39c6660c585e --- .../android/content/AttributionSource.java | 20 ++++++++++++++++--- 1 file changed, 17 insertions(+), 3 deletions(-) diff --git a/core/java/android/content/AttributionSource.java b/core/java/android/content/AttributionSource.java index 2f61fee88e9fa..ec56f9a7cf0c8 100644 --- a/core/java/android/content/AttributionSource.java +++ b/core/java/android/content/AttributionSource.java @@ -30,6 +30,7 @@ import android.os.Parcelable; import android.os.Process; import android.permission.PermissionManager; import android.util.ArraySet; +import android.util.Log; import com.android.internal.annotations.Immutable; @@ -86,6 +87,8 @@ import java.util.Set; */ @Immutable public final class AttributionSource implements Parcelable { + private static final String TAG = "AttributionSource"; + private static final String DESCRIPTOR = "android.content.AttributionSource"; private static final Binder sDefaultToken = new Binder(DESCRIPTOR); @@ -153,9 +156,20 @@ public final class AttributionSource implements Parcelable { AttributionSource(@NonNull Parcel in) { this(AttributionSourceState.CREATOR.createFromParcel(in)); - // Since we just unpacked this object as part of it transiting a Binder - // call, this is the perfect time to enforce that its UID and PID can be trusted - enforceCallingUidAndPid(); + if (!Binder.isHandlingTransaction()) { + Log.e(TAG, "Unable to verify calling UID #" + mAttributionSourceState.uid + " PID #" + + mAttributionSourceState.pid + " when not handling Binder transaction; " + + "clearing."); + mAttributionSourceState.pid = -1; + mAttributionSourceState.uid = -1; + mAttributionSourceState.packageName = null; + mAttributionSourceState.attributionTag = null; + mAttributionSourceState.next = null; + } else { + // Since we just unpacked this object as part of it transiting a Binder + // call, this is the perfect time to enforce that its UID and PID can be trusted + enforceCallingUidAndPid(); + } } /** @hide */