validate links before opening

Signed-off-by: Andrea Maria Piana <[email protected]>

src/status_im/ui/screens/chat/utils.cljs

@@ -4,6 +4,7 @@
             [status-im.ethereum.stateofus :as stateofus]
             [status-im.utils.gfycat.core :as gfycat]
             [status-im.utils.platform :as platform]
+            [status-im.utils.security :as security]
             [status-im.i18n :as i18n]
             [status-im.utils.core :as core-utils]
             [status-im.ui.components.react :as react]
@@ -35,12 +36,14 @@
                        :color            colors/green}}})
 
 (def ^:private action->prop-fn
-  {:link   (fn [text {:keys [outgoing]}]
+  {:link   (fn [text {:keys [outgoing] :as message}]
              {:style    {:color                (if outgoing colors/white colors/blue)
                          :text-decoration-line :underline}
-              :on-press (if platform/desktop?
-                          #(.openURL (react/linking) (http/normalize-url text))
-                          #(re-frame/dispatch [:browser.ui/message-link-pressed text]))})
+              :on-press #(when (and (security/safe-link? text)
+                                    (security/safe-link-text? (-> message :content :text)))
+                           (if platform/desktop?
+                             (.openURL (react/linking) (http/normalize-url text))
+                             (re-frame/dispatch [:browser.ui/message-link-pressed text])))})
    :tag    (fn [text {:keys [outgoing]}]
              {:style    {:color                (if outgoing colors/white colors/blue)
                          :text-decoration-line :underline}

src/status_im/utils/security.cljs

@@ -22,3 +22,21 @@
   (if (instance? MaskedData data)
     (unmask data)
     data))
+
+;; Links starting with javascript:// should not be handled at all
+(def javascript-link-regex #"javascript://.*")
+;; Anything with rtlo character we don't handle as it might be a spoofed url
+(def rtlo-link-regex #".*\u202e.*")
+
+(defn safe-link?
+  "Check the link is safe to be handled, it is not a javavascript link or contains
+  an rtlo character, which might mean is a spoofed url"
+  [link]
+  (not (or (re-matches javascript-link-regex link)
+           (re-matches rtlo-link-regex link))))
+
+(defn safe-link-text?
+  "Check the text of the message containing a link  is safe to be handled
+  and does not contain an rtlo character, which might mean that the url is spoofed"
+  [text]
+  (not (re-matches rtlo-link-regex text)))

src/status_im/utils/universal_links/core.cljs

@@ -7,6 +7,7 @@
             [status-im.constants :as constants]
             [status-im.ethereum.eip681 :as eip681]
             [status-im.pairing.core :as pairing]
+            [status-im.utils.security :as security]
             [status-im.ui.components.list-selection :as list-selection]
             [status-im.ui.components.react :as react]
             [status-im.ui.screens.add-new.new-chat.db :as new-chat.db]
@@ -53,14 +54,16 @@
 (defn open! [url]
   (log/info "universal-links:  opening " url)
   (if-let [dapp-url (match-url url browse-regex)]
-    (list-selection/browse-dapp dapp-url)
+    (when (security/safe-link? url)
+      (list-selection/browse-dapp dapp-url))
     ;; We need to dispatch here, we can't openURL directly
     ;; as it is opened in safari on iOS
     (re-frame/dispatch [:handle-universal-link url])))
 
 (fx/defn handle-browse [cofx url]
   (log/info "universal-links: handling browse" url)
-  {:browser/show-browser-selection url})
+  (when (security/safe-link? url)
+    {:browser/show-browser-selection url}))
 
 (fx/defn handle-public-chat [cofx public-chat]
   (log/info "universal-links: handling public chat" public-chat)

test/cljs/status_im/test/runner.cljs

@@ -61,6 +61,7 @@
             [status-im.test.utils.money]
             [status-im.test.utils.prices]
             [status-im.test.utils.random]
+            [status-im.test.utils.security]
             [status-im.test.utils.signing-phrase.core]
             [status-im.test.utils.transducers]
             [status-im.test.utils.universal-links.core]
@@ -145,6 +146,7 @@
  'status-im.test.utils.money
  'status-im.test.utils.prices
  'status-im.test.utils.random
+ 'status-im.test.utils.security
  'status-im.test.utils.signing-phrase.core
  'status-im.test.utils.transducers
  'status-im.test.utils.universal-links.core

test/cljs/status_im/test/utils/security.cljs

@@ -0,0 +1,24 @@
+(ns status-im.test.utils.security
+  (:require [cljs.test :refer-macros [deftest is testing]]
+            [status-im.utils.security :as security]))
+
+(def rtlo-link "‮http://google.com")
+(def rtlo-link-text "blah blah ‮  some other blah blah http://google.com blah bash")
+
+(deftest safe-link-test-happy-path
+  (testing "an http link"
+    (is (security/safe-link? "http://test.com")))
+  (testing "an https link"
+    (is (security/safe-link? "https://test.com")))
+  (testing "a link without a a protocol"
+    (is (security/safe-link? "test.com"))))
+
+(deftest safe-link-test-exceptions
+  (testing "a javascript link"
+    (is (not (security/safe-link? "javascript://anything"))))
+  (testing "rtlo links"
+    (is (not (security/safe-link? rtlo-link)))))
+
+(deftest safe-link-text-test-exceptions
+  (testing "rtlo links"
+    (is (not (security/safe-link-text? rtlo-link-text)))))