From fa55c3dc1b4bbdc6a53bd11fa6c0b2ec6bbb64ae Mon Sep 17 00:00:00 2001 From: MarcoFalke Date: Sat, 21 Aug 2021 16:17:52 +0200 Subject: Raise InitError when peers.dat is invalid or corrupted --- src/addrdb.cpp | 77 +++++++++++++++++++++++-------------------- src/addrdb.h | 2 +- src/test/addrman_tests.cpp | 4 +-- src/test/fuzz/data_stream.cpp | 5 ++- 4 files changed, 48 insertions(+), 40 deletions(-) (limited to 'src') diff --git a/src/addrdb.cpp b/src/addrdb.cpp index 77f0897a7f..1e73750ce3 100644 --- a/src/addrdb.cpp +++ b/src/addrdb.cpp @@ -21,6 +21,12 @@ #include namespace { + +class DbNotFoundError : public std::exception +{ + using std::exception::exception; +}; + template bool SerializeDB(Stream& stream, const Data& data) { @@ -78,47 +84,40 @@ bool SerializeFileDB(const std::string& prefix, const fs::path& path, const Data } template -bool DeserializeDB(Stream& stream, Data& data, bool fCheckSum = true) +void DeserializeDB(Stream& stream, Data& data, bool fCheckSum = true) { - try { - CHashVerifier verifier(&stream); - // de-serialize file header (network specific magic number) and .. - unsigned char pchMsgTmp[4]; - verifier >> pchMsgTmp; - // ... verify the network matches ours - if (memcmp(pchMsgTmp, Params().MessageStart(), sizeof(pchMsgTmp))) - return error("%s: Invalid network magic number", __func__); - - // de-serialize data - verifier >> data; - - // verify checksum - if (fCheckSum) { - uint256 hashTmp; - stream >> hashTmp; - if (hashTmp != verifier.GetHash()) { - return error("%s: Checksum mismatch, data corrupted", __func__); - } + CHashVerifier verifier(&stream); + // de-serialize file header (network specific magic number) and .. + unsigned char pchMsgTmp[4]; + verifier >> pchMsgTmp; + // ... verify the network matches ours + if (memcmp(pchMsgTmp, Params().MessageStart(), sizeof(pchMsgTmp))) { + throw std::runtime_error{"Invalid network magic number"}; + } + + // de-serialize data + verifier >> data; + + // verify checksum + if (fCheckSum) { + uint256 hashTmp; + stream >> hashTmp; + if (hashTmp != verifier.GetHash()) { + throw std::runtime_error{"Checksum mismatch, data corrupted"}; } } - catch (const std::exception& e) { - return error("%s: Deserialize or I/O error - %s", __func__, e.what()); - } - - return true; } template -bool DeserializeFileDB(const fs::path& path, Data& data, int version) +void DeserializeFileDB(const fs::path& path, Data& data, int version) { // open input file, and associate with CAutoFile FILE* file = fsbridge::fopen(path, "rb"); CAutoFile filein(file, SER_DISK, version); if (filein.IsNull()) { - LogPrintf("Missing or invalid file %s\n", path.string()); - return false; + throw DbNotFoundError{}; } - return DeserializeDB(filein, data); + DeserializeDB(filein, data); } } // namespace @@ -177,9 +176,9 @@ bool DumpPeerAddresses(const ArgsManager& args, const CAddrMan& addr) return SerializeFileDB("peers", pathAddr, addr, CLIENT_VERSION); } -bool ReadFromStream(CAddrMan& addr, CDataStream& ssPeers) +void ReadFromStream(CAddrMan& addr, CDataStream& ssPeers) { - return DeserializeDB(ssPeers, addr, false); + DeserializeDB(ssPeers, addr, false); } std::optional LoadAddrman(const std::vector& asmap, const ArgsManager& args, std::unique_ptr& addrman) @@ -189,13 +188,18 @@ std::optional LoadAddrman(const std::vector& asmap, const A int64_t nStart = GetTimeMillis(); const auto path_addr{args.GetDataDirNet() / "peers.dat"}; - if (DeserializeFileDB(path_addr, *addrman, CLIENT_VERSION)) { + try { + DeserializeFileDB(path_addr, *addrman, CLIENT_VERSION); LogPrintf("Loaded %i addresses from peers.dat %dms\n", addrman->size(), GetTimeMillis() - nStart); - } else { + } catch (const DbNotFoundError&) { // Addrman can be in an inconsistent state after failure, reset it addrman = std::make_unique(asmap, /* deterministic */ false, /* consistency_check_ratio */ check_addrman); - LogPrintf("Recreating peers.dat\n"); + LogPrintf("Creating peers.dat because the file was not found (%s)\n", path_addr); DumpPeerAddresses(args, *addrman); + } catch (const std::exception& e) { + addrman = nullptr; + return strprintf(_("Invalid or corrupt peers.dat (%s). If you believe this is a bug, please report it to %s. As a workaround, you can move the file (%s) out of the way (rename, move, or delete) to have a new one created on the next start."), + e.what(), PACKAGE_BUGREPORT, path_addr); } return std::nullopt; } @@ -209,9 +213,10 @@ void DumpAnchors(const fs::path& anchors_db_path, const std::vector& a std::vector ReadAnchors(const fs::path& anchors_db_path) { std::vector anchors; - if (DeserializeFileDB(anchors_db_path, anchors, CLIENT_VERSION | ADDRV2_FORMAT)) { + try { + DeserializeFileDB(anchors_db_path, anchors, CLIENT_VERSION | ADDRV2_FORMAT); LogPrintf("Loaded %i addresses from %s\n", anchors.size(), anchors_db_path.filename()); - } else { + } catch (const std::exception&) { anchors.clear(); } diff --git a/src/addrdb.h b/src/addrdb.h index 55fcd7508f..33cc1f9204 100644 --- a/src/addrdb.h +++ b/src/addrdb.h @@ -21,7 +21,7 @@ struct bilingual_str; bool DumpPeerAddresses(const ArgsManager& args, const CAddrMan& addr); /** Only used by tests. */ -bool ReadFromStream(CAddrMan& addr, CDataStream& ssPeers); +void ReadFromStream(CAddrMan& addr, CDataStream& ssPeers); /** Access to the banlist database (banlist.json) */ class CBanDB diff --git a/src/test/addrman_tests.cpp b/src/test/addrman_tests.cpp index 835b18d42e..01a492a20b 100644 --- a/src/test/addrman_tests.cpp +++ b/src/test/addrman_tests.cpp @@ -1043,7 +1043,7 @@ BOOST_AUTO_TEST_CASE(load_addrman) CAddrMan addrman2(/* asmap */ std::vector(), /* deterministic */ false, /* consistency_check_ratio */ 100); BOOST_CHECK(addrman2.size() == 0); - BOOST_CHECK(ReadFromStream(addrman2, ssPeers2)); + ReadFromStream(addrman2, ssPeers2); BOOST_CHECK(addrman2.size() == 3); } @@ -1073,7 +1073,7 @@ BOOST_AUTO_TEST_CASE(load_addrman_corrupted) CAddrMan addrman2(/* asmap */ std::vector(), /* deterministic */ false, /* consistency_check_ratio */ 100); BOOST_CHECK(addrman2.size() == 0); - BOOST_CHECK(!ReadFromStream(addrman2, ssPeers2)); + BOOST_CHECK_THROW(ReadFromStream(addrman2, ssPeers2), std::ios_base::failure); } diff --git a/src/test/fuzz/data_stream.cpp b/src/test/fuzz/data_stream.cpp index 08e9c91ba3..323090e041 100644 --- a/src/test/fuzz/data_stream.cpp +++ b/src/test/fuzz/data_stream.cpp @@ -23,5 +23,8 @@ FUZZ_TARGET_INIT(data_stream_addr_man, initialize_data_stream_addr_man) FuzzedDataProvider fuzzed_data_provider{buffer.data(), buffer.size()}; CDataStream data_stream = ConsumeDataStream(fuzzed_data_provider); CAddrMan addr_man(/* asmap */ std::vector(), /* deterministic */ false, /* consistency_check_ratio */ 0); - ReadFromStream(addr_man, data_stream); + try { + ReadFromStream(addr_man, data_stream); + } catch (const std::exception&) { + } } -- cgit v1.2.3