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

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<ComponentName>(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<Control>(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<PrivateParcelable?> {
+            override fun createFromParcel(source: Parcel?): PrivateParcelable {
+                return PrivateParcelable()
+            }
+
+            override fun newArray(size: Int): Array<PrivateParcelable?> {
+                return arrayOfNulls(size)
+            }
+        }
+    }
 }
\ No newline at end of file