Merge pull request #154 from hotwired/mb/start_location_vulnerability_fix

Navigation: deeplink start location vulnerability fix

Author: mbarta

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

@@ -2,6 +2,9 @@ package dev.hotwire.navigation.navigator
 
 import android.os.Bundle
 import android.view.View
+import androidx.annotation.VisibleForTesting
+import androidx.annotation.VisibleForTesting.Companion.PROTECTED
+import androidx.core.net.toUri
 import androidx.fragment.app.Fragment
 import androidx.fragment.app.FragmentManager
 import androidx.fragment.app.FragmentOnAttachListener
@@ -11,6 +14,9 @@ import dev.hotwire.core.config.Hotwire
 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 LOCATION_KEY = "location"
+
 open class NavigatorHost : NavHostFragment(), FragmentOnAttachListener {
     internal lateinit var activity: HotwireActivity
     lateinit var navigator: Navigator
@@ -51,6 +57,8 @@ open class NavigatorHost : NavHostFragment(), FragmentOnAttachListener {
     }
 
     internal fun initControllerGraph() {
+        ensureDeeplinkStartLocationValid()
+
         navController.apply {
             graph = NavigatorGraphBuilder(
                 navigatorName = configuration.name,
@@ -63,6 +71,26 @@ 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.
+     */
+    @VisibleForTesting(otherwise = PROTECTED)
+    fun ensureDeeplinkStartLocationValid() {
+        val extrasBundle = activity.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)
+        }
+    }
+
     private val configuration get() = activity.navigatorConfigurations().firstOrNull {
         id == it.navigatorHostId
     } ?: throw IllegalStateException("No configuration found for NavigatorHost")

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

@@ -0,0 +1,61 @@
+package dev.hotwire.navigation.navigator
+
+import android.R.attr.host
+import android.content.Intent
+import android.os.Bundle
+import androidx.core.os.bundleOf
+import dev.hotwire.navigation.activities.HotwireActivity
+import org.assertj.core.api.Assertions.assertThat
+import org.junit.Before
+import org.junit.Test
+import org.junit.runner.RunWith
+import org.robolectric.Robolectric
+import org.robolectric.RobolectricTestRunner
+
+@RunWith(RobolectricTestRunner::class)
+class NavigatorHostTest {
+
+    private lateinit var activity: TestActivity
+    private lateinit var host: NavigatorHost
+
+    @Before
+    fun setUp() {
+        host = NavigatorHost()
+    }
+
+    @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) }
+        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")
+    }
+
+    @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) }
+        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")
+    }
+
+    private class TestActivity : HotwireActivity() {
+        private val navConfig = NavigatorConfiguration(
+            name = "test",
+            startLocation = "https://example.com/start",
+            navigatorHostId = 0
+        )
+
+        override fun navigatorConfigurations(): List<NavigatorConfiguration> = listOf(navConfig)
+    }
+}