From 0719e745b6b49c976c34b0c8f5cd0a8779b48a8c Mon Sep 17 00:00:00 2001 From: Fabián Kozynski Date: Tue, 19 Mar 2024 13:02:09 -0400 Subject: Properly check for malformed extras In ControlsRequestReceiver, we trust the extras in the intent, but because it could be assembled by a malicious app, those could be malformed, causing SystemUI to crash. Make sure that we properly check for exceptions in loading the parcellables as well as missing data and return early (after logging an error). Fixes: 329369938 Flag: None Test: manual, using test apk Test: atest ControlsRequestReceiverTest Change-Id: I945d2c5a8daf56c0708de5ccdd6020cac857827a --- .../controls/management/ControlsRequestReceiver.kt | 19 ++++-- .../management/ControlsRequestReceiverTest.kt | 77 ++++++++++++++++++++++ 2 files changed, 89 insertions(+), 7 deletions(-) diff --git a/packages/SystemUI/src/com/android/systemui/controls/management/ControlsRequestReceiver.kt b/packages/SystemUI/src/com/android/systemui/controls/management/ControlsRequestReceiver.kt index 7e5b26732e00..c110d06aeedb 100644 --- a/packages/SystemUI/src/com/android/systemui/controls/management/ControlsRequestReceiver.kt +++ b/packages/SystemUI/src/com/android/systemui/controls/management/ControlsRequestReceiver.kt @@ -28,7 +28,6 @@ import android.os.UserHandle import android.service.controls.Control import android.service.controls.ControlsProviderService import android.util.Log -import java.lang.ClassCastException /** * Proxy to launch in user 0 @@ -61,22 +60,28 @@ class ControlsRequestReceiver : BroadcastReceiver() { } val targetComponent = try { - intent.getParcelableExtra(Intent.EXTRA_COMPONENT_NAME) - } catch (e: ClassCastException) { + intent.getParcelableExtra(Intent.EXTRA_COMPONENT_NAME, ComponentName::class.java) + } catch (e: Exception) { Log.e(TAG, "Malformed intent extra ComponentName", e) return + } ?: run { + Log.e(TAG, "Null target component") + return } val control = try { - intent.getParcelableExtra(ControlsProviderService.EXTRA_CONTROL) - } catch (e: ClassCastException) { + intent.getParcelableExtra(ControlsProviderService.EXTRA_CONTROL, Control::class.java) + } catch (e: Exception) { Log.e(TAG, "Malformed intent extra Control", e) return + } ?: run { + Log.e(TAG, "Null control") + return } - val packageName = targetComponent?.packageName + val packageName = targetComponent.packageName - if (packageName == null || !isPackageInForeground(context, packageName)) { + if (!isPackageInForeground(context, packageName)) { return } diff --git a/packages/SystemUI/tests/src/com/android/systemui/controls/management/ControlsRequestReceiverTest.kt b/packages/SystemUI/tests/src/com/android/systemui/controls/management/ControlsRequestReceiverTest.kt index 890b9aec69bf..ae77d1f590e3 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/controls/management/ControlsRequestReceiverTest.kt +++ b/packages/SystemUI/tests/src/com/android/systemui/controls/management/ControlsRequestReceiverTest.kt @@ -24,6 +24,9 @@ import android.content.Context import android.content.ContextWrapper import android.content.Intent import android.content.pm.PackageManager +import android.os.Bundle +import android.os.Parcel +import android.os.Parcelable import android.os.UserHandle import android.service.controls.Control import android.service.controls.ControlsProviderService @@ -176,6 +179,64 @@ class ControlsRequestReceiverTest : SysuiTestCase() { assertNull(wrapper.intent) } + @Test + fun testClassNotFoundExceptionComponent_noCrash() { + val bundle = Bundle().apply { + putParcelable(Intent.EXTRA_COMPONENT_NAME, PrivateParcelable()) + putParcelable(ControlsProviderService.EXTRA_CONTROL, control) + } + val parcel = Parcel.obtain() + bundle.writeToParcel(parcel, 0) + + parcel.setDataPosition(0) + + val badIntent = Intent(ControlsProviderService.ACTION_ADD_CONTROL).apply { + parcel.readBundle()?.let { putExtras(it) } + } + receiver.onReceive(wrapper, badIntent) + + assertNull(wrapper.intent) + } + + @Test + fun testClassNotFoundExceptionControl_noCrash() { + val bundle = Bundle().apply { + putParcelable(Intent.EXTRA_COMPONENT_NAME, componentName) + putParcelable(ControlsProviderService.EXTRA_CONTROL, PrivateParcelable()) + } + val parcel = Parcel.obtain() + bundle.writeToParcel(parcel, 0) + + parcel.setDataPosition(0) + + val badIntent = Intent(ControlsProviderService.ACTION_ADD_CONTROL).apply { + parcel.readBundle()?.let { putExtras(it) } + } + receiver.onReceive(wrapper, badIntent) + + assertNull(wrapper.intent) + } + + @Test + fun testMissingComponentName_noCrash() { + val badIntent = Intent(ControlsProviderService.ACTION_ADD_CONTROL).apply { + putExtra(ControlsProviderService.EXTRA_CONTROL, control) + } + receiver.onReceive(wrapper, badIntent) + + assertNull(wrapper.intent) + } + + @Test + fun testMissingControl_noCrash() { + val badIntent = Intent(ControlsProviderService.ACTION_ADD_CONTROL).apply { + putExtra(Intent.EXTRA_COMPONENT_NAME, componentName) + } + receiver.onReceive(wrapper, badIntent) + + assertNull(wrapper.intent) + } + class MyWrapper(context: Context) : ContextWrapper(context) { var intent: Intent? = null @@ -189,4 +250,20 @@ class ControlsRequestReceiverTest : SysuiTestCase() { this.intent = intent } } + + class PrivateParcelable : Parcelable { + override fun describeContents() = 0 + + override fun writeToParcel(dest: Parcel, flags: Int) {} + + companion object CREATOR : Parcelable.Creator { + override fun createFromParcel(source: Parcel?): PrivateParcelable { + return PrivateParcelable() + } + + override fun newArray(size: Int): Array { + return arrayOfNulls(size) + } + } + } } \ No newline at end of file -- cgit v1.2.3-59-g8ed1b