From 7264ae2a1440b31fc7b6d70822c920685255b9df Mon Sep 17 00:00:00 2001 From: Roman Volosovskyi Date: Sat, 20 Oct 2018 17:29:11 +0200 Subject: [PATCH] prevent logging of re-frame events parameters mask password received from keychain --- src/status_im/accounts/db.cljs | 5 +++-- src/status_im/accounts/login/core.cljs | 7 ++++--- src/status_im/accounts/recover/core.cljs | 6 +++--- src/status_im/ui/screens/accounts/login/views.cljs | 8 +++++--- src/status_im/ui/screens/wallet/send/events.cljs | 4 ++-- src/status_im/utils/ethereum/erc20.cljs | 2 +- src/status_im/utils/handlers.cljs | 4 +--- src/status_im/utils/keychain/core.cljs | 8 ++++---- src/status_im/utils/security.cljs | 5 +++++ 9 files changed, 28 insertions(+), 21 deletions(-) diff --git a/src/status_im/accounts/db.cljs b/src/status_im/accounts/db.cljs index 0e3ba1e595..13deffd7a6 100644 --- a/src/status_im/accounts/db.cljs +++ b/src/status_im/accounts/db.cljs @@ -5,7 +5,8 @@ status-im.ui.screens.extensions.db [clojure.string :as string] [cljs.spec.alpha :as spec] - [status-im.constants :as const]) + [status-im.constants :as const] + [status-im.utils.security :as security]) (:require-macros [status-im.utils.db :refer [allowed-keys]])) (defn logged-in? [cofx] @@ -19,7 +20,7 @@ (>= (count password) const/min-password-length)) (defn account-creation-next-enabled? [{:keys [step password password-confirm name]}] - (or (and password (= :enter-password step) (spec/valid? ::password password)) + (or (and password (= :enter-password step) (spec/valid? ::password (security/safe-unmask-data password))) (and password-confirm (= :confirm-password step) (spec/valid? ::password password-confirm)) (and name (= :enter-name step) (not (string/blank? name))))) diff --git a/src/status_im/accounts/login/core.cljs b/src/status_im/accounts/login/core.cljs index 1165c3529d..fd7bfdcc0c 100644 --- a/src/status_im/accounts/login/core.cljs +++ b/src/status_im/accounts/login/core.cljs @@ -8,7 +8,8 @@ [status-im.utils.fx :as fx] [status-im.utils.keychain.core :as keychain] [status-im.utils.types :as types] - [taoensso.timbre :as log])) + [taoensso.timbre :as log] + [status-im.utils.security :as security])) ;; login flow: ;; @@ -84,7 +85,7 @@ (re-frame/reg-fx :accounts.login/login (fn [[address password save-password?]] - (login! address password save-password?))) + (login! address (security/safe-unmask-data password) save-password?))) (re-frame/reg-fx :accounts.login/clear-web-data @@ -93,4 +94,4 @@ (re-frame/reg-fx :data-store/change-account (fn [[address password]] - (change-account! address password))) + (change-account! address (security/safe-unmask-data password)))) diff --git a/src/status_im/accounts/recover/core.cljs b/src/status_im/accounts/recover/core.cljs index deffde3fd8..068bfde0cc 100644 --- a/src/status_im/accounts/recover/core.cljs +++ b/src/status_im/accounts/recover/core.cljs @@ -26,7 +26,7 @@ (defn recover-account! [masked-passphrase password] (status/recover-account - (mnemonic/sanitize-passphrase (security/unmask masked-passphrase)) + (mnemonic/sanitize-passphrase (security/safe-unmask-data masked-passphrase)) password (fn [result] ;; here we deserialize result, dissoc mnemonic and serialize the result again @@ -39,7 +39,7 @@ (fx/defn set-phrase [{:keys [db]} masked-recovery-phrase] - (let [recovery-phrase (security/unmask masked-recovery-phrase)] + (let [recovery-phrase (security/safe-unmask-data masked-recovery-phrase)] {:db (update db :accounts/recover assoc :passphrase (string/lower-case recovery-phrase) :passphrase-valid? (not (check-phrase-errors recovery-phrase)))})) @@ -53,7 +53,7 @@ (fx/defn set-password [{:keys [db]} masked-password] - (let [password (security/unmask masked-password)] + (let [password (security/safe-unmask-data masked-password)] {:db (update db :accounts/recover assoc :password password :password-valid? (not (check-password-errors password)))})) diff --git a/src/status_im/ui/screens/accounts/login/views.cljs b/src/status_im/ui/screens/accounts/login/views.cljs index 20692b8094..6f02620acd 100644 --- a/src/status_im/ui/screens/accounts/login/views.cljs +++ b/src/status_im/ui/screens/accounts/login/views.cljs @@ -17,7 +17,8 @@ [re-frame.core :as re-frame] [cljs.spec.alpha :as spec] [status-im.utils.platform :as platform] - [status-im.accounts.db :as db])) + [status-im.accounts.db :as db] + [status-im.utils.security :as security])) (defn login-toolbar [can-navigate-back?] [toolbar/toolbar @@ -71,7 +72,8 @@ :auto-focus true :on-submit-editing #(login-account @password-text-input) :on-change-text #(do - (re-frame/dispatch [:set-in [:accounts/login :password] %]) + (re-frame/dispatch [:set-in [:accounts/login :password] + (security/mask-data %)]) (re-frame/dispatch [:set-in [:accounts/login :error] ""])) :secure-text-entry true :error (when (not-empty error) (i18n/label (error-key error)))}]] @@ -98,5 +100,5 @@ [components.common/bottom-button {:forward? true :label (i18n/label :t/sign-in) - :disabled? (not (spec/valid? ::db/password password)) + :disabled? (not (spec/valid? ::db/password (security/safe-unmask-data password))) :on-press #(login-account @password-text-input)}]])])) diff --git a/src/status_im/ui/screens/wallet/send/events.cljs b/src/status_im/ui/screens/wallet/send/events.cljs index b17db74fc5..d015f4ac24 100644 --- a/src/status_im/ui/screens/wallet/send/events.cljs +++ b/src/status_im/ui/screens/wallet/send/events.cljs @@ -24,7 +24,7 @@ (defn- send-ethers [params on-completed masked-password] (status/send-transaction (types/clj->json params) - (security/unmask masked-password) + (security/safe-unmask-data masked-password) on-completed)) (defn- send-tokens [symbol chain {:keys [from to value gas gasPrice]} on-completed masked-password] @@ -75,7 +75,7 @@ (let [{:keys [data from password]} (get-in db [:wallet :send-transaction])] {:db (assoc-in db [:wallet :send-transaction :in-progress?] true) ::sign-message {:params {:data data - :password (security/unmask password) + :password (security/safe-unmask-data password) :account from} :on-completed #(re-frame/dispatch [::transaction-completed (types/json->clj %)])}}))) diff --git a/src/status_im/utils/ethereum/erc20.cljs b/src/status_im/utils/ethereum/erc20.cljs index da17b3e982..ffe27cedf2 100644 --- a/src/status_im/utils/ethereum/erc20.cljs +++ b/src/status_im/utils/ethereum/erc20.cljs @@ -50,7 +50,7 @@ {:from from :gas gas :gasPrice gas-price})) - (security/unmask masked-password) + (security/safe-unmask-data masked-password) on-completed)) (defn transfer-from [web3 contract from-address to-address value cb] diff --git a/src/status_im/utils/handlers.cljs b/src/status_im/utils/handlers.cljs index 5f1d8fd814..9705cb1735 100644 --- a/src/status_im/utils/handlers.cljs +++ b/src/status_im/utils/handlers.cljs @@ -21,9 +21,7 @@ (defn- pretty-print-event [ctx] (let [[first second] (get-coeffect ctx :event)] - (if (or (string? second) (keyword? second) (boolean? second)) - (str first " " second) - first))) + first)) (def debug-handlers-names "Interceptor which logs debug information to js/console for each event." diff --git a/src/status_im/utils/keychain/core.cljs b/src/status_im/utils/keychain/core.cljs index 70cd1eda50..f0c0123646 100644 --- a/src/status_im/utils/keychain/core.cljs +++ b/src/status_im/utils/keychain/core.cljs @@ -2,8 +2,8 @@ (:require [re-frame.core :as re-frame] [taoensso.timbre :as log] [status-im.react-native.js-dependencies :as rn] - [status-im.utils.handlers :as handlers] - [status-im.utils.platform :as platform])) + [status-im.utils.platform :as platform] + [status-im.utils.security :as security])) (def key-bytes 64) (def username "status-im.encryptionkey") @@ -55,7 +55,7 @@ (defn handle-callback [callback result] (if result - (callback (.-password result)) + (callback (security/mask-data (.-password result))) (callback nil))) ;; Gets the password for a specified address from the Keychain @@ -161,7 +161,7 @@ (fn [[address password]] (save-user-password address - password + (security/safe-unmask-data password) #(when-not % (log/error (str "Error while saving password." diff --git a/src/status_im/utils/security.cljs b/src/status_im/utils/security.cljs index 25c08019ad..c4f809ac2f 100644 --- a/src/status_im/utils/security.cljs +++ b/src/status_im/utils/security.cljs @@ -17,3 +17,8 @@ ;; Returns a MaskedData instance that stores the piece of data. (defn mask-data [data] (MaskedData. data)) + +(defn safe-unmask-data [data] + (if (instance? MaskedData data) + (unmask data) + data))