diff --git a/turbo/src/main/kotlin/dev/hotwire/turbo/session/TurboSessionNavHostFragment.kt b/turbo/src/main/kotlin/dev/hotwire/turbo/session/TurboSessionNavHostFragment.kt index be1a5abe..0c36c724 100644 --- a/turbo/src/main/kotlin/dev/hotwire/turbo/session/TurboSessionNavHostFragment.kt +++ b/turbo/src/main/kotlin/dev/hotwire/turbo/session/TurboSessionNavHostFragment.kt @@ -17,6 +17,7 @@ import dev.hotwire.turbo.views.TurboWebView import kotlin.reflect.KClass 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" abstract class TurboSessionNavHostFragment : NavHostFragment() { @@ -114,14 +115,22 @@ abstract class TurboSessionNavHostFragment : NavHostFragment() { ?: throw IllegalStateException("No current destination found in NavHostFragment") /** - * 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 internal fun ensureDeeplinkStartLocationValid(activity: FragmentActivity) { - 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(DEEPLINK_ARGS_KEY)?.let { args -> + intent.putParcelableArrayListExtra(DEEPLINK_ARGS_KEY, ArrayList(args.map { Bundle() })) + } + + val extrasBundle = intent.extras?.getBundle(DEEPLINK_EXTRAS_KEY) ?: return val startLocationFromIntent = extrasBundle.getString(LOCATION_KEY) ?: return val deepLinkStartUri = startLocationFromIntent.toUri() @@ -129,7 +138,7 @@ abstract class TurboSessionNavHostFragment : NavHostFragment() { if (deepLinkStartUri.host != configStartUri.host) { extrasBundle.putString(LOCATION_KEY, startLocation) - activity.intent.putExtra(DEEPLINK_EXTRAS_KEY, extrasBundle) + intent.putExtra(DEEPLINK_EXTRAS_KEY, extrasBundle) } } diff --git a/turbo/src/test/kotlin/dev/hotwire/turbo/session/TurboSessionNavHostFragmentTest.kt b/turbo/src/test/kotlin/dev/hotwire/turbo/session/TurboSessionNavHostFragmentTest.kt index 0b6049a2..0ce2b363 100644 --- a/turbo/src/test/kotlin/dev/hotwire/turbo/session/TurboSessionNavHostFragmentTest.kt +++ b/turbo/src/test/kotlin/dev/hotwire/turbo/session/TurboSessionNavHostFragmentTest.kt @@ -2,6 +2,7 @@ package dev.hotwire.turbo.session import android.content.Intent import android.os.Build +import android.os.Bundle import androidx.appcompat.app.AppCompatActivity import androidx.core.os.bundleOf import androidx.fragment.app.Fragment @@ -54,6 +55,28 @@ class TurboSessionNavHostFragmentTest : BaseUnitTest() { assertThat(resultBundle?.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).create().get() + + host = TestNavHostFragment() + host.ensureDeeplinkStartLocationValid(activity) + + val survivingArgs = activity.intent.getParcelableArrayListExtra(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" + } + } class TestActivity : AppCompatActivity()