diff options
author | MarcoFalke <falke.marco@gmail.com> | 2021-01-28 15:13:11 +0100 |
---|---|---|
committer | MarcoFalke <falke.marco@gmail.com> | 2021-01-28 15:13:25 +0100 |
commit | 4d5eaf7a90bc59d03795b9ade7c8c7bc5c5bf636 (patch) | |
tree | 5c3f7988e61988d3d83095e01561e5fb6f1b9d86 /src/test/fuzz/util.h | |
parent | 68692d33c6c51e4d0e873d2626ff25567432c71e (diff) | |
parent | fad3d7625aa1c2b6c343946e709e87e7168f9d9d (diff) | |
download | bitcoin-4d5eaf7a90bc59d03795b9ade7c8c7bc5c5bf636.tar.xz |
Merge #20995: fuzz: Avoid initializing version to less than MIN_PEER_PROTO_VERSION
fad3d7625aa1c2b6c343946e709e87e7168f9d9d fuzz: Avoid initializing version to less than MIN_PEER_PROTO_VERSION (MarcoFalke)
fa99e33aebed0109630474e11183b0726b410c2e fuzz: move-only FillNode implementation to cpp file (MarcoFalke)
Pull request description:
This fixes a fuzz bug introduced in #20881. Previously the nodes in the fuzz tests had their version initialized to a constant (`PROTOCOL_VERSION`). After #20881, the nodes have their version initialized to an arbitrary signed integer. This is problematic for several reasons:
* Both `nVersion` and `m_greatest_common_version` may be initialized to `0`. If a `version` message is processed, this leads to a crash, because `m_greatest_common_version` must be `INIT_PROTO_VERSION` while the `version` message is processed. See #20138
* The "valid" range for `nVersion` is `[MIN_PEER_PROTO_VERSION, std::numeric_limits<int32_t>::max()]` (see check in net_processing)
* The "valid" range for `m_greatest_common_version` is `std::min(nVersion, PROTOCOL_VERSION)` (see net_processing)
Fix all issues by initializing `nVersion` and `m_greatest_common_version` to their valid ranges.
-----
The crashers, if someone wants to try this at home:
```
( echo 'dmVyc2lvbgAWFhYWFhYWFhYWFhYWFhYWFhYWFhZp/29uAPX//xYWFhYWFhYWFhYWFhYWFhYWFhYW
FhYWFhYWaW9uAOr1//8WFhYWFha0ZXJzaW9uAPX//wAAAAAAABAAAAAAAAAAAAC0ZXJzaW9uAPX/
/wBPT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT08AAAAAABAAAAAAAAAAAAAAAAAA
AAAAAAAAAAAAAAAAAAAAAAAAAAAACgAAAAAAAAAAgAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA
AAAAAAAAAAAAAAB2ZXJzaW9uAACDJIO9vXYKAAAAAAAAAAAAAAAAAAAAAAB2ZfS1qmu1qhUVFWs=' | base64 --decode > /tmp/a ) && FUZZ=process_message_version ./src/test/fuzz/fuzz /tmp/a
```
```
( echo 'dmVyc2lvbgD//wAhTmiqN///NDcAAACENDL/iv//8DYAAHL///////79/RtcAJqamhqa/QEAAAD/
///+/f1oZWFkZXJzAAAAAM8BAAAAIAYibkYRGgtZyq8SaGVhZGVycwAAAAD/NDcAAACENDL/iv//
8DYAAHL///////79/RtcAJqamhqa/QEAAAD////+/f1oZWFkZXJzAAAAAM8BAAAAIAYibkYRGgtZ
yq8SaGVhZGVycwAAAADPAQAAACAGIm5GERoLWS1wb3J061u/KMNPOkwFXqZ///b5IgIAAD+5ubkb
XD5hZGRyAJqamhqasP0BAAAAAAAAAP0BAAAAIf39/R0dHQAAAAAAMgAA///7//+gXqZ///b5IgIA
AD+5ubm5ubm5AAAAAAAAAAAAAAAAAAAAAAAAAACAAAAAAAAAFgAAAAAAAAAAAAlBmv39/f1/f39B
f39hZGRyAG5vAACaLgAdGzY2zwEAAAAgBiJuRhEaC1ktcG9ydOtbvyjDTzpMBV6mf//2+SICAAA/
ubm5G1w+YWRkcgCampoamrD9AQAAAAAAAAD9AQAAACH9/f0dHR0AAAAAADIAAP//+///oF6mf//2
+SICAAA/ubm5ubm5uQAAAAAAAAAAAAAAAAAAAAAAAAAAgAAAAAAAABYAAAAAAAAAAAAJQZr9/f39
f39/QX9/YWRkcgBubwAAmi4AHRs2NjY2NjY2NjYCAgI2NgIA/f39/f39Nv39/TUmABxc' | base64 --decode > /tmp/b ) && FUZZ=process_message_version ./src/test/fuzz/fuzz /tmp/b
```
ACKs for top commit:
practicalswift:
cr ACK fad3d7625aa1c2b6c343946e709e87e7168f9d9d
Tree-SHA512: ea64ee99b94d8e619e3949d2d21252c1236412c0e40f44f2b73595ca70cd2da0bdab005fb1a54f65fb291e7b07fdd33577ce4a3a078ca933246b511ebcb0e52a
Diffstat (limited to 'src/test/fuzz/util.h')
-rw-r--r-- | src/test/fuzz/util.h | 21 |
1 files changed, 3 insertions, 18 deletions
diff --git a/src/test/fuzz/util.h b/src/test/fuzz/util.h index 5b8eacf051..a12dc9a56d 100644 --- a/src/test/fuzz/util.h +++ b/src/test/fuzz/util.h @@ -320,24 +320,9 @@ auto ConsumeNode(FuzzedDataProvider& fuzzed_data_provider, const std::optional<N return CNode{node_id, local_services, socket, address, keyed_net_group, local_host_nonce, addr_bind, addr_name, conn_type, inbound_onion}; } } -inline std::unique_ptr<CNode> ConsumeNodeAsUniquePtr(FuzzedDataProvider& fdp, const std::optional<NodeId>& node_id_in = nullopt) { return ConsumeNode<true>(fdp, node_id_in); } - -inline void FillNode(FuzzedDataProvider& fuzzed_data_provider, CNode& node, const std::optional<int32_t>& version_in = std::nullopt) noexcept -{ - const ServiceFlags remote_services = ConsumeWeakEnum(fuzzed_data_provider, ALL_SERVICE_FLAGS); - const NetPermissionFlags permission_flags = ConsumeWeakEnum(fuzzed_data_provider, ALL_NET_PERMISSION_FLAGS); - const int32_t version = version_in.value_or(fuzzed_data_provider.ConsumeIntegral<int32_t>()); - const bool filter_txs = fuzzed_data_provider.ConsumeBool(); - - node.nServices = remote_services; - node.m_permissionFlags = permission_flags; - node.nVersion = version; - node.SetCommonVersion(version); - if (node.m_tx_relay != nullptr) { - LOCK(node.m_tx_relay->cs_filter); - node.m_tx_relay->fRelayTxes = filter_txs; - } -} +inline std::unique_ptr<CNode> ConsumeNodeAsUniquePtr(FuzzedDataProvider& fdp, const std::optional<NodeId>& node_id_in = std::nullopt) { return ConsumeNode<true>(fdp, node_id_in); } + +void FillNode(FuzzedDataProvider& fuzzed_data_provider, CNode& node, bool init_version) noexcept; template <class T = const BasicTestingSetup> std::unique_ptr<T> MakeFuzzingContext(const std::string& chain_name = CBaseChainParams::REGTEST, const std::vector<const char*>& extra_args = {}) |