Validate deep-link args when resolving the start location

navigation-fragments/src/main/java/dev/hotwire/navigation/navigator/NavigatorHost.kt

@@ -15,6 +15,7 @@ import dev.hotwire.navigation.activities.HotwireActivity
 import dev.hotwire.navigation.config.HotwireNavigation
 
 internal const val DEEPLINK_EXTRAS_KEY = "android-support-nav:controller:deepLinkExtras"
+internal const val DEEPLINK_ARGS_KEY = "android-support-nav:controller:deepLinkArgs"
 internal const val LOCATION_KEY = "location"
 
 open class NavigatorHost : NavHostFragment(), FragmentOnAttachListener {
@@ -72,22 +73,30 @@ open class NavigatorHost : NavHostFragment(), FragmentOnAttachListener {
     }
 
     /**
-     * Google's Navigation library automatically navigates to deep links provided in the
-     * Activity's Intent. This exposes a vulnerability for malicious Intents to open an arbitrary
-     * webpage outside of the app's domain, allowing javascript injection on the page. Ensure
-     * that deep link intents always match the app's domain.
+     * Google's Navigation library automatically navigates to deep links provided in the launching
+     * Intent, which lets a malicious Intent open an arbitrary page in the WebView. Sanitize the
+     * Intent's attacker-controllable deep-link arguments so the start location stays within the
+     * app's domain.
      */
     @VisibleForTesting(otherwise = PROTECTED)
     fun ensureDeeplinkStartLocationValid() {
-        val extrasBundle = activity.intent.extras?.getBundle(DEEPLINK_EXTRAS_KEY) ?: return
+        val intent = activity.intent
+
+        // NavController merges deepLinkArgs over the validated deepLinkExtras (last write wins), so
+        // empty each per-destination bundle to stop it overriding the validated start location.
+        intent.extras?.getParcelableArrayList<Bundle>(DEEPLINK_ARGS_KEY)?.let { args ->
+            intent.putParcelableArrayListExtra(DEEPLINK_ARGS_KEY, ArrayList(args.map { Bundle() }))
+        }
+
+        val extrasBundle = intent.extras?.getBundle(DEEPLINK_EXTRAS_KEY) ?: return
         val startLocation = extrasBundle.getString(LOCATION_KEY) ?: return
 
         val deepLinkStartUri = startLocation.toUri()
         val configStartUri = configuration.startLocation.toUri()
 
         if (deepLinkStartUri.host != configStartUri.host) {
             extrasBundle.putString(LOCATION_KEY, configuration.startLocation)
-            activity.intent.putExtra(DEEPLINK_EXTRAS_KEY, extrasBundle)
+            intent.putExtra(DEEPLINK_EXTRAS_KEY, extrasBundle)
         }
     }
 

navigation-fragments/src/test/kotlin/dev/hotwire/navigation/navigator/NavigatorHostTest.kt

@@ -1,6 +1,5 @@
 package dev.hotwire.navigation.navigator
 
-import android.R.attr.host
 import android.content.Intent
 import android.os.Bundle
 import androidx.core.os.bundleOf
@@ -25,28 +24,52 @@ class NavigatorHostTest {
 
     @Test
     fun `reverts to config start location when deep link host differs`() {
-        val extras = bundleOf(LOCATION_KEY to "https://other.com/path")
-        val intent = Intent().apply { putExtra(DEEPLINK_EXTRAS_KEY, extras) }
+        val intent = Intent().apply {
+            putExtra(DEEPLINK_EXTRAS_KEY, bundleOf(LOCATION_KEY to "https://other.com/path"))
+        }
         activity = Robolectric.buildActivity(TestActivity::class.java, intent).get()
 
         host.activity = activity
         host.ensureDeeplinkStartLocationValid()
 
-        val resultBundle = activity.intent.getBundleExtra(DEEPLINK_EXTRAS_KEY)
-        assertThat(resultBundle?.getString(LOCATION_KEY)).isEqualTo("https://example.com/start")
+        assertThat(activity.intent.getBundleExtra(DEEPLINK_EXTRAS_KEY)?.getString(LOCATION_KEY))
+            .isEqualTo("https://example.com/start")
     }
 
     @Test
     fun `does not change start location when deep link host matches config`() {
-        val extras = bundleOf(LOCATION_KEY to "https://example.com/path")
-        val intent = Intent().apply { putExtra(DEEPLINK_EXTRAS_KEY, extras) }
+        val intent = Intent().apply {
+            putExtra(DEEPLINK_EXTRAS_KEY, bundleOf(LOCATION_KEY to "https://example.com/path"))
+        }
         activity = Robolectric.buildActivity(TestActivity::class.java, intent).get()
 
         host.activity = activity
         host.ensureDeeplinkStartLocationValid()
 
-        val resultBundle = activity.intent.getBundleExtra(DEEPLINK_EXTRAS_KEY)
-        assertThat(resultBundle?.getString(LOCATION_KEY)).isEqualTo("https://example.com/path")
+        assertThat(activity.intent.getBundleExtra(DEEPLINK_EXTRAS_KEY)?.getString(LOCATION_KEY))
+            .isEqualTo("https://example.com/path")
+    }
+
+    // NavController merges deepLinkArgs over deepLinkExtras (last write wins); the intent's args
+    // must not survive to override the validated start location.
+    @Test
+    fun `empties deepLinkArgs so they cannot override the start location`() {
+        val intent = Intent().apply {
+            putExtra(DEEPLINK_EXTRAS_KEY, bundleOf(LOCATION_KEY to "https://example.com/ok"))
+            putParcelableArrayListExtra(DEEPLINK_ARGS_KEY, arrayListOf(bundleOf(LOCATION_KEY to ATTACKER_URL)))
+        }
+        activity = Robolectric.buildActivity(TestActivity::class.java, intent).get()
+
+        host.activity = activity
+        host.ensureDeeplinkStartLocationValid()
+
+        val survivingArgs = activity.intent.getParcelableArrayListExtra<Bundle>(DEEPLINK_ARGS_KEY)
+            ?.mapNotNull { it.getString(LOCATION_KEY) }.orEmpty()
+        assertThat(survivingArgs).doesNotContain(ATTACKER_URL)
+    }
+
+    companion object {
+        private const val ATTACKER_URL = "https://attacker.example/steal"
     }
 
     private class TestActivity : HotwireActivity() {