Fix the problem of memory growth caused by forged P2P message packets (…

…FISCO-BCOS#1958)
hnustyangjieyu · Jun 28, 2021 · 2ac180e · 2ac180e
1 parent 8c1f492
commit 2ac180e
Show file tree

Hide file tree

Showing 8 changed files with 23 additions and 16 deletions.
diff --git a/cmake/Options.cmake b/cmake/Options.cmake
@@ -63,7 +63,7 @@ macro(configure_project)
     eth_default_option(ARCH_NATIVE OFF)
 
     if(ARCH_NATIVE)
-        set(MARCH_TYPE "-march=native -mtune=native -fvisibility=hidden -fvisibility-inlines-hidden")
+        set(MARCH_TYPE "-march=native -mtune=generic -fvisibility=hidden -fvisibility-inlines-hidden")
     endif()
 
     # unit tests

diff --git a/libchannelserver/ChannelMessage.h b/libchannelserver/ChannelMessage.h
@@ -48,7 +48,8 @@ enum ChannelMessageType
     CLIENT_HEARTBEAT = 0x13,              // type for heart beat for sdk
     CLIENT_HANDSHAKE = 0x14,              // type for hand shake
     CLIENT_REGISTER_EVENT_LOG = 0x15,     // type for event log filter register request and response
-    CLIENT_UNREGISTER_EVENT_LOG = 0x16,   // type for event log filter unregister request and response
+    CLIENT_UNREGISTER_EVENT_LOG = 0x16,   // type for event log filter unregister request and
+                                          // response
     AMOP_REQUEST = 0x30,                  // type for request from sdk
     AMOP_RESPONSE = 0x31,                 // type for response to sdk
     AMOP_CLIENT_SUBSCRIBE_TOPICS = 0x32,  // type for topic request
@@ -93,13 +94,13 @@ class ChannelMessage : public Message
         }
 
         m_length = ntohl(*((uint32_t*)&buffer[0]));
-
-#if 0
-        if (_length > MAX_LENGTH)
+        // invalid packet
+        if (m_length > MAX_LENGTH)
         {
+            CHANNEL_LOG(WARNING) << LOG_DESC("Illegal message packet") << LOG_KV("length", m_length)
+                                 << LOG_KV("maxLen", MAX_LENGTH);
             return -1;
         }
-#endif
 
         if (size < m_length)
         {
@@ -119,7 +120,7 @@ class ChannelMessage : public Message
     const static size_t MIN_HEADER_LENGTH = 4;
 
     const static size_t HEADER_LENGTH = 4 + 2 + 32 + 4;
-    const static size_t MAX_LENGTH = ULONG_MAX;  // max 4G
+    const static size_t MAX_LENGTH = 100 * 1024 * 1024;  // max 100MB
 };
 
 class ChannelMessageFactory : public MessageFactory

diff --git a/libp2p/P2PMessage.cpp b/libp2p/P2PMessage.cpp
@@ -52,12 +52,12 @@ ssize_t P2PMessage::decode(const byte* buffer, size_t size)
 
     int32_t offset = 0;
     m_length = ntohl(*((uint32_t*)&buffer[offset]));
-
-    /*if (m_length > MAX_LENGTH)
+    if (m_length > MAX_MESSAGE_LENGTH)
     {
-        return PACKET_ERROR;
-    }*/
-
+        SESSION_LOG(WARNING) << LOG_DESC("Illegal p2p message packet") << LOG_KV("length", m_length)
+                             << LOG_KV("maxLen", MAX_MESSAGE_LENGTH);
+        return dev::network::PACKET_ERROR;
+    }
     if (size < m_length)
     {
         return dev::network::PACKET_INCOMPLETE;

diff --git a/libp2p/P2PMessage.h b/libp2p/P2PMessage.h
@@ -38,7 +38,8 @@ class P2PMessage : public dev::network::Message
     /// packetType: 2bytes
     /// seq: 4 bytes
     const static size_t HEADER_LENGTH = 12;
-    const static size_t MAX_LENGTH = 1024 * 1024;  ///< The maximum length of data is 1M.
+    const static size_t MAX_MESSAGE_LENGTH =
+        100 * 1024 * 1024;  ///< The maximum length of data is 100M.
 
     P2PMessage() { m_buffer = std::make_shared<bytes>(); }
 

diff --git a/libp2p/P2PMessageRC2.cpp b/libp2p/P2PMessageRC2.cpp
@@ -105,6 +105,12 @@ ssize_t P2PMessageRC2::decode(const byte* buffer, size_t size)
 
     int32_t offset = 0;
     m_length = ntohl(*((uint32_t*)&buffer[offset]));
+    if (m_length > P2PMessage::MAX_MESSAGE_LENGTH)
+    {
+        SESSION_LOG(WARNING) << LOG_DESC("Illegal p2p message packet") << LOG_KV("length", m_length)
+                             << LOG_KV("maxLen", P2PMessage::MAX_MESSAGE_LENGTH);
+        return dev::network::PACKET_ERROR;
+    }
 
     if (size < m_length)
     {

diff --git a/libp2p/P2PMessageRC2.h b/libp2p/P2PMessageRC2.h
@@ -33,7 +33,6 @@ class P2PMessageRC2 : public P2PMessage
     /// m_length(4bytes) + m_version(2bytes) + m_protocolID(2bytes) + m_groupID(2bytes) +
     /// m_packetType(2bytes) + m_seq(4bytes)
     const static size_t HEADER_LENGTH = 16;
-    const static size_t MAX_LENGTH = 1024 * 1024;  ///< The maximum length of data is 1M.
 
     P2PMessageRC2()
     {

diff --git a/libsync/SyncMsgEngine.cpp b/libsync/SyncMsgEngine.cpp
@@ -29,7 +29,7 @@ using namespace dev::p2p;
 using namespace dev::blockchain;
 using namespace dev::txpool;
 
-static size_t const c_maxPayload = dev::p2p::P2PMessage::MAX_LENGTH - 2048;
+static size_t const c_maxPayload = dev::p2p::P2PMessage::MAX_MESSAGE_LENGTH - 2048;
 
 void SyncMsgEngine::messageHandler(
     NetworkException, std::shared_ptr<dev::p2p::P2PSession> _session, P2PMessage::Ptr _msg)

diff --git a/test/unittests/libsync/SyncMsgEngineTest.cpp b/test/unittests/libsync/SyncMsgEngineTest.cpp
@@ -278,7 +278,7 @@ BOOST_AUTO_TEST_CASE(SyncReqBlockPacketTest)
 BOOST_AUTO_TEST_CASE(BatchSendTest)
 {
     size_t maxPayloadSize =
-        dev::p2p::P2PMessage::MAX_LENGTH - 2048;  // should be the same as syncMsgEngine.cpp
+        dev::p2p::P2PMessage::MAX_MESSAGE_LENGTH - 2048;  // should be the same as syncMsgEngine.cpp
     size_t quarterPayloadSize = maxPayloadSize / 4;
 
     FakeServiceForDownloadBlocksContainer::Ptr service =