Merge pull request #6164 from vector-im/bug/adm/link-checking

Ask the user to confirm urls which contain unicode direction overrides

changelog.d/6163.feature

@@ -0,0 +1 @@
+Security - Asking for user confirmation when tapping URLs which contain unicode directional overrides

vector/src/main/java/im/vector/app/core/extensions/String.kt

@@ -0,0 +1,26 @@
+/*
+ * Copyright (c) 2022 New Vector Ltd
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package im.vector.app.core.extensions
+
+private const val RTL_OVERRIDE_CHAR = '\u202E'
+private const val LTR_OVERRIDE_CHAR = '\u202D'
+
+fun String.ensureEndsLeftToRight() = if (containsRtLOverride()) "$this$LTR_OVERRIDE_CHAR" else this
+
+fun String.containsRtLOverride() = contains(RTL_OVERRIDE_CHAR)
+
+fun String.filterDirectionOverrides() = filterNot { it == RTL_OVERRIDE_CHAR || it == LTR_OVERRIDE_CHAR }

vector/src/main/java/im/vector/app/features/home/room/detail/TimelineFragment.kt

@@ -74,6 +74,9 @@ import im.vector.app.core.dialogs.ConfirmationDialogBuilder
 import im.vector.app.core.dialogs.GalleryOrCameraDialogHelper
 import im.vector.app.core.epoxy.LayoutManagerStateRestorer
 import im.vector.app.core.extensions.cleanup
+import im.vector.app.core.extensions.containsRtLOverride
+import im.vector.app.core.extensions.ensureEndsLeftToRight
+import im.vector.app.core.extensions.filterDirectionOverrides
 import im.vector.app.core.extensions.hideKeyboard
 import im.vector.app.core.extensions.registerStartForActivityResult
 import im.vector.app.core.extensions.setTextOrHide
@@ -1922,23 +1925,20 @@ class TimelineFragment @Inject constructor(
                         }
                     })
             if (!isManaged) {
-                if (title.isValidUrl() && url.isValidUrl() && URL(title).host != URL(url).host) {
-                    MaterialAlertDialogBuilder(requireActivity(), R.style.ThemeOverlay_Vector_MaterialAlertDialog_NegativeDestructive)
-                            .setTitle(R.string.external_link_confirmation_title)
-                            .setMessage(
-                                    getString(R.string.external_link_confirmation_message, title, url)
-                                            .toSpannable()
-                                            .colorizeMatchingText(url, colorProvider.getColorFromAttribute(R.attr.vctr_content_tertiary))
-                                            .colorizeMatchingText(title, colorProvider.getColorFromAttribute(R.attr.vctr_content_tertiary))
+                when {
+                    url.containsRtLOverride()                                                  -> {
+                        displayUrlConfirmationDialog(
+                                seenUrl = title.ensureEndsLeftToRight(),
+                                actualUrl = url.filterDirectionOverrides(),
+                                continueTo = url
                         )
-                            .setPositiveButton(R.string._continue) { _, _ ->
+                    }
+                    title.isValidUrl() && url.isValidUrl() && URL(title).host != URL(url).host -> {
+                        displayUrlConfirmationDialog(title, url)
+                    }
+                    else                                                                       -> {
                         openUrlInExternalBrowser(requireContext(), url)
                     }
-                            .setNegativeButton(R.string.action_cancel, null)
-                            .show()
-                } else {
-                    // Open in external browser, in a new Tab
-                    openUrlInExternalBrowser(requireContext(), url)
                 }
             }
         }
@@ -1946,6 +1946,22 @@ class TimelineFragment @Inject constructor(
         return true
     }
 
+    private fun displayUrlConfirmationDialog(seenUrl: String, actualUrl: String, continueTo: String = actualUrl) {
+        MaterialAlertDialogBuilder(requireActivity(), R.style.ThemeOverlay_Vector_MaterialAlertDialog_NegativeDestructive)
+                .setTitle(R.string.external_link_confirmation_title)
+                .setMessage(
+                        getString(R.string.external_link_confirmation_message, seenUrl, actualUrl)
+                                .toSpannable()
+                                .colorizeMatchingText(actualUrl, colorProvider.getColorFromAttribute(R.attr.vctr_content_tertiary))
+                                .colorizeMatchingText(seenUrl, colorProvider.getColorFromAttribute(R.attr.vctr_content_tertiary))
+                )
+                .setPositiveButton(R.string._continue) { _, _ ->
+                    openUrlInExternalBrowser(requireContext(), continueTo)
+                }
+                .setNegativeButton(R.string.action_cancel, null)
+                .show()
+    }
+
     override fun onUrlLongClicked(url: String): Boolean {
         if (url != getString(R.string.edited_suffix) && url.isValidUrl()) {
             // Copy the url to the clipboard

vector/src/test/java/im/vector/app/core/extensions/StringExtensionsTest.kt

@@ -0,0 +1,59 @@
+/*
+ * Copyright (c) 2022 New Vector Ltd
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package im.vector.app.core.extensions
+
+import org.amshove.kluent.shouldBeEqualTo
+import org.junit.Test
+
+class StringExtensionsTest {
+
+    @Test
+    fun `given text with RtL unicode override, when checking contains RtL Override, then returns true`() {
+        val textWithRtlOverride = "hello\u202Eworld"
+
+        val result = textWithRtlOverride.containsRtLOverride()
+
+        result shouldBeEqualTo true
+    }
+
+    @Test
+    fun `given text without RtL unicode override, when checking contains RtL Override, then returns false`() {
+        val textWithRtlOverride = "hello world"
+
+        val result = textWithRtlOverride.containsRtLOverride()
+
+        result shouldBeEqualTo false
+    }
+
+    @Test
+    fun `given text with RtL unicode override, when ensuring ends LtR, then appends a LtR unicode override`() {
+        val textWithRtlOverride = "123\u202E456"
+
+        val result = textWithRtlOverride.ensureEndsLeftToRight()
+
+        result shouldBeEqualTo "$textWithRtlOverride\u202D"
+    }
+
+    @Test
+    fun `given text with unicode direction overrides, when filtering direction overrides, then removes all overrides`() {
+        val textWithDirectionOverrides = "123\u202E456\u202d789"
+
+        val result = textWithDirectionOverrides.filterDirectionOverrides()
+
+        result shouldBeEqualTo "123456789"
+    }
+}