addrman: Replace assert with throw on corrupt data

MarcoFalke · MarcoFalke · commit fa7a883f5a21 · 2021-09-21T10:09:45.000+02:00
Assert should only be used for program internal logic errors, not to
sanitize external user input.
diff --git 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
@@ -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 @@ def run_test(self):
             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)

Original file line number	Diff line number	Diff line change
`@@ -386,7 +386,12 @@ void CAddrMan::Unserialize(Stream& s_)`
`386`	`386`	`LogPrint(BCLog::ADDRMAN, "addrman lost %i new and %i tried addresses due to collisions or invalid addresses\n", nLostUnk, nLost);`
`387`	`387`	`}`
`388`	`388`
`389`		`- Check();`
	`389`	`+ const int check_code{ForceCheckAddrman()};`
	`390`	`+ if (check_code != 0) {`
	`391`	`+ throw std::ios_base::failure(strprintf(`
	`392`	`+ "Corrupt data. Consistency check failed with code %s",`
	`393`	`+ check_code));`
	`394`	`+ }`
`390`	`395`	`}`
`391`	`396`
`392`	`397`	`// explicit instantiation`