From c1c7daea00058fc20b85fd1628bb35ddd7bcef23 Mon Sep 17 00:00:00 2001
From: dec05eba <dec05eba@protonmail.com>
Date: Mon, 21 May 2018 10:09:54 +0200
Subject: Prevent malicious peer from replaying user ping

---
 src/Channel.cpp        | 34 ++++++++++------------------------
 src/User.cpp           |  3 +--
 src/UsersSidePanel.cpp |  8 ++++----
 3 files changed, 15 insertions(+), 30 deletions(-)

(limited to 'src')

diff --git a/src/Channel.cpp b/src/Channel.cpp
index 5043665..1ac34c0 100644
--- a/src/Channel.cpp
+++ b/src/Channel.cpp
@@ -48,14 +48,10 @@ namespace dchat
                     
                     string unsignedData = userPublicKey.unsign(odhtdb::DataView((void*)deserializer.getBuffer(), deserializer.getSize()));
                     sibs::SafeDeserializer unsignedDeserializer((const u8*)unsignedData.data(), unsignedData.size());
-                    u32 pingCounter = unsignedDeserializer.extract<u32>();
-                    u64 pingTimestamp = unsignedDeserializer.extract<u64>();
-                    // TODO: A malicious peer can capture the packets and reply them after the user has reconnect and counter has reset, need to fix this somehow.
-                    // One solution is for the user to store the counter locally in file and continue using it when reconnecting
-                    if(pingTimestamp > user->pingTimestamp)
+                    u32 pingTimestampSec = unsignedDeserializer.extract<u32>();
+                    if(pingTimestampSec > user->pingTimestampSec)
                     {
-                        user->pingCounter = pingCounter;
-                        user->pingTimestamp = pingTimestamp;
+                        user->pingTimestampSec = pingTimestampSec;
                     }
                 }
                 catch(std::exception &e)
@@ -65,11 +61,7 @@ namespace dchat
                 return result;
             });
             
-            if(localUser->type == User::Type::ONLINE_LOCAL_USER)
-            {
-                auto onlineLocalUser = static_cast<OnlineLocalUser*>(localUser);
-                sendPing(onlineLocalUser->pingCounter + 1, database->getSyncedTimestampUtc().getCombined());
-            }
+            sendPing(database->getSyncedTimestampUtc().seconds);
         }
     }
     
@@ -79,11 +71,7 @@ namespace dchat
         {
             database->cancelNodeListener(pingKey, pingListener);
             database->stopSeeding(*databaseNodeInfo.getRequestHash());
-            if(database && localUser->type == User::Type::ONLINE_LOCAL_USER)
-            {
-                auto onlineLocalUser = static_cast<OnlineLocalUser*>(localUser);
-                sendPing(onlineLocalUser->pingCounter + 1, 0);
-            }
+            sendPing(0);
         }
         
         for(User *user : users)
@@ -266,12 +254,11 @@ namespace dchat
         if(database && localUser->type == User::Type::ONLINE_LOCAL_USER && pingTimer.getElapsedTime().asMilliseconds() > 5000)
         {
             pingTimer.restart();
-            auto onlineLocalUser = static_cast<OnlineLocalUser*>(localUser);
-            sendPing(onlineLocalUser->pingCounter + 1, database->getSyncedTimestampUtc().getCombined());
+            sendPing(database->getSyncedTimestampUtc().seconds);
         }
     }
     
-    void Channel::sendPing(u32 pingCounter, u64 pingTimestamp)
+    void Channel::sendPing(u32 pingTimestampSec)
     {
         if(database && localUser->type == User::Type::ONLINE_LOCAL_USER)
         {
@@ -281,19 +268,18 @@ namespace dchat
             serializer.add((const u8*)onlineLocalUser->getPublicKey().getData(), onlineLocalUser->getPublicKey().getSize());
             
             sibs::SafeSerializer signedSerializer;
-            signedSerializer.add(pingCounter);
-            signedSerializer.add(pingTimestamp);
+            signedSerializer.add(pingTimestampSec);
             string signedData = onlineLocalUser->keyPair.getPrivateKey().sign(odhtdb::DataView(signedSerializer.getBuffer().data(), signedSerializer.getBuffer().size()));
             serializer.add((const u8*)signedData.data(), signedData.size());
             database->sendCustomMessage(pingKey, move(serializer.getBuffer()));
         }
     }
     
-    u64 Channel::getSyncedTimestampUtcCombined()
+    u32 Channel::getSyncedTimestampUtcInSec()
     {
         if(!database)
             return 0;
-        return database->getSyncedTimestampUtc().getCombined();
+        return database->getSyncedTimestampUtc().seconds;
     }
     
     void Channel::setCurrent(Channel *channel)
diff --git a/src/User.cpp b/src/User.cpp
index 0cfbb4e..aabfd0b 100644
--- a/src/User.cpp
+++ b/src/User.cpp
@@ -13,8 +13,7 @@ namespace dchat
     OnlineUser::OnlineUser(const std::string &_name, Type type) : 
         User(type),
         name(_name),
-        pingCounter(0),
-        pingTimestamp(0)
+        pingTimestampSec(0)
     {
         
     }
diff --git a/src/UsersSidePanel.cpp b/src/UsersSidePanel.cpp
index 6ddfbe2..c05a54f 100644
--- a/src/UsersSidePanel.cpp
+++ b/src/UsersSidePanel.cpp
@@ -85,7 +85,7 @@ namespace dchat
         
         const float textHeight = font->getLineSpacing(FONT_SIZE * Settings::getScaling());
         
-        i64 timestamp = currentChannel->getSyncedTimestampUtcCombined();
+        i64 timestampSec = currentChannel->getSyncedTimestampUtcInSec();
         
         u32 numOnlineUsers = 0;
         u32 numOfflineUsers = 0;
@@ -95,7 +95,7 @@ namespace dchat
             if(user->isOnlineUser())
             {
                 auto onlineUser = static_cast<OnlineUser*>(user);
-                i64 pingTimeDiffSec = (timestamp - (i64)onlineUser->pingTimestamp) >> 32LL;
+                i64 pingTimeDiffSec = timestampSec - (i64)onlineUser->pingTimestampSec;
                 if(pingTimeDiffSec > USER_TIMEOUT_SEC)
                     hasUserTimedOut = true;
             }
@@ -124,7 +124,7 @@ namespace dchat
             if(user->isOnlineUser())
             {
                 auto onlineUser = static_cast<OnlineUser*>(user);
-                i64 pingTimeDiffSec = (timestamp - (i64)onlineUser->pingTimestamp) >> 32LL;
+                i64 pingTimeDiffSec = timestampSec - (i64)onlineUser->pingTimestampSec;
                 if(pingTimeDiffSec > USER_TIMEOUT_SEC)
                     isUserOnline = false;
             }
@@ -152,7 +152,7 @@ namespace dchat
             if(user->isOnlineUser()) 
             {
                 auto onlineUser = static_cast<OnlineUser*>(user);
-                i64 pingTimeDiffSec = (timestamp - (i64)onlineUser->pingTimestamp) >> 32LL;
+                i64 pingTimeDiffSec = timestampSec - (i64)onlineUser->pingTimestampSec;
                 if(pingTimeDiffSec > USER_TIMEOUT_SEC)
                     isUserOnline = false;
             }
-- 
cgit v1.2.3