From 12f0bd51c5707013cf368548dfbe0e465e9851be Mon Sep 17 00:00:00 2001 From: Evgeny Date: Sun, 10 Aug 2025 12:38:34 +0100 Subject: [PATCH] core, ui: improve removing tracking parameters (#6170) * core: make URI sanitizer less zealous * simplify * subdomains * update rule * make "remove link tracking" option disabled by default --- .../Views/Chat/ChatItem/CILinkView.swift | 2 +- .../Chat/ComposeMessage/ComposeView.swift | 2 +- .../Views/UserSettings/PrivacySettings.swift | 2 +- apps/ios/SimpleXChat/AppGroup.swift | 2 +- .../chat/simplex/common/model/SimpleXAPI.kt | 2 +- .../simplex/common/views/chat/ComposeView.kt | 4 +- .../common/views/chat/item/TextItemView.kt | 2 +- simplex-chat.cabal | 1 + src/Simplex/Chat/Markdown.hs | 49 +++++++++++++++++-- tests/MarkdownTests.hs | 25 ++++++++++ 10 files changed, 81 insertions(+), 10 deletions(-) diff --git a/apps/ios/Shared/Views/Chat/ChatItem/CILinkView.swift b/apps/ios/Shared/Views/Chat/ChatItem/CILinkView.swift index b9bf6dd63a..680986acfe 100644 --- a/apps/ios/Shared/Views/Chat/ChatItem/CILinkView.swift +++ b/apps/ios/Shared/Views/Chat/ChatItem/CILinkView.swift @@ -57,7 +57,7 @@ func openBrowserAlert(uri: String) { cancelAlertAction, UIAlertAction( title: NSLocalizedString("Open full link", comment: "alert action"), - style: .destructive, + style: .default, handler: { _ in UIApplication.shared.open(url.uri) } ), UIAlertAction( diff --git a/apps/ios/Shared/Views/Chat/ComposeMessage/ComposeView.swift b/apps/ios/Shared/Views/Chat/ComposeMessage/ComposeView.swift index adaa2edad4..ac5e3b2cd7 100644 --- a/apps/ios/Shared/Views/Chat/ComposeMessage/ComposeView.swift +++ b/apps/ios/Shared/Views/Chat/ComposeMessage/ComposeView.swift @@ -353,7 +353,7 @@ struct ComposeView: View { @UserDefault(DEFAULT_PRIVACY_SAVE_LAST_DRAFT) private var saveLastDraft = true @UserDefault(DEFAULT_TOOLBAR_MATERIAL) private var toolbarMaterial = ToolbarMaterial.defaultMaterial @AppStorage(GROUP_DEFAULT_INCOGNITO, store: groupDefaults) private var incognitoDefault = false - @AppStorage(GROUP_DEFAULT_PRIVACY_SANITIZE_LINKS, store: groupDefaults) private var privacySanitizeLinks = true + @AppStorage(GROUP_DEFAULT_PRIVACY_SANITIZE_LINKS, store: groupDefaults) private var privacySanitizeLinks = false @State private var updatingCompose = false var body: some View { diff --git a/apps/ios/Shared/Views/UserSettings/PrivacySettings.swift b/apps/ios/Shared/Views/UserSettings/PrivacySettings.swift index 48c2efe0ac..eec820833c 100644 --- a/apps/ios/Shared/Views/UserSettings/PrivacySettings.swift +++ b/apps/ios/Shared/Views/UserSettings/PrivacySettings.swift @@ -14,7 +14,7 @@ struct PrivacySettings: View { @EnvironmentObject var theme: AppTheme @AppStorage(DEFAULT_PRIVACY_ACCEPT_IMAGES) private var autoAcceptImages = true @AppStorage(DEFAULT_PRIVACY_LINK_PREVIEWS) private var useLinkPreviews = true - @AppStorage(GROUP_DEFAULT_PRIVACY_SANITIZE_LINKS, store: groupDefaults) private var privacySanitizeLinks = true + @AppStorage(GROUP_DEFAULT_PRIVACY_SANITIZE_LINKS, store: groupDefaults) private var privacySanitizeLinks = false @AppStorage(DEFAULT_PRIVACY_SHOW_CHAT_PREVIEWS) private var showChatPreviews = true @AppStorage(DEFAULT_PRIVACY_SAVE_LAST_DRAFT) private var saveLastDraft = true @AppStorage(GROUP_DEFAULT_PRIVACY_ENCRYPT_LOCAL_FILES, store: groupDefaults) private var encryptLocalFiles = true diff --git a/apps/ios/SimpleXChat/AppGroup.swift b/apps/ios/SimpleXChat/AppGroup.swift index 47932397fc..77fff873ea 100644 --- a/apps/ios/SimpleXChat/AppGroup.swift +++ b/apps/ios/SimpleXChat/AppGroup.swift @@ -98,7 +98,7 @@ public func registerGroupDefaults() { GROUP_DEFAULT_ALLOW_SHARE_EXTENSION: false, GROUP_DEFAULT_PRIVACY_LINK_PREVIEWS: true, GROUP_DEFAULT_PRIVACY_LINK_PREVIEWS_SHOW_ALERT: true, - GROUP_DEFAULT_PRIVACY_SANITIZE_LINKS: true, + GROUP_DEFAULT_PRIVACY_SANITIZE_LINKS: false, GROUP_DEFAULT_PRIVACY_ACCEPT_IMAGES: true, GROUP_DEFAULT_PRIVACY_TRANSFER_IMAGES_INLINE: false, GROUP_DEFAULT_PRIVACY_ENCRYPT_LOCAL_FILES: true, diff --git a/apps/multiplatform/common/src/commonMain/kotlin/chat/simplex/common/model/SimpleXAPI.kt b/apps/multiplatform/common/src/commonMain/kotlin/chat/simplex/common/model/SimpleXAPI.kt index 3e56ef3a2e..93cae0b787 100644 --- a/apps/multiplatform/common/src/commonMain/kotlin/chat/simplex/common/model/SimpleXAPI.kt +++ b/apps/multiplatform/common/src/commonMain/kotlin/chat/simplex/common/model/SimpleXAPI.kt @@ -105,7 +105,7 @@ class AppPreferences { val privacyAcceptImages = mkBoolPreference(SHARED_PREFS_PRIVACY_ACCEPT_IMAGES, true) val privacyLinkPreviews = mkBoolPreference(SHARED_PREFS_PRIVACY_LINK_PREVIEWS, true) val privacyLinkPreviewsShowAlert = mkBoolPreference(SHARED_PREFS_PRIVACY_LINK_PREVIEWS_SHOW_ALERT, true) - val privacySanitizeLinks = mkBoolPreference(SHARED_PREFS_PRIVACY_SANITIZE_LINKS, true) + val privacySanitizeLinks = mkBoolPreference(SHARED_PREFS_PRIVACY_SANITIZE_LINKS, false) // TODO remove val privacyChatListOpenLinks = mkEnumPreference(SHARED_PREFS_PRIVACY_CHAT_LIST_OPEN_LINKS, PrivacyChatListOpenLinksMode.ASK) { PrivacyChatListOpenLinksMode.values().firstOrNull { it.name == this } } val simplexLinkMode: SharedPreference = mkSafeEnumPreference(SHARED_PREFS_PRIVACY_SIMPLEX_LINK_MODE, SimplexLinkMode.default) diff --git a/apps/multiplatform/common/src/commonMain/kotlin/chat/simplex/common/views/chat/ComposeView.kt b/apps/multiplatform/common/src/commonMain/kotlin/chat/simplex/common/views/chat/ComposeView.kt index c575efcb35..5746c76f18 100644 --- a/apps/multiplatform/common/src/commonMain/kotlin/chat/simplex/common/views/chat/ComposeView.kt +++ b/apps/multiplatform/common/src/commonMain/kotlin/chat/simplex/common/views/chat/ComposeView.kt @@ -43,6 +43,7 @@ import kotlinx.serialization.encoding.Encoder import java.io.File import java.net.URI import java.nio.file.Files +import kotlin.math.min const val MAX_NUMBER_OF_MENTIONS = 3 @@ -907,7 +908,8 @@ fun ComposeView( if (sanitizedPos == null) { composeState.value = composeState.value.copy(message = s, parsedMessage = parsedMessage) } else { - val message = if (sanitizedPos < s.selection.start) s.copy(text = updatedMsg) else ComposeMessage(updatedMsg, TextRange(sanitizedPos, sanitizedPos)) + val pos = min(updatedMsg.count(), if (sanitizedPos < s.selection.start) s.selection.start else sanitizedPos) + val message = s.copy(text = updatedMsg, selection = TextRange(pos)) composeState.value = composeState.value.copy(message = message, parsedMessage = updatedParsedMsg) parsedMessage = updatedParsedMsg } diff --git a/apps/multiplatform/common/src/commonMain/kotlin/chat/simplex/common/views/chat/item/TextItemView.kt b/apps/multiplatform/common/src/commonMain/kotlin/chat/simplex/common/views/chat/item/TextItemView.kt index 6a6485bc42..075946c008 100644 --- a/apps/multiplatform/common/src/commonMain/kotlin/chat/simplex/common/views/chat/item/TextItemView.kt +++ b/apps/multiplatform/common/src/commonMain/kotlin/chat/simplex/common/views/chat/item/TextItemView.kt @@ -391,7 +391,7 @@ fun openBrowserAlert(uri: String, uriHandler: UriHandler) { AlertManager.shared.hideAlert() safeOpenUri(uri, uriHandler) }) { - Text(generalGetString(MR.strings.privacy_chat_list_open_full_web_link), Modifier.fillMaxWidth(), textAlign = TextAlign.Center, color = MaterialTheme.colors.error) + Text(generalGetString(MR.strings.privacy_chat_list_open_full_web_link), Modifier.fillMaxWidth(), textAlign = TextAlign.Center, color = MaterialTheme.colors.primary) } SectionItemView({ AlertManager.shared.hideAlert() diff --git a/simplex-chat.cabal b/simplex-chat.cabal index 5d7e69cbf2..22a7da2ccf 100644 --- a/simplex-chat.cabal +++ b/simplex-chat.cabal @@ -587,6 +587,7 @@ test-suite simplex-chat-test , time ==1.12.* , unicode-transforms ==0.4.* , unliftio ==0.2.* + , uri-bytestring >=0.3.3.1 && <0.4 default-language: Haskell2010 if flag(client_postgres) build-depends: diff --git a/src/Simplex/Chat/Markdown.hs b/src/Simplex/Chat/Markdown.hs index 19e80e4339..8f34b740d7 100644 --- a/src/Simplex/Chat/Markdown.hs +++ b/src/Simplex/Chat/Markdown.hs @@ -20,6 +20,7 @@ import qualified Data.Aeson as J import qualified Data.Aeson.TH as JQ import Data.Attoparsec.Text (Parser) import qualified Data.Attoparsec.Text as A +import Data.ByteString.Char8 (ByteString) import qualified Data.ByteString.Char8 as B import Data.Char (isAlpha, isAscii, isDigit, isPunctuation, isSpace) import Data.Either (fromRight) @@ -328,7 +329,7 @@ markdownP = mconcat <$> A.many' fragmentP strEncodeText :: StrEncoding a => a -> Text strEncodeText = safeDecodeUtf8 . strEncode -parseUri :: B.ByteString -> Either Text U.URI +parseUri :: ByteString -> Either Text U.URI parseUri s = case U.parseURI U.laxURIParserOptions s of Left e -> Left $ "Invalid URI: " <> tshow e Right uri@U.URI {uriScheme = U.Scheme sch, uriAuthority} @@ -339,12 +340,54 @@ parseUri s = case U.parseURI U.laxURIParserOptions s of | '.' `B.notElem` h -> Left $ "Invalid URI host: " <> safeDecodeUtf8 h | otherwise -> Right uri +-- the heuristic for removing tracking parameters is the following: +-- 1) if the URI path looks like page name* rather than an identifier, allow the first parameter, as long as it is not blacklisted, +-- 2) also allow whitelisted parameters, +-- 3) remove all other parameters. +-- *page name: lowercase latin in snake-case or hyphen-case, allowing for sinlge leading or trailing hyphen or underscore. sanitizeUri :: U.URI -> Maybe U.URI -sanitizeUri uri@U.URI {uriQuery = U.Query originalQS} = - let sanitizedQS = filter (\(p, _) -> p == "q" || p == "search") originalQS +sanitizeUri uri@U.URI {uriAuthority, uriPath, uriQuery = U.Query originalQS} = + let sanitizedQS + | isNamePath = case originalQS of + p : ps -> (if isBlacklisted (fst p) then id else (p :)) $ filter (isWhitelisted . fst) ps + [] -> [] + | otherwise = filter (isWhitelisted . fst) originalQS in if length sanitizedQS == length originalQS then Nothing else Just $ uri {U.uriQuery = U.Query sanitizedQS} + where + isBlacklisted p = any ($ p) qsBlacklist + isWhitelisted p = any (\(f, ps) -> f host && p `elem` ps) qsWhitelist + host = maybe "" (\U.Authority {authorityHost = U.Host h} -> h) uriAuthority + isNamePath = B.all (\c -> (c >= 'a' && c <= 'z') || c == '_' || c == '-' || c == '/') uriPath + qsWhitelist :: [(ByteString -> Bool, [ByteString])] + qsWhitelist = + [ (const True, ["q", "search"]), + (dom "youtube.com", ["v", "t"]), + (dom "youtu.be", ["t"]) + ] + dom d h = d == h || (('.' `B.cons` d) `B.isSuffixOf` h) + qsBlacklist :: [ByteString -> Bool] + qsBlacklist = + [ (B.any (== '_')), + ("ad" `B.isPrefixOf`), + ("af" `B.isPrefixOf`), + ("dc" `B.isPrefixOf`), + ("fb" `B.isPrefixOf`), + ("gc" `B.isPrefixOf`), + ("li" `B.isPrefixOf`), + ("ref" `B.isPrefixOf`), + ("si" `B.isPrefixOf`), + ("tw" `B.isPrefixOf`), + ("utm" `B.isPrefixOf`), + ("camp" `B.isInfixOf`), + ("cmp" `B.isInfixOf`), + ("dev" `B.isInfixOf`), + ("id" `B.isInfixOf`), + ("prom" `B.isInfixOf`), + ("source" `B.isInfixOf`), + ("src" `B.isInfixOf`) + ] markdownText :: FormattedText -> Text markdownText (FormattedText f_ t) = case f_ of diff --git a/tests/MarkdownTests.hs b/tests/MarkdownTests.hs index d6a14391b0..e246a7a989 100644 --- a/tests/MarkdownTests.hs +++ b/tests/MarkdownTests.hs @@ -11,8 +11,10 @@ import qualified Data.Text as T import Data.Text.Encoding (encodeUtf8) import Simplex.Chat.Markdown import Simplex.Messaging.Encoding.String +import Simplex.Messaging.Util ((<$$>)) import System.Console.ANSI.Types import Test.Hspec +import qualified URI.ByteString as U markdownTests :: Spec markdownTests = do @@ -26,6 +28,7 @@ markdownTests = do textWithMentions textWithCommands multilineMarkdownList + testSanitizeUri infixr 1 ==>, <==, <==>, ==>>, <<==, <<==>> @@ -359,3 +362,25 @@ multilineMarkdownList = describe "multiline markdown" do it "command markdown" do "/link 1" <<==>> [command' "link 1" "/link 1"] " /link 1" <<==>> [command' "link 1" " /link 1"] + +testSanitizeUri :: Spec +testSanitizeUri = describe "sanitizeUri" $ do + it "should allow the first parameter and whitelisted parameters on pages without IDs" $ do + "https://example.com/page?ref=123" `sanitized` Just "https://example.com/page" + "https://example.com/page?name" `sanitized` Nothing + "https://example.com/page?name=abc" `sanitized` Nothing + "https://example.com/page?name=abc&ref=123" `sanitized` Just "https://example.com/page?name=abc" + "https://example.com/page?search=query" `sanitized` Nothing + "https://example.com/page?q=query" `sanitized` Nothing + "https://example.com/page?ref=123&q=query" `sanitized` Just "https://example.com/page?q=query" + "https://youtube.com/watch?v=abc&t=123" `sanitized` Nothing + "https://www.youtube.com/watch?v=abc" `sanitized` Nothing + "https://www.youtube.com/watch?v=abc&t=123" `sanitized` Nothing + "https://www.youtube.com/watch?ref=456&v=abc&t=123" `sanitized` Just "https://www.youtube.com/watch?v=abc&t=123" + it "should only allow whitelisted parameters if path contains IDs" $ do + "https://example.com/page/a123?name=abc" `sanitized` Just "https://example.com/page/a123" + "https://youtu.be/a123?si=456" `sanitized` Just "https://youtu.be/a123" + "https://youtu.be/a123?t=456" `sanitized` Nothing + "https://youtu.be/a123?si=456&t=789" `sanitized` Just "https://youtu.be/a123?t=789" + where + s `sanitized` res = (U.serializeURIRef' <$$> (sanitizeUri <$> parseUri s)) `shouldBe` Right res