diff --git a/changelog.d/6163.feature b/changelog.d/6163.feature new file mode 100644 index 0000000000..7c64f89b12 --- /dev/null +++ b/changelog.d/6163.feature @@ -0,0 +1 @@ +Security - Asking for user confirmation when tapping URLs which contain unicode directional overrides diff --git a/vector/src/main/java/im/vector/app/core/extensions/String.kt b/vector/src/main/java/im/vector/app/core/extensions/String.kt new file mode 100644 index 0000000000..f035de469c --- /dev/null +++ b/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 } diff --git a/vector/src/main/java/im/vector/app/features/home/room/detail/TimelineFragment.kt b/vector/src/main/java/im/vector/app/features/home/room/detail/TimelineFragment.kt index f0cfff4cf8..3120decd0c 100644 --- a/vector/src/main/java/im/vector/app/features/home/room/detail/TimelineFragment.kt +++ b/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)) - ) - .setPositiveButton(R.string._continue) { _, _ -> - openUrlInExternalBrowser(requireContext(), url) - } - .setNegativeButton(R.string.action_cancel, null) - .show() - } else { - // Open in external browser, in a new Tab - openUrlInExternalBrowser(requireContext(), url) + when { + url.containsRtLOverride() -> { + displayUrlConfirmationDialog( + seenUrl = title.ensureEndsLeftToRight(), + actualUrl = url.filterDirectionOverrides(), + continueTo = url + ) + } + title.isValidUrl() && url.isValidUrl() && URL(title).host != URL(url).host -> { + displayUrlConfirmationDialog(title, url) + } + else -> { + 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 diff --git a/vector/src/test/java/im/vector/app/core/extensions/StringExtensionsTest.kt b/vector/src/test/java/im/vector/app/core/extensions/StringExtensionsTest.kt new file mode 100644 index 0000000000..c4c7a2d38a --- /dev/null +++ b/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" + } +}