diff options
author | fanquake <fanquake@gmail.com> | 2023-09-07 11:15:52 +0100 |
---|---|---|
committer | fanquake <fanquake@gmail.com> | 2023-09-07 11:20:12 +0100 |
commit | d98180a969db2dc16cd2ebba12bad3c66acce700 (patch) | |
tree | f974132e8f6d628514f2518f3a1eacd0ab4d1a8b /src | |
parent | d2ccca253f4294b8f480e1d192913f4985a1af08 (diff) | |
parent | 583af18fd1d0bda5a6a1d0403ffc498a512a546d (diff) | |
download | bitcoin-d98180a969db2dc16cd2ebba12bad3c66acce700.tar.xz |
Merge bitcoin/bitcoin#28419: fuzz: introduce and use `ConsumePrivateKey` helper
583af18fd1d0bda5a6a1d0403ffc498a512a546d fuzz: introduce and use `ConsumePrivateKey` helper (Sebastian Falbesoner)
Pull request description:
In the course of reviewing BIP324 related PRs I noticed a frequent pattern of creating private keys (`CKey` instances) with data consumed from the fuzz data provider:
```
auto key_data = provider.ConsumeBytes<unsigned char>(32);
key_data.resize(32);
CKey key;
key.Set(key_data.begin(), key_data.end(), /*fCompressedIn=*/true);
```
This PR introduces a corresponding helper `ConsumePrivateKey` in order to deduplicate code. The compressed flag can either be set to a fixed value, or, if `std::nullopt` is passed (=default), is also consumed from the fuzz data provider via `.ConsumeBool()`.
Note that this is not a pure refactor, as some of the replaced call-sites previously consumed a random length (`ConsumeRandomLengthByteVector`) instead of a fixed size of 32 bytes for key data. As far as I can see, there is not much value in using a random size, as in all those cases we can only proceed or do something useful with a valid private key, and key data with sizes other than 32 bytes always lead to invalid keys.
ACKs for top commit:
sipa:
utACK 583af18fd1d0bda5a6a1d0403ffc498a512a546d
brunoerg:
crACK 583af18fd1d0bda5a6a1d0403ffc498a512a546d
Tree-SHA512: 58a178432ba1eb0a2f7597b6700e96477e8b10f429ef642445a52db12e74d81aec307888315b772bfda9610f90df3e1d556cf024c2bef1d520303b12584feaaa
Diffstat (limited to 'src')
-rw-r--r-- | src/test/fuzz/bip324.cpp | 10 | ||||
-rw-r--r-- | src/test/fuzz/key.cpp | 16 | ||||
-rw-r--r-- | src/test/fuzz/message.cpp | 4 | ||||
-rw-r--r-- | src/test/fuzz/rpc.cpp | 8 | ||||
-rw-r--r-- | src/test/fuzz/script_sign.cpp | 4 | ||||
-rw-r--r-- | src/test/fuzz/util.cpp | 10 | ||||
-rw-r--r-- | src/test/fuzz/util.h | 3 |
7 files changed, 23 insertions, 32 deletions
diff --git a/src/test/fuzz/bip324.cpp b/src/test/fuzz/bip324.cpp index 98ac10e364..e5ed9bfd52 100644 --- a/src/test/fuzz/bip324.cpp +++ b/src/test/fuzz/bip324.cpp @@ -30,19 +30,13 @@ FUZZ_TARGET(bip324_cipher_roundtrip, .init=Initialize) // Load keys from fuzzer. FuzzedDataProvider provider(buffer.data(), buffer.size()); // Initiator key - auto init_key_data = provider.ConsumeBytes<unsigned char>(32); - init_key_data.resize(32); - CKey init_key; - init_key.Set(init_key_data.begin(), init_key_data.end(), true); + CKey init_key = ConsumePrivateKey(provider, /*compressed=*/true); if (!init_key.IsValid()) return; // Initiator entropy auto init_ent = provider.ConsumeBytes<std::byte>(32); init_ent.resize(32); // Responder key - auto resp_key_data = provider.ConsumeBytes<unsigned char>(32); - resp_key_data.resize(32); - CKey resp_key; - resp_key.Set(resp_key_data.begin(), resp_key_data.end(), true); + CKey resp_key = ConsumePrivateKey(provider, /*compressed=*/true); if (!resp_key.IsValid()) return; // Responder entropy auto resp_ent = provider.ConsumeBytes<std::byte>(32); diff --git a/src/test/fuzz/key.cpp b/src/test/fuzz/key.cpp index a5a579d982..60f4081432 100644 --- a/src/test/fuzz/key.cpp +++ b/src/test/fuzz/key.cpp @@ -17,6 +17,7 @@ #include <streams.h> #include <test/fuzz/FuzzedDataProvider.h> #include <test/fuzz/fuzz.h> +#include <test/fuzz/util.h> #include <util/chaintype.h> #include <util/strencodings.h> @@ -312,10 +313,7 @@ FUZZ_TARGET(ellswift_roundtrip, .init = initialize_key) { FuzzedDataProvider fdp{buffer.data(), buffer.size()}; - auto key_bytes = fdp.ConsumeBytes<uint8_t>(32); - key_bytes.resize(32); - CKey key; - key.Set(key_bytes.begin(), key_bytes.end(), true); + CKey key = ConsumePrivateKey(fdp, /*compressed=*/true); if (!key.IsValid()) return; auto ent32 = fdp.ConsumeBytes<std::byte>(32); @@ -332,17 +330,11 @@ FUZZ_TARGET(bip324_ecdh, .init = initialize_key) FuzzedDataProvider fdp{buffer.data(), buffer.size()}; // We generate private key, k1. - auto rnd32 = fdp.ConsumeBytes<uint8_t>(32); - rnd32.resize(32); - CKey k1; - k1.Set(rnd32.begin(), rnd32.end(), true); + CKey k1 = ConsumePrivateKey(fdp, /*compressed=*/true); if (!k1.IsValid()) return; // They generate private key, k2. - rnd32 = fdp.ConsumeBytes<uint8_t>(32); - rnd32.resize(32); - CKey k2; - k2.Set(rnd32.begin(), rnd32.end(), true); + CKey k2 = ConsumePrivateKey(fdp, /*compressed=*/true); if (!k2.IsValid()) return; // We construct an ellswift encoding for our key, k1_ellswift. diff --git a/src/test/fuzz/message.cpp b/src/test/fuzz/message.cpp index f839f9e326..b5c95441f8 100644 --- a/src/test/fuzz/message.cpp +++ b/src/test/fuzz/message.cpp @@ -28,9 +28,7 @@ FUZZ_TARGET(message, .init = initialize_message) FuzzedDataProvider fuzzed_data_provider(buffer.data(), buffer.size()); const std::string random_message = fuzzed_data_provider.ConsumeRandomLengthString(1024); { - const std::vector<uint8_t> random_bytes = ConsumeRandomLengthByteVector(fuzzed_data_provider); - CKey private_key; - private_key.Set(random_bytes.begin(), random_bytes.end(), fuzzed_data_provider.ConsumeBool()); + CKey private_key = ConsumePrivateKey(fuzzed_data_provider); std::string signature; const bool message_signed = MessageSign(private_key, random_message, signature); if (private_key.IsValid()) { diff --git a/src/test/fuzz/rpc.cpp b/src/test/fuzz/rpc.cpp index 24ec0e4a73..74f06b481a 100644 --- a/src/test/fuzz/rpc.cpp +++ b/src/test/fuzz/rpc.cpp @@ -285,9 +285,7 @@ std::string ConsumeScalarRPCArgument(FuzzedDataProvider& fuzzed_data_provider) }, [&] { // base58 encoded key - const std::vector<uint8_t> random_bytes = fuzzed_data_provider.ConsumeBytes<uint8_t>(32); - CKey key; - key.Set(random_bytes.begin(), random_bytes.end(), fuzzed_data_provider.ConsumeBool()); + CKey key = ConsumePrivateKey(fuzzed_data_provider); if (!key.IsValid()) { return; } @@ -295,9 +293,7 @@ std::string ConsumeScalarRPCArgument(FuzzedDataProvider& fuzzed_data_provider) }, [&] { // hex encoded pubkey - const std::vector<uint8_t> random_bytes = fuzzed_data_provider.ConsumeBytes<uint8_t>(32); - CKey key; - key.Set(random_bytes.begin(), random_bytes.end(), fuzzed_data_provider.ConsumeBool()); + CKey key = ConsumePrivateKey(fuzzed_data_provider); if (!key.IsValid()) { return; } diff --git a/src/test/fuzz/script_sign.cpp b/src/test/fuzz/script_sign.cpp index cec98432e1..b2d7d68fb4 100644 --- a/src/test/fuzz/script_sign.cpp +++ b/src/test/fuzz/script_sign.cpp @@ -79,9 +79,7 @@ FUZZ_TARGET(script_sign, .init = initialize_script_sign) } FillableSigningProvider provider; - CKey k; - const std::vector<uint8_t> key_data = ConsumeRandomLengthByteVector(fuzzed_data_provider); - k.Set(key_data.begin(), key_data.end(), fuzzed_data_provider.ConsumeBool()); + CKey k = ConsumePrivateKey(fuzzed_data_provider); if (k.IsValid()) { provider.AddKey(k); } diff --git a/src/test/fuzz/util.cpp b/src/test/fuzz/util.cpp index 9da84fe90e..cffe3e1f06 100644 --- a/src/test/fuzz/util.cpp +++ b/src/test/fuzz/util.cpp @@ -193,6 +193,16 @@ CTxDestination ConsumeTxDestination(FuzzedDataProvider& fuzzed_data_provider) no return tx_destination; } +CKey ConsumePrivateKey(FuzzedDataProvider& fuzzed_data_provider, std::optional<bool> compressed) noexcept +{ + auto key_data = fuzzed_data_provider.ConsumeBytes<uint8_t>(32); + key_data.resize(32); + CKey key; + bool compressed_value = compressed ? *compressed : fuzzed_data_provider.ConsumeBool(); + key.Set(key_data.begin(), key_data.end(), compressed_value); + return key; +} + bool ContainsSpentInput(const CTransaction& tx, const CCoinsViewCache& inputs) noexcept { for (const CTxIn& tx_in : tx.vin) { diff --git a/src/test/fuzz/util.h b/src/test/fuzz/util.h index 5d27d2a180..4c2176f05b 100644 --- a/src/test/fuzz/util.h +++ b/src/test/fuzz/util.h @@ -11,6 +11,7 @@ #include <compat/compat.h> #include <consensus/amount.h> #include <consensus/consensus.h> +#include <key.h> #include <merkleblock.h> #include <primitives/transaction.h> #include <script/script.h> @@ -165,6 +166,8 @@ template <typename WeakEnumType, size_t size> [[nodiscard]] CTxDestination ConsumeTxDestination(FuzzedDataProvider& fuzzed_data_provider) noexcept; +[[nodiscard]] CKey ConsumePrivateKey(FuzzedDataProvider& fuzzed_data_provider, std::optional<bool> compressed = std::nullopt) noexcept; + template <typename T> [[nodiscard]] bool MultiplicationOverflow(const T i, const T j) noexcept { |