rpc: reject empty node argument in addnode
What the developer wrote
Authored by w0xlt
rpc: reject empty node argument in addnode
An empty (or whitespace only) node address cannot be resolved, but would
be added to the added nodes list and retried indefinitely, the same way
that an empty -addnode value was before the previous commits.
Reject it for all commands instead. Returning false from AddNode() would
report the misleading "Node already added" error, so check it here.
What changed, and why it matters
This change fixes a minor bug in Bitcoin Core's RPC command `addnode`. Previously, if a user passed an empty or whitespace-only address, Bitcoin Core would add it to its persistent 'try to connect' list and keep retrying forever, even though the address could never work. Now it immediately rejects the bad input with a clear error message. It is a hardening fix rather than a serious security vulnerability.
No urgent action required. Operators should ensure they are running a version that includes this fix if they expose `addnode` RPC to untrusted callers, as part of normal patch management.
Security signals we found
Input validation hardening for RPC parameter
Prevention of resource-wasting infinite retry loop
Functional test coverage added for malformed input
Evidence from the diff
The addnode RPC now trims and validates the node argument before processing. If TrimStringView(node_arg).empty() is true, it throws RPC_INVALID_PARAMETER (‘Node address cannot be empty’). This prevents empty strings from being inserted into CConnman::added_nodes and retried indefinitely. The patch includes functional tests covering add, remove, and onetry subcommands with empty and whitespace-only inputs.
Changed components
src/rpc/net.cpptest/functional/rpc_net.pyaddnode RPCInspect captured patch +11 / −0
diff --git a/src/rpc/net.cpp b/src/rpc/net.cpp
index ba1080ed..382e4d56 100644
--- a/src/rpc/net.cpp
+++ b/src/rpc/net.cpp
@@ -47,6 +47,7 @@
using node::NodeContext;
using util::Join;
+using util::TrimStringView;
const std::vector<std::string> CONNECTION_TYPE_DOC{
"outbound-full-relay (default automatic connections)",
@@ -348,6 +349,11 @@ static RPCMethod addnode()
CConnman& connman = EnsureConnman(node);
const auto node_arg{self.Arg<std::string_view>("node")};
+ if (TrimStringView(node_arg).empty()) {
+ // Such a node would never resolve, but would be retried indefinitely.
+ throw JSONRPCError(RPC_INVALID_PARAMETER, "Error: Node address cannot be empty");
+ }
+
bool node_v2transport = connman.GetLocalServices() & NODE_P2P_V2;
bool use_v2transport = self.MaybeArg<bool>("v2transport").value_or(node_v2transport);
diff --git a/test/functional/rpc_net.py b/test/functional/rpc_net.py
index 81cf4da7..8d1bc244 100755
--- a/test/functional/rpc_net.py
+++ b/test/functional/rpc_net.py
@@ -272,6 +272,11 @@ class NetTest(BitcoinTestFramework):
assert_equal(added_nodes[0]['addednode'], "11.22.33.44")
self.log.info("Check that an invalid command returns an error")
assert_raises_rpc_error(-1, 'addnode "node" "command"', self.nodes[0].addnode, node=ip_port, command='abc')
+ self.log.info("Check that an empty node address returns an error")
+ for command in ['add', 'remove', 'onetry']:
+ assert_raises_rpc_error(-8, "Node address cannot be empty", self.nodes[0].addnode, node="", command=command)
+ assert_raises_rpc_error(-8, "Node address cannot be empty", self.nodes[0].addnode, node=" ", command=command)
+ assert_equal(len(self.nodes[0].getaddednodeinfo()), 1)
self.log.info("Check that trying to remove the node again returns an error")
assert_raises_rpc_error(-24, "Node could not be removed", self.nodes[0].addnode, node=ip_port, command='remove')
self.log.info("Check that a non-existent node returns an error")
Why this scored 27/100
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.