From 91f73b473f19e483157457e6a0fc31406f818321 Mon Sep 17 00:00:00 2001 From: Alex Hart Date: Fri, 17 Apr 2026 16:08:53 -0300 Subject: [PATCH] Sanitize donations webview intents. Co-authored-by: Greyson Parrelli --- .../donate/stripe/ExternalNavigationHelper.kt | 49 +++++++- app/src/main/res/values/strings.xml | 2 + .../stripe/ExternalNavigationHelperTest.kt | 113 ++++++++++++++++++ 3 files changed, 159 insertions(+), 5 deletions(-) create mode 100644 app/src/test/java/org/thoughtcrime/securesms/components/settings/app/subscription/donate/stripe/ExternalNavigationHelperTest.kt diff --git a/app/src/main/java/org/thoughtcrime/securesms/components/settings/app/subscription/donate/stripe/ExternalNavigationHelper.kt b/app/src/main/java/org/thoughtcrime/securesms/components/settings/app/subscription/donate/stripe/ExternalNavigationHelper.kt index dd9397aa61..b0e50ed0d7 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/components/settings/app/subscription/donate/stripe/ExternalNavigationHelper.kt +++ b/app/src/main/java/org/thoughtcrime/securesms/components/settings/app/subscription/donate/stripe/ExternalNavigationHelper.kt @@ -10,11 +10,13 @@ import android.content.Context import android.content.Intent import android.net.Uri import android.widget.Toast +import androidx.annotation.VisibleForTesting import com.google.android.material.dialog.MaterialAlertDialogBuilder import org.signal.core.util.isNotNullOrBlank import org.signal.core.util.logging.Log import org.signal.donations.StripeApi import org.thoughtcrime.securesms.R +import java.net.URISyntaxException /** * Encapsulates the logic for navigating a user to a deeplink from within a webview or parsing out the fallback @@ -30,18 +32,31 @@ object ExternalNavigationHelper { return false } + val intent = try { + Intent.parseUri(url.toString(), Intent.URI_INTENT_SCHEME).sanitizeWebIntent() + } catch (e: URISyntaxException) { + Log.w(TAG, "Failed to parse web intent URI.", e) + return false + } + + val targetLabel = resolveTargetLabel(context, intent) + val message = if (targetLabel != null) { + context.getString(R.string.ExternalNavigationHelper__once_payment_confirmed_in_app, targetLabel) + } else { + context.getString(R.string.ExternalNavigationHelper__once_this_payment_is_confirmed) + } + MaterialAlertDialogBuilder(context) .setTitle(R.string.ExternalNavigationHelper__leave_signal_to_confirm_payment) - .setMessage(R.string.ExternalNavigationHelper__once_this_payment_is_confirmed) - .setPositiveButton(android.R.string.ok) { _, _ -> attemptIntentLaunch(context, url, launchIntent) } + .setMessage(message) + .setPositiveButton(android.R.string.ok) { _, _ -> attemptIntentLaunch(context, intent, launchIntent) } .setNegativeButton(android.R.string.cancel, null) .show() return true } - private fun attemptIntentLaunch(context: Context, url: Uri, launchIntent: (Intent) -> Unit) { - val intent = Intent.parseUri(url.toString(), Intent.URI_INTENT_SCHEME) + private fun attemptIntentLaunch(context: Context, intent: Intent, launchIntent: (Intent) -> Unit) { try { launchIntent(intent) } catch (e: ActivityNotFoundException) { @@ -50,7 +65,7 @@ object ExternalNavigationHelper { val fallback = intent.getStringExtra("browser_fallback_url") if (fallback.isNotNullOrBlank()) { try { - launchIntent(Intent.parseUri(fallback, Intent.URI_INTENT_SCHEME)) + launchIntent(Intent.parseUri(fallback, Intent.URI_INTENT_SCHEME).sanitizeWebIntent()) } catch (e: ActivityNotFoundException) { Log.w(TAG, "Failed to launch fallback URL.", e) toastOnActivityNotFound(context) @@ -59,6 +74,30 @@ object ExternalNavigationHelper { } } + private fun resolveTargetLabel(context: Context, intent: Intent): CharSequence? { + val resolveInfo = context.packageManager.resolveActivity(intent, 0) ?: return null + return resolveInfo.loadLabel(context.packageManager).toString().takeIf { it.isNotBlank() } + } + + /** + * Sanitize an intent parsed from a web-originated URI to prevent targeting + * non-exported or internal activities. This mirrors the sanitization that + * browsers apply to intent:// URIs before dispatching them. + */ + @VisibleForTesting + fun Intent.sanitizeWebIntent(): Intent { + component = null + selector = null + addCategory(Intent.CATEGORY_BROWSABLE) + flags = flags and ( + Intent.FLAG_GRANT_READ_URI_PERMISSION or + Intent.FLAG_GRANT_WRITE_URI_PERMISSION or + Intent.FLAG_GRANT_PERSISTABLE_URI_PERMISSION or + Intent.FLAG_GRANT_PREFIX_URI_PERMISSION + ).inv() + return this + } + private fun toastOnActivityNotFound(context: Context) { Toast.makeText(context, R.string.CommunicationActions_no_browser_found, Toast.LENGTH_SHORT).show() } diff --git a/app/src/main/res/values/strings.xml b/app/src/main/res/values/strings.xml index d571901b8d..384a8101ed 100644 --- a/app/src/main/res/values/strings.xml +++ b/app/src/main/res/values/strings.xml @@ -7610,6 +7610,8 @@ Leave Signal to confirm donation? When confirmed, return to Signal to finish processing your donation. + + You\'ll be sent to %1$s. When confirmed, return to Signal to finish processing your donation. diff --git a/app/src/test/java/org/thoughtcrime/securesms/components/settings/app/subscription/donate/stripe/ExternalNavigationHelperTest.kt b/app/src/test/java/org/thoughtcrime/securesms/components/settings/app/subscription/donate/stripe/ExternalNavigationHelperTest.kt new file mode 100644 index 0000000000..7a5d8f16cc --- /dev/null +++ b/app/src/test/java/org/thoughtcrime/securesms/components/settings/app/subscription/donate/stripe/ExternalNavigationHelperTest.kt @@ -0,0 +1,113 @@ +package org.thoughtcrime.securesms.components.settings.app.subscription.donate.stripe + +import android.app.Application +import android.content.ComponentName +import android.content.Intent +import org.junit.Assert.assertEquals +import org.junit.Assert.assertFalse +import org.junit.Assert.assertNotNull +import org.junit.Assert.assertNull +import org.junit.Assert.assertTrue +import org.junit.Test +import org.junit.runner.RunWith +import org.robolectric.RobolectricTestRunner +import org.robolectric.annotation.Config + +@RunWith(RobolectricTestRunner::class) +@Config(manifest = Config.NONE, application = Application::class) +class ExternalNavigationHelperTest { + + @Test + fun `sanitizeWebIntent clears explicit component`() { + val intent = Intent().apply { + component = ComponentName("org.thoughtcrime.securesms", "org.thoughtcrime.securesms.FakeInternalActivity") + } + + val sanitized = with(ExternalNavigationHelper) { intent.sanitizeWebIntent() } + + assertNull(sanitized.component) + } + + @Test + fun `sanitizeWebIntent clears selector`() { + val intent = Intent().apply { + selector = Intent(Intent.ACTION_MAIN).addCategory(Intent.CATEGORY_APP_BROWSER) + } + + val sanitized = with(ExternalNavigationHelper) { intent.sanitizeWebIntent() } + + assertNull(sanitized.selector) + } + + @Test + fun `sanitizeWebIntent adds CATEGORY_BROWSABLE`() { + val intent = Intent() + + val sanitized = with(ExternalNavigationHelper) { intent.sanitizeWebIntent() } + + assertTrue(sanitized.hasCategory(Intent.CATEGORY_BROWSABLE)) + } + + @Test + fun `sanitizeWebIntent strips URI permission grant flags`() { + val intent = Intent().apply { + flags = Intent.FLAG_GRANT_READ_URI_PERMISSION or + Intent.FLAG_GRANT_WRITE_URI_PERMISSION or + Intent.FLAG_GRANT_PERSISTABLE_URI_PERMISSION or + Intent.FLAG_GRANT_PREFIX_URI_PERMISSION + } + + val sanitized = with(ExternalNavigationHelper) { intent.sanitizeWebIntent() } + + assertFalse(sanitized.flags and Intent.FLAG_GRANT_READ_URI_PERMISSION != 0) + assertFalse(sanitized.flags and Intent.FLAG_GRANT_WRITE_URI_PERMISSION != 0) + assertFalse(sanitized.flags and Intent.FLAG_GRANT_PERSISTABLE_URI_PERMISSION != 0) + assertFalse(sanitized.flags and Intent.FLAG_GRANT_PREFIX_URI_PERMISSION != 0) + } + + @Test + fun `sanitizeWebIntent preserves unrelated flags`() { + val intent = Intent().apply { + flags = Intent.FLAG_ACTIVITY_NEW_TASK or Intent.FLAG_GRANT_READ_URI_PERMISSION + } + + val sanitized = with(ExternalNavigationHelper) { intent.sanitizeWebIntent() } + + assertTrue(sanitized.flags and Intent.FLAG_ACTIVITY_NEW_TASK != 0) + assertFalse(sanitized.flags and Intent.FLAG_GRANT_READ_URI_PERMISSION != 0) + } + + @Test + fun `sanitizeWebIntent preserves browser_fallback_url extra`() { + val intent = Intent().apply { + putExtra("browser_fallback_url", "https://example.com/fallback") + } + + val sanitized = with(ExternalNavigationHelper) { intent.sanitizeWebIntent() } + + assertEquals("https://example.com/fallback", sanitized.getStringExtra("browser_fallback_url")) + } + + @Test + fun `parsed web intent URI loses explicit component after sanitization`() { + val uri = "intent://x#Intent;component=org.thoughtcrime.securesms/.sharing.v2.ShareActivity;action=android.intent.action.SEND;end" + val parsed = Intent.parseUri(uri, Intent.URI_INTENT_SCHEME) + assertNotNull("Test precondition: parsed URI should set the component", parsed.component) + + val sanitized = with(ExternalNavigationHelper) { parsed.sanitizeWebIntent() } + + assertNull(sanitized.component) + assertTrue(sanitized.hasCategory(Intent.CATEGORY_BROWSABLE)) + } + + @Test + fun `parsed web intent URI loses selector after sanitization`() { + val uri = "intent://x#Intent;action=android.intent.action.VIEW;SEL;scheme=https;end" + val parsed = Intent.parseUri(uri, Intent.URI_INTENT_SCHEME) + assertNotNull("Test precondition: parsed URI should set a selector", parsed.selector) + + val sanitized = with(ExternalNavigationHelper) { parsed.sanitizeWebIntent() } + + assertNull(sanitized.selector) + } +}