util,test: fix roundtrip asymmetries in parser and serializer code

src/CMakeLists.txt

@@ -194,6 +194,7 @@ add_library(bitcoin_node STATIC EXCLUDE_FROM_ALL
   index/coinstatsindex.cpp
   index/txindex.cpp
   init.cpp
+  init/args.cpp
   kernel/chain.cpp
   kernel/checks.cpp
   kernel/coinstats.cpp

src/compressor.h

@@ -6,6 +6,7 @@
 #ifndef BITCOIN_COMPRESSOR_H
 #define BITCOIN_COMPRESSOR_H
 
+#include <consensus/amount.h>
 #include <prevector.h>
 #include <primitives/transaction.h>
 #include <script/script.h>
@@ -105,7 +106,11 @@ struct AmountCompression
     {
         uint64_t v;
         s >> VARINT(v);
-        val = DecompressAmount(v);
+        const uint64_t amount{DecompressAmount(v)};
+        if (amount > static_cast<uint64_t>(MAX_MONEY)) {
+            throw std::ios_base::failure("AmountCompression::Unser(): amount out of range");
+        }
+        val = amount;
     }
 };
 

src/key_io.cpp

@@ -6,6 +6,7 @@
 
 #include <base58.h>
 #include <bech32.h>
+#include <chainparams.h>
 #include <script/interpreter.h>
 #include <script/solver.h>
 #include <tinyformat.h>

src/key_io.h

@@ -7,12 +7,12 @@
 #define BITCOIN_KEY_IO_H
 
 #include <addresstype.h>
-#include <chainparams.h>
 #include <key.h>
-#include <pubkey.h>
 
 #include <string>
 
+class CChainParams;
+
 CKey DecodeSecret(const std::string& str);
 std::string EncodeSecret(const CKey& key);
 

src/musig.cpp

@@ -4,6 +4,7 @@
 
 #include <musig.h>
 #include <support/allocators/secure.h>
+#include <util/strencodings.h>
 
 #include <secp256k1_musig.h>
 

src/outputtype.cpp

@@ -9,6 +9,7 @@
 #include <script/script.h>
 #include <script/sign.h>
 #include <script/signingprovider.h>
+#include <util/string.h>
 
 #include <cassert>
 #include <optional>

src/rpc/rawtransaction_util.cpp

@@ -21,6 +21,7 @@
 #include <util/string.h>
 #include <util/strencodings.h>
 #include <util/translation.h>
+#include <util/vector.h>
 
 void AddInputs(CMutableTransaction& rawTx, const UniValue& inputs_in, std::optional<bool> rbf)
 {

src/test/compress_tests.cpp

@@ -4,10 +4,12 @@
 
 #include <compressor.h>
 #include <script/script.h>
+#include <streams.h>
 #include <test/util/random.h>
 #include <test/util/setup_common.h>
 
 #include <cstdint>
+#include <limits>
 
 #include <boost/test/unit_test.hpp>
 
@@ -63,6 +65,15 @@ BOOST_AUTO_TEST_CASE(compress_amounts)
         BOOST_CHECK(TestDecode(i));
 }
 
+BOOST_AUTO_TEST_CASE(compress_amount_deserialize_out_of_range)
+{
+    DataStream stream{};
+    stream << VARINT(std::numeric_limits<uint64_t>::max());
+
+    CAmount amount;
+    BOOST_CHECK_THROW(stream >> Using<AmountCompression>(amount), std::ios_base::failure);
+}
+
 BOOST_AUTO_TEST_CASE(compress_script_to_ckey_id)
 {
     // case CKeyID

src/test/fuzz/base_encode_decode.cpp

@@ -7,6 +7,7 @@
 #include <base58.h>
 #include <psbt.h>
 #include <span.h>
+#include <streams.h>
 #include <test/fuzz/FuzzedDataProvider.h>
 #include <util/strencodings.h>
 #include <util/string.h>
@@ -18,6 +19,15 @@
 
 using util::TrimStringView;
 
+namespace {
+std::vector<std::byte> SerializePSBT(const PartiallySignedTransaction& psbt)
+{
+    DataStream ds{};
+    ds << psbt;
+    return {ds.begin(), ds.end()};
+}
+} // namespace
+
 FUZZ_TARGET(base58_encode_decode)
 {
     FuzzedDataProvider provider{buffer.data(), buffer.size()};
@@ -94,4 +104,12 @@ FUZZ_TARGET(psbt_base64_decode)
     std::string error;
     const bool ok{DecodeBase64PSBT(psbt, random_string, error)};
     assert(ok == error.empty());
+    if (ok) {
+        const std::vector<std::byte> serialized_psbt = SerializePSBT(psbt);
+        PartiallySignedTransaction roundtrip_psbt;
+        std::string roundtrip_error;
+        assert(DecodeBase64PSBT(roundtrip_psbt, EncodeBase64(serialized_psbt), roundtrip_error));
+        assert(roundtrip_error.empty());
+        assert(SerializePSBT(roundtrip_psbt) == serialized_psbt);
+    }
 }

src/test/fuzz/decode_tx.cpp

@@ -29,4 +29,10 @@ FUZZ_TARGET(decode_tx)
         assert(!no_witness_mtx.HasWitness());
         assert(result_try_witness_and_maybe_no_witness);
     }
+    if (result_try_witness || result_try_witness_and_maybe_no_witness) {
+        const std::string canonical_tx_hex = EncodeHexTx(CTransaction{mtx});
+        CMutableTransaction roundtrip_mtx;
+        assert(DecodeHexTx(roundtrip_mtx, canonical_tx_hex, /*try_no_witness=*/true, /*try_witness=*/true));
+        assert(CTransaction{roundtrip_mtx} == CTransaction{mtx});
+    }
 }

src/test/fuzz/deserialize.cpp

@@ -225,14 +225,28 @@ FUZZ_TARGET_DESERIALIZE(blockheader_deserialize, {
 FUZZ_TARGET_DESERIALIZE(txundo_deserialize, {
     CTxUndo tu;
     DeserializeFromFuzzingInput(buffer, tu);
+    for (const auto& coin : tu.vprevout) {
+        assert(MoneyRange(coin.out.nValue));
+    }
 })
 FUZZ_TARGET_DESERIALIZE(blockundo_deserialize, {
     CBlockUndo bu;
     DeserializeFromFuzzingInput(buffer, bu);
+    for (const auto& tx_undo : bu.vtxundo) {
+        for (const auto& coin : tx_undo.vprevout) {
+            assert(MoneyRange(coin.out.nValue));
+        }
+    }
 })
 FUZZ_TARGET_DESERIALIZE(coins_deserialize, {
     Coin coin;
     DeserializeFromFuzzingInput(buffer, coin);
+    assert(MoneyRange(coin.out.nValue));
+    const Coin roundtrip{Deserialize<Coin>(Serialize(coin))};
+    assert(roundtrip.fCoinBase == coin.fCoinBase);
+    assert(roundtrip.nHeight == coin.nHeight);
+    assert(roundtrip.out.nValue == coin.out.nValue);
+    assert(roundtrip.out.scriptPubKey == coin.out.scriptPubKey);
 })
 FUZZ_TARGET(netaddr_deserialize, .init = initialize_deserialize)
 {
@@ -306,6 +320,11 @@ FUZZ_TARGET_DESERIALIZE(txoutcompressor_deserialize, {
     CTxOut to;
     auto toc = Using<TxOutCompression>(to);
     DeserializeFromFuzzingInput(buffer, toc);
+    assert(MoneyRange(to.nValue));
+    const auto params = [](auto& tx_out) { return Using<TxOutCompression>(tx_out); };
+    const CTxOut roundtrip{Deserialize<CTxOut>(Serialize(to, params), params)};
+    assert(roundtrip.nValue == to.nValue);
+    assert(roundtrip.scriptPubKey == to.scriptPubKey);
 })
 FUZZ_TARGET_DESERIALIZE(blocktransactions_deserialize, {
     BlockTransactions bt;

src/test/fuzz/integer.cpp

@@ -3,6 +3,7 @@
 // file COPYING or http://www.opensource.org/licenses/mit-license.php.
 
 #include <arith_uint256.h>
+#include <chainparams.h>
 #include <common/args.h>
 #include <common/system.h>
 #include <compressor.h>

src/test/fuzz/net_permissions.cpp

@@ -15,6 +15,41 @@
 #include <string>
 #include <vector>
 
+namespace {
+std::string JoinPermissions(const std::vector<std::string>& permissions)
+{
+    std::string result;
+    for (size_t i = 0; i < permissions.size(); ++i) {
+        if (i != 0) result += ",";
+        result += permissions[i];
+    }
+    return result;
+}
+
+std::string CanonicalWhitebindString(const NetWhitebindPermissions& permissions)
+{
+    if (NetPermissions::HasFlag(permissions.m_flags, NetPermissionFlags::Implicit)) {
+        return permissions.m_service.ToStringAddrPort();
+    }
+
+    return JoinPermissions(NetPermissions::ToStrings(permissions.m_flags)) + "@" + permissions.m_service.ToStringAddrPort();
+}
+
+std::string CanonicalWhitelistString(const NetWhitelistPermissions& permissions, ConnectionDirection connection_direction)
+{
+    if (NetPermissions::HasFlag(permissions.m_flags, NetPermissionFlags::Implicit)) {
+        return permissions.m_subnet.ToString();
+    }
+
+    std::vector<std::string> flags = NetPermissions::ToStrings(permissions.m_flags);
+    if (connection_direction != ConnectionDirection::In) {
+        if (connection_direction & ConnectionDirection::In) flags.emplace_back("in");
+        if (connection_direction & ConnectionDirection::Out) flags.emplace_back("out");
+    }
+    return JoinPermissions(flags) + "@" + permissions.m_subnet.ToString();
+}
+} // namespace
+
 FUZZ_TARGET(net_permissions)
 {
     FuzzedDataProvider fuzzed_data_provider(buffer.data(), buffer.size());
@@ -25,6 +60,13 @@ FUZZ_TARGET(net_permissions)
     bilingual_str error_net_whitebind_permissions;
     if (NetWhitebindPermissions::TryParse(s, net_whitebind_permissions, error_net_whitebind_permissions)) {
         (void)NetPermissions::ToStrings(net_whitebind_permissions.m_flags);
+        NetWhitebindPermissions roundtrip_whitebind_permissions;
+        bilingual_str roundtrip_error;
+        const std::string canonical_string = CanonicalWhitebindString(net_whitebind_permissions);
+        assert(NetWhitebindPermissions::TryParse(canonical_string, roundtrip_whitebind_permissions, roundtrip_error));
+        assert(roundtrip_whitebind_permissions.m_flags == net_whitebind_permissions.m_flags);
+        assert(roundtrip_whitebind_permissions.m_service == net_whitebind_permissions.m_service);
+
         (void)NetPermissions::AddFlag(net_whitebind_permissions.m_flags, net_permission_flags);
         assert(NetPermissions::HasFlag(net_whitebind_permissions.m_flags, net_permission_flags));
         (void)NetPermissions::ClearFlag(net_whitebind_permissions.m_flags, NetPermissionFlags::Implicit);
@@ -36,6 +78,15 @@ FUZZ_TARGET(net_permissions)
     bilingual_str error_net_whitelist_permissions;
     if (NetWhitelistPermissions::TryParse(s, net_whitelist_permissions, connection_direction, error_net_whitelist_permissions)) {
         (void)NetPermissions::ToStrings(net_whitelist_permissions.m_flags);
+        NetWhitelistPermissions roundtrip_whitelist_permissions;
+        ConnectionDirection roundtrip_connection_direction;
+        bilingual_str roundtrip_error;
+        const std::string canonical_string = CanonicalWhitelistString(net_whitelist_permissions, connection_direction);
+        assert(NetWhitelistPermissions::TryParse(canonical_string, roundtrip_whitelist_permissions, roundtrip_connection_direction, roundtrip_error));
+        assert(roundtrip_whitelist_permissions.m_flags == net_whitelist_permissions.m_flags);
+        assert(roundtrip_whitelist_permissions.m_subnet == net_whitelist_permissions.m_subnet);
+        assert(roundtrip_connection_direction == connection_direction);
+
         (void)NetPermissions::AddFlag(net_whitelist_permissions.m_flags, net_permission_flags);
         assert(NetPermissions::HasFlag(net_whitelist_permissions.m_flags, net_permission_flags));
         (void)NetPermissions::ClearFlag(net_whitelist_permissions.m_flags, NetPermissionFlags::Implicit);

src/test/fuzz/netaddress.cpp

@@ -2,6 +2,7 @@
 // Distributed under the MIT software license, see the accompanying
 // file COPYING or http://www.opensource.org/licenses/mit-license.php.
 
+#include <netbase.h>
 #include <netaddress.h>
 #include <test/fuzz/FuzzedDataProvider.h>
 #include <test/fuzz/fuzz.h>
@@ -10,8 +11,32 @@
 
 #include <cassert>
 #include <cstdint>
+#include <optional>
+#include <string>
 #include <vector>
 
+namespace {
+std::optional<CNetAddr> LookupHostRoundtrip(const CNetAddr& net_addr)
+{
+    const std::optional<CNetAddr> parsed = LookupHost(net_addr.ToStringAddr(), /*fAllowLookup=*/false);
+    if (!parsed.has_value()) return std::nullopt;
+    if (net_addr.IsCJDNS()) {
+        return static_cast<CNetAddr>(MaybeFlipIPv6toCJDNS(CService{*parsed, /*port=*/0}));
+    }
+    return parsed;
+}
+
+std::optional<CService> LookupServiceRoundtrip(const CService& service)
+{
+    const std::optional<CService> parsed = Lookup(service.ToStringAddrPort(), /*portDefault=*/0, /*fAllowLookup=*/false);
+    if (!parsed.has_value()) return std::nullopt;
+    if (service.IsCJDNS()) {
+        return MaybeFlipIPv6toCJDNS(*parsed);
+    }
+    return parsed;
+}
+} // namespace
+
 FUZZ_TARGET(netaddress)
 {
     SeedRandomStateForTest(SeedRand::ZEROS);
@@ -84,16 +109,30 @@ FUZZ_TARGET(netaddress)
         assert(net_addr.GetNetwork() == Network::NET_CJDNS);
     }
     (void)net_addr.IsValid();
-    (void)net_addr.ToStringAddr();
+    if (!net_addr.IsInternal()) {
+        const auto roundtrip_net_addr = LookupHostRoundtrip(net_addr);
+        assert(roundtrip_net_addr.has_value());
+        assert(*roundtrip_net_addr == net_addr);
+    }
 
     const CSubNet sub_net{net_addr, fuzzed_data_provider.ConsumeIntegral<uint8_t>()};
     (void)sub_net.IsValid();
-    (void)sub_net.ToString();
+    const auto assert_subnet_roundtrip = [](const CSubNet& subnet) {
+        if (!subnet.IsValid()) return;
+        const std::string subnet_string = subnet.ToString();
+        if (subnet_string.ends_with(".internal")) return;
+        assert(LookupSubNet(subnet_string) == subnet);
+    };
+    assert_subnet_roundtrip(sub_net);
 
     const CService service{net_addr, fuzzed_data_provider.ConsumeIntegral<uint16_t>()};
     (void)service.GetKey();
     (void)service.GetPort();
-    (void)service.ToStringAddrPort();
+    if (!service.IsInternal()) {
+        const auto roundtrip_service = LookupServiceRoundtrip(service);
+        assert(roundtrip_service.has_value());
+        assert(*roundtrip_service == service);
+    }
     (void)CServiceHash()(service);
     (void)CServiceHash(0, 0)(service);
 
@@ -111,6 +150,8 @@ FUZZ_TARGET(netaddress)
 
     const CSubNet sub_net_copy_1{net_addr, other_net_addr};
     const CSubNet sub_net_copy_2{net_addr};
+    assert_subnet_roundtrip(sub_net_copy_1);
+    assert_subnet_roundtrip(sub_net_copy_2);
 
     CNetAddr mutable_net_addr;
     mutable_net_addr.SetIP(net_addr);