Merge pull request #2997 from patricklodder/1.14.6-enhance-addnode-memory

net: constrain the memory usage of manually added nodes

qa/pull-tester/rpc-tests.py

@@ -171,6 +171,7 @@ testScripts = [
     'p2p-policy.py',
     'wallet_create_tx.py',
     'liststucktransactions.py',
+    'addnode.py',
 ]
 if ENABLE_ZMQ:
     testScripts.append('zmq_test.py')

qa/rpc-tests/addnode.py

@@ -0,0 +1,67 @@
+#!/usr/bin/env python3
+# Copyright (c) 2021 The Dogecoin Core developers
+# Distributed under the MIT software license, see the accompanying
+# file COPYING or http://www.opensource.org/licenses/mit-license.php.
+"""Addnode QA test.
+# Tests the addnode command
+"""
+
+from test_framework.test_framework import BitcoinTestFramework
+from test_framework.util import *
+
+class AddnodeTest(BitcoinTestFramework):
+
+    def __init__(self):
+        super().__init__()
+        self.setup_clean_chain = True
+        self.num_nodes = 1
+        self.counter = 0
+
+    def setup_network(self, split=False):
+        self.nodes = []
+        self.nodes.append(start_node(0, self.options.tmpdir, []))
+
+        self.is_network_split=False
+        self.sync_all()
+
+    def get_next_dummy_address(self):
+        self.counter += 1
+        assert self.counter < 256 ** 2  # Don't allow the returned ip addresses to wrap.
+        return f"123.123.{self.counter // 256}.{self.counter % 256}:{10000 + self.counter}"
+
+    def run_test(self):
+        node = self.nodes[0]
+        # mine a block
+        node.generate(1)
+
+        # add one address
+        first_addr = self.get_next_dummy_address()
+        node.addnode(first_addr, 'add')
+
+        # try adding it again
+        try:
+            node.addnode(first_addr, 'add')
+            raise AssertionError("Must reject an existing addnode")
+        except JSONRPCException as e:
+            assert("Error: Unable to add node" in e.error["message"])
+
+        # add 799 valid addresses - must not throw an error
+        for i in range((8 * 100) - 1):
+            node.addnode(self.get_next_dummy_address(), 'add')
+
+        # add one more address
+        try:
+            node.addnode(self.get_next_dummy_address(), 'add')
+            raise AssertionError("Must reject when more than 800 addnode entries exist")
+        except JSONRPCException as e:
+            assert("Error: Unable to add node" in e.error["message"])
+
+        # try adding a large string domain
+        try:
+            node.addnode(f'{"manybigstr" * 25}.domain:10020', 'add')
+            raise AssertionError("Must reject large strings")
+        except JSONRPCException as e:
+            assert("Error: Node address is invalid" in e.error["message"])
+
+if __name__ == '__main__':
+    AddnodeTest().main()

src/net.cpp

@@ -2492,6 +2492,13 @@ std::vector<CAddress> CConnman::GetAddresses()
 bool CConnman::AddNode(const std::string& strNode)
 {
     LOCK(cs_vAddedNodes);
+
+    // We only allow 100x the amount of total connections, to protect against
+    // script errors that fill up memory of the node with addresses
+    if (vAddedNodes.size() >= MAX_ADDNODE_CONNECTIONS * 100) {
+      return false;
+    }
+
     for(std::vector<std::string>::const_iterator it = vAddedNodes.begin(); it != vAddedNodes.end(); ++it) {
         if (strNode == *it)
             return false;

src/qt/peerdialog.cpp

@@ -34,6 +34,10 @@ QString PeerTools::ManagePeer(QString type, QString peer)
 {
     std::string peerAddress = peer.toStdString();
 
+    if (peerAddress.size() > 256) {
+      return tr("Error: Node address is invalid");
+    }
+
     if(!g_connman)
         return tr("Error: Peer-to-peer functionality missing or disabled");
 
@@ -47,7 +51,7 @@ QString PeerTools::ManagePeer(QString type, QString peer)
     if (type == "add")
     {
         if(!g_connman->AddNode(peerAddress))
-            return tr("Error: Node already added");
+            return tr("Error: Unable to add node");
     }
     else if(type == "remove")
     {

src/rpc/net.cpp

@@ -247,6 +247,10 @@ UniValue addnode(const JSONRPCRequest& request)
 
     string strNode = request.params[0].get_str();
 
+    if (strNode.size() > 256) {
+      throw JSONRPCError(RPC_CLIENT_NODE_ADDRESS_INVALID, "Error: Node address is invalid");
+    }
+
     if (strCommand == "onetry")
     {
         CAddress addr;
@@ -257,7 +261,7 @@ UniValue addnode(const JSONRPCRequest& request)
     if (strCommand == "add")
     {
         if(!g_connman->AddNode(strNode))
-            throw JSONRPCError(RPC_CLIENT_NODE_ALREADY_ADDED, "Error: Node already added");
+            throw JSONRPCError(RPC_CLIENT_NODE_ALREADY_ADDED, "Error: Unable to add node");
     }
     else if(strCommand == "remove")
     {

src/rpc/protocol.h

@@ -64,6 +64,7 @@ enum RPCErrorCode
     RPC_CLIENT_NODE_NOT_CONNECTED   = -29, //!< Node to disconnect not found in connected nodes
     RPC_CLIENT_INVALID_IP_OR_SUBNET = -30, //!< Invalid IP/Subnet
     RPC_CLIENT_P2P_DISABLED         = -31, //!< No valid connection manager instance found
+    RPC_CLIENT_NODE_ADDRESS_INVALID = -32, //!< Node address provided is not valid
 
     //! Wallet errors
     RPC_WALLET_ERROR                = -4,  //!< Unspecified problem with wallet (key not found etc.)