From b99f1f20f773eb55c870a033b3f2e8f13d55d0c8 Mon Sep 17 00:00:00 2001 From: brunoerg Date: Thu, 5 Jan 2023 15:11:32 -0300 Subject: [PATCH 1/2] p2p, rpc: don't allow past absolute timestamp in `setban` --- src/rpc/net.cpp | 4 ++++ test/functional/p2p_disconnect_ban.py | 3 +++ 2 files changed, 7 insertions(+) diff --git a/src/rpc/net.cpp b/src/rpc/net.cpp index 4611e03270..d430087358 100644 --- a/src/rpc/net.cpp +++ b/src/rpc/net.cpp @@ -741,6 +741,10 @@ static RPCHelpMan setban() const bool absolute{request.params[3].isNull() ? false : request.params[3].get_bool()}; + if (absolute && banTime < GetTime()) { + throw JSONRPCError(RPC_INVALID_PARAMETER, "Error: Absolute timestamp is in the past"); + } + if (isSubnet) { node.banman->Ban(subNet, banTime, absolute); if (node.connman) { diff --git a/test/functional/p2p_disconnect_ban.py b/test/functional/p2p_disconnect_ban.py index 91c2a43932..8d4f50e681 100755 --- a/test/functional/p2p_disconnect_ban.py +++ b/test/functional/p2p_disconnect_ban.py @@ -46,6 +46,9 @@ class DisconnectBanTest(BitcoinTestFramework): assert_raises_rpc_error(-30, "Error: Invalid IP/Subnet", self.nodes[1].setban, "127.0.0.1/42", "add") assert_equal(len(self.nodes[1].listbanned()), 1) # still only one banned ip because 127.0.0.1 is within the range of 127.0.0.0/24 + self.log.info("setban: fail to ban with past absolute timestamp") + assert_raises_rpc_error(-8, "Error: Absolute timestamp is in the past", self.nodes[1].setban, "127.27.0.1", "add", 123, True) + self.log.info("setban remove: fail to unban a non-banned subnet") assert_raises_rpc_error(-30, "Error: Unban failed", self.nodes[1].setban, "127.0.0.1", "remove") assert_equal(len(self.nodes[1].listbanned()), 1) From abccb27466c5a240a59f3f7c86f2bc60d51e9fee Mon Sep 17 00:00:00 2001 From: brunoerg Date: Fri, 6 Jan 2023 09:12:02 -0300 Subject: [PATCH 2/2] test: add coverage for absolute timestamp in `setban` --- test/functional/p2p_disconnect_ban.py | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/test/functional/p2p_disconnect_ban.py b/test/functional/p2p_disconnect_ban.py index 8d4f50e681..7169cc8937 100755 --- a/test/functional/p2p_disconnect_ban.py +++ b/test/functional/p2p_disconnect_ban.py @@ -69,9 +69,13 @@ class DisconnectBanTest(BitcoinTestFramework): self.nodes[1].setban("2001:4d48:ac57:400:cacf:e9ff:fe1d:9c63/19", "add", 1000) # ban for 1000 seconds listBeforeShutdown = self.nodes[1].listbanned() assert_equal("192.168.0.1/32", listBeforeShutdown[2]['address']) + + self.log.info("setban: test banning with absolute timestamp") + self.nodes[1].setban("192.168.0.2", "add", old_time + 120, True) + # Move time forward by 3 seconds so the third ban has expired self.nodes[1].setmocktime(old_time + 3) - assert_equal(len(self.nodes[1].listbanned()), 3) + assert_equal(len(self.nodes[1].listbanned()), 4) self.log.info("Test ban_duration and time_remaining") for ban in self.nodes[1].listbanned(): @@ -81,13 +85,17 @@ class DisconnectBanTest(BitcoinTestFramework): elif ban["address"] == "2001:4d48:ac57:400:cacf:e9ff:fe1d:9c63/19": assert_equal(ban["ban_duration"], 1000) assert_equal(ban["time_remaining"], 997) + elif ban["address"] == "192.168.0.2/32": + assert_equal(ban["ban_duration"], 120) + assert_equal(ban["time_remaining"], 117) self.restart_node(1) listAfterShutdown = self.nodes[1].listbanned() assert_equal("127.0.0.0/24", listAfterShutdown[0]['address']) assert_equal("127.0.0.0/32", listAfterShutdown[1]['address']) - assert_equal("/19" in listAfterShutdown[2]['address'], True) + assert_equal("192.168.0.2/32", listAfterShutdown[2]['address']) + assert_equal("/19" in listAfterShutdown[3]['address'], True) # Clear ban lists self.nodes[1].clearbanned()