From 3b52a61bdf1c1302f4e5e5e132f38c7de97cec95 Mon Sep 17 00:00:00 2001 From: Andrea Maria Piana Date: Wed, 28 Aug 2019 09:05:04 +0200 Subject: [PATCH] validate links before opening Signed-off-by: Andrea Maria Piana --- src/status_im/ui/screens/chat/utils.cljs | 11 +++++---- src/status_im/utils/security.cljs | 18 ++++++++++++++ src/status_im/utils/universal_links/core.cljs | 7 ++++-- test/cljs/status_im/test/runner.cljs | 2 ++ test/cljs/status_im/test/utils/security.cljs | 24 +++++++++++++++++++ 5 files changed, 56 insertions(+), 6 deletions(-) create mode 100644 test/cljs/status_im/test/utils/security.cljs diff --git a/src/status_im/ui/screens/chat/utils.cljs b/src/status_im/ui/screens/chat/utils.cljs index 6576265ba0..9d13a71616 100644 --- a/src/status_im/ui/screens/chat/utils.cljs +++ b/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} diff --git a/src/status_im/utils/security.cljs b/src/status_im/utils/security.cljs index c4f809ac2f..897b70a375 100644 --- a/src/status_im/utils/security.cljs +++ b/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))) diff --git a/src/status_im/utils/universal_links/core.cljs b/src/status_im/utils/universal_links/core.cljs index c43af634a0..4d986f08ff 100644 --- a/src/status_im/utils/universal_links/core.cljs +++ b/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) diff --git a/test/cljs/status_im/test/runner.cljs b/test/cljs/status_im/test/runner.cljs index f5a0f4e17f..b8a19fcccc 100644 --- a/test/cljs/status_im/test/runner.cljs +++ b/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 diff --git a/test/cljs/status_im/test/utils/security.cljs b/test/cljs/status_im/test/utils/security.cljs new file mode 100644 index 0000000000..8a28739b36 --- /dev/null +++ b/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)))))