From fa7a883f5a219d5f3c2f992b090db4e6c279db12 Mon Sep 17 00:00:00 2001 From: MarcoFalke Date: Wed, 18 Aug 2021 09:57:56 +0200 Subject: [PATCH] addrman: Replace assert with throw on corrupt data Assert should only be used for program internal logic errors, not to sanitize external user input. --- src/addrman.cpp | 7 ++++++- test/functional/feature_addrman.py | 11 ++++++++++- 2 files changed, 16 insertions(+), 2 deletions(-) diff --git a/src/addrman.cpp b/src/addrman.cpp index a6f38075d8..7c6b8fe64d 100644 --- a/src/addrman.cpp +++ b/src/addrman.cpp @@ -386,7 +386,12 @@ void CAddrMan::Unserialize(Stream& s_) LogPrint(BCLog::ADDRMAN, "addrman lost %i new and %i tried addresses due to collisions or invalid addresses\n", nLostUnk, nLost); } - Check(); + const int check_code{ForceCheckAddrman()}; + if (check_code != 0) { + throw std::ios_base::failure(strprintf( + "Corrupt data. Consistency check failed with code %s", + check_code)); + } } // explicit instantiation diff --git a/test/functional/feature_addrman.py b/test/functional/feature_addrman.py index 42afd74ac9..55d3e48c64 100755 --- a/test/functional/feature_addrman.py +++ b/test/functional/feature_addrman.py @@ -19,6 +19,7 @@ def serialize_addrman( format=1, lowest_compatible=3, net_magic="regtest", + bucket_key=1, len_new=None, len_tried=None, mock_checksum=None, @@ -29,7 +30,7 @@ def serialize_addrman( r = MAGIC_BYTES[net_magic] r += struct.pack("B", format) r += struct.pack("B", INCOMPATIBILITY_BASE + lowest_compatible) - r += ser_uint256(1) + r += ser_uint256(bucket_key) r += struct.pack("i", len_new or len(new)) r += struct.pack("i", len_tried or len(tried)) ADDRMAN_NEW_BUCKET_COUNT = 1 << 10 @@ -119,6 +120,14 @@ class AddrmanTest(BitcoinTestFramework): match=ErrorMatch.FULL_REGEX, ) + self.log.info("Check that corrupt addrman cannot be read (failed check)") + self.stop_node(0) + write_addrman(peers_dat, bucket_key=0) + self.nodes[0].assert_start_raises_init_error( + expected_msg=init_error("Corrupt data. Consistency check failed with code -16: .*"), + match=ErrorMatch.FULL_REGEX, + ) + self.log.info("Check that missing addrman is recreated") self.stop_node(0) os.remove(peers_dat)