Ignore messages with too high clock-value and prevent stored messages to pollute the database

Signed-off-by: Andrea Maria Piana <andrea.maria.piana@gmail.com>

src/status_im/data_store/chats.cljs

@@ -3,6 +3,7 @@
             [cljs.core.async :as async]
             [re-frame.core :as re-frame]
             [status-im.utils.ethereum.core :as utils.ethereum]
+            [status-im.utils.clocks :as utils.clocks]
             [status-im.data-store.realm.core :as core]))
 
 (defn remove-empty-vals
@@ -49,15 +50,16 @@
                          :message :chat-id chat-id)
       (core/sorted :clock-value :desc)
       (core/single-clj :message)
-      :clock-value))
+      :clock-value
+      (utils.clocks/safe-timestamp)))
 
 (defn- normalize-chat [{:keys [chat-id] :as chat}]
-  (let [last-clock-value (get-last-clock-value chat-id)]
-    (-> chat
-        (update :admins   #(into #{} %))
-        (update :contacts #(into #{} %))
-        (update :membership-updates  (partial unmarshal-membership-updates chat-id))
-        (assoc :last-clock-value (or last-clock-value 0)))))
+  (-> chat
+      (update :admins   #(into #{} %))
+      (update :contacts #(into #{} %))
+      (update :membership-updates  (partial unmarshal-membership-updates chat-id))
+      ;; We cap the clock value to a safe value in case the db has been polluted
+      (assoc :last-clock-value (get-last-clock-value chat-id))))
 
 (re-frame/reg-cofx
  :data-store/all-chats

src/status_im/transport/db.cljs

@@ -3,6 +3,7 @@
   (:require-macros [status-im.utils.db :refer [allowed-keys]])
   (:require [cljs.spec.alpha :as spec]
             status-im.ui.screens.contacts.db
+            [status-im.utils.clocks :as utils.clocks]
             [clojure.string :as s]))
 
 ;; required
@@ -60,7 +61,8 @@
 
 (spec/def ::content-type #{"text/plain" "command" "command-request"})
 (spec/def ::message-type #{:group-user-message :public-group-user-message :user-message})
-(spec/def ::clock-value (spec/nilable pos-int?))
+(spec/def ::clock-value (spec/and pos-int?
+                                  utils.clocks/safe-timestamp?))
 (spec/def ::timestamp (spec/nilable pos-int?))
 
 (spec/def :message/id string?)

src/status_im/transport/message/v1/protocol.cljs

@@ -5,6 +5,7 @@
             [taoensso.timbre :as log]
             [status-im.utils.config :as config]
             [status-im.chat.core :as chat]
+            [status-im.utils.clocks :as utils.clocks]
             [status-im.transport.db :as transport.db]
             [status-im.transport.message.core :as message]
             [status-im.transport.message.v1.core :as transport]
@@ -110,8 +111,9 @@
              :from       signature
              :js-obj     (:js-obj cofx))]})
   (validate [this]
-    (when (spec/valid? :message/message this)
-      this)))
+    (if (spec/valid? :message/message this)
+      this
+      (log/warn "failed to validate Message" (spec/explain :message/message this)))))
 
 (defrecord MessagesSeen [message-ids]
   message/StatusMessage

src/status_im/utils/clocks.cljs

@@ -75,6 +75,8 @@
 (defn- ->timestamp-bid []
   (* (utils.datetime/timestamp) post-id-digits))
 
+(defn max-timestamp []
+  (* (+ one-month-in-ms (utils.datetime/timestamp)) post-id-digits))
 ; The timestamp has an upper limit of Number.MAX_SAFE_INTEGER
 ; A malicious client could send a crafted message with timestamp = Number.MAX_SAFE_INTEGER
 ; which effectively would DoS the chat, as any new message would get
@@ -83,7 +85,10 @@
 ; then now + 20s.
 ; We cap the timestamp to time now + 1 month to give some room for trusted peers
 (defn- safe-timestamp [t]
-  (min t (* (+ one-month-in-ms (utils.datetime/timestamp)) post-id-digits)))
+  (min t (max-timestamp)))
+
+(defn safe-timestamp? [t]
+  (<= t (max-timestamp)))
 
 (defn send [local-clock]
   (inc (max local-clock (->timestamp-bid))))

test/cljs/status_im/test/utils/clocks.cljs

@@ -92,6 +92,12 @@
     (is (< (clocks/receive js/Number.MAX_SAFE_INTEGER 0)
            js/Number.MAX_SAFE_INTEGER))))
 
+(deftest safe-timestamp?-test
+  (testing "it returns false for a high number"
+    (is (not (clocks/safe-timestamp? js/Number.MAX_SAFE_INTEGER))))
+  (testing "it returns true for a normal timestamp number"
+    (is (clocks/safe-timestamp? (clocks/send 0)))))
+
   ;; Debugging
 ;;(println "******************************************")
 ;;(println "A's POV :foo" (format-thread (thread a :foo)))