Skip to content

Commit d98180a

Browse files
committed
Merge bitcoin/bitcoin#28419: fuzz: introduce and use ConsumePrivateKey helper
583af18 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 583af18 brunoerg: crACK 583af18 Tree-SHA512: 58a178432ba1eb0a2f7597b6700e96477e8b10f429ef642445a52db12e74d81aec307888315b772bfda9610f90df3e1d556cf024c2bef1d520303b12584feaaa
2 parents d2ccca2 + 583af18 commit d98180a

File tree

7 files changed

+23
-32
lines changed

7 files changed

+23
-32
lines changed

src/test/fuzz/bip324.cpp

+2-8
Original file line numberDiff line numberDiff line change
@@ -30,19 +30,13 @@ FUZZ_TARGET(bip324_cipher_roundtrip, .init=Initialize)
3030
// Load keys from fuzzer.
3131
FuzzedDataProvider provider(buffer.data(), buffer.size());
3232
// Initiator key
33-
auto init_key_data = provider.ConsumeBytes<unsigned char>(32);
34-
init_key_data.resize(32);
35-
CKey init_key;
36-
init_key.Set(init_key_data.begin(), init_key_data.end(), true);
33+
CKey init_key = ConsumePrivateKey(provider, /*compressed=*/true);
3734
if (!init_key.IsValid()) return;
3835
// Initiator entropy
3936
auto init_ent = provider.ConsumeBytes<std::byte>(32);
4037
init_ent.resize(32);
4138
// Responder key
42-
auto resp_key_data = provider.ConsumeBytes<unsigned char>(32);
43-
resp_key_data.resize(32);
44-
CKey resp_key;
45-
resp_key.Set(resp_key_data.begin(), resp_key_data.end(), true);
39+
CKey resp_key = ConsumePrivateKey(provider, /*compressed=*/true);
4640
if (!resp_key.IsValid()) return;
4741
// Responder entropy
4842
auto resp_ent = provider.ConsumeBytes<std::byte>(32);

src/test/fuzz/key.cpp

+4-12
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
#include <streams.h>
1818
#include <test/fuzz/FuzzedDataProvider.h>
1919
#include <test/fuzz/fuzz.h>
20+
#include <test/fuzz/util.h>
2021
#include <util/chaintype.h>
2122
#include <util/strencodings.h>
2223

@@ -312,10 +313,7 @@ FUZZ_TARGET(ellswift_roundtrip, .init = initialize_key)
312313
{
313314
FuzzedDataProvider fdp{buffer.data(), buffer.size()};
314315

315-
auto key_bytes = fdp.ConsumeBytes<uint8_t>(32);
316-
key_bytes.resize(32);
317-
CKey key;
318-
key.Set(key_bytes.begin(), key_bytes.end(), true);
316+
CKey key = ConsumePrivateKey(fdp, /*compressed=*/true);
319317
if (!key.IsValid()) return;
320318

321319
auto ent32 = fdp.ConsumeBytes<std::byte>(32);
@@ -332,17 +330,11 @@ FUZZ_TARGET(bip324_ecdh, .init = initialize_key)
332330
FuzzedDataProvider fdp{buffer.data(), buffer.size()};
333331

334332
// We generate private key, k1.
335-
auto rnd32 = fdp.ConsumeBytes<uint8_t>(32);
336-
rnd32.resize(32);
337-
CKey k1;
338-
k1.Set(rnd32.begin(), rnd32.end(), true);
333+
CKey k1 = ConsumePrivateKey(fdp, /*compressed=*/true);
339334
if (!k1.IsValid()) return;
340335

341336
// They generate private key, k2.
342-
rnd32 = fdp.ConsumeBytes<uint8_t>(32);
343-
rnd32.resize(32);
344-
CKey k2;
345-
k2.Set(rnd32.begin(), rnd32.end(), true);
337+
CKey k2 = ConsumePrivateKey(fdp, /*compressed=*/true);
346338
if (!k2.IsValid()) return;
347339

348340
// We construct an ellswift encoding for our key, k1_ellswift.

src/test/fuzz/message.cpp

+1-3
Original file line numberDiff line numberDiff line change
@@ -28,9 +28,7 @@ FUZZ_TARGET(message, .init = initialize_message)
2828
FuzzedDataProvider fuzzed_data_provider(buffer.data(), buffer.size());
2929
const std::string random_message = fuzzed_data_provider.ConsumeRandomLengthString(1024);
3030
{
31-
const std::vector<uint8_t> random_bytes = ConsumeRandomLengthByteVector(fuzzed_data_provider);
32-
CKey private_key;
33-
private_key.Set(random_bytes.begin(), random_bytes.end(), fuzzed_data_provider.ConsumeBool());
31+
CKey private_key = ConsumePrivateKey(fuzzed_data_provider);
3432
std::string signature;
3533
const bool message_signed = MessageSign(private_key, random_message, signature);
3634
if (private_key.IsValid()) {

src/test/fuzz/rpc.cpp

+2-6
Original file line numberDiff line numberDiff line change
@@ -285,19 +285,15 @@ std::string ConsumeScalarRPCArgument(FuzzedDataProvider& fuzzed_data_provider)
285285
},
286286
[&] {
287287
// base58 encoded key
288-
const std::vector<uint8_t> random_bytes = fuzzed_data_provider.ConsumeBytes<uint8_t>(32);
289-
CKey key;
290-
key.Set(random_bytes.begin(), random_bytes.end(), fuzzed_data_provider.ConsumeBool());
288+
CKey key = ConsumePrivateKey(fuzzed_data_provider);
291289
if (!key.IsValid()) {
292290
return;
293291
}
294292
r = EncodeSecret(key);
295293
},
296294
[&] {
297295
// hex encoded pubkey
298-
const std::vector<uint8_t> random_bytes = fuzzed_data_provider.ConsumeBytes<uint8_t>(32);
299-
CKey key;
300-
key.Set(random_bytes.begin(), random_bytes.end(), fuzzed_data_provider.ConsumeBool());
296+
CKey key = ConsumePrivateKey(fuzzed_data_provider);
301297
if (!key.IsValid()) {
302298
return;
303299
}

src/test/fuzz/script_sign.cpp

+1-3
Original file line numberDiff line numberDiff line change
@@ -79,9 +79,7 @@ FUZZ_TARGET(script_sign, .init = initialize_script_sign)
7979
}
8080

8181
FillableSigningProvider provider;
82-
CKey k;
83-
const std::vector<uint8_t> key_data = ConsumeRandomLengthByteVector(fuzzed_data_provider);
84-
k.Set(key_data.begin(), key_data.end(), fuzzed_data_provider.ConsumeBool());
82+
CKey k = ConsumePrivateKey(fuzzed_data_provider);
8583
if (k.IsValid()) {
8684
provider.AddKey(k);
8785
}

src/test/fuzz/util.cpp

+10
Original file line numberDiff line numberDiff line change
@@ -193,6 +193,16 @@ CTxDestination ConsumeTxDestination(FuzzedDataProvider& fuzzed_data_provider) no
193193
return tx_destination;
194194
}
195195

196+
CKey ConsumePrivateKey(FuzzedDataProvider& fuzzed_data_provider, std::optional<bool> compressed) noexcept
197+
{
198+
auto key_data = fuzzed_data_provider.ConsumeBytes<uint8_t>(32);
199+
key_data.resize(32);
200+
CKey key;
201+
bool compressed_value = compressed ? *compressed : fuzzed_data_provider.ConsumeBool();
202+
key.Set(key_data.begin(), key_data.end(), compressed_value);
203+
return key;
204+
}
205+
196206
bool ContainsSpentInput(const CTransaction& tx, const CCoinsViewCache& inputs) noexcept
197207
{
198208
for (const CTxIn& tx_in : tx.vin) {

src/test/fuzz/util.h

+3
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111
#include <compat/compat.h>
1212
#include <consensus/amount.h>
1313
#include <consensus/consensus.h>
14+
#include <key.h>
1415
#include <merkleblock.h>
1516
#include <primitives/transaction.h>
1617
#include <script/script.h>
@@ -165,6 +166,8 @@ template <typename WeakEnumType, size_t size>
165166

166167
[[nodiscard]] CTxDestination ConsumeTxDestination(FuzzedDataProvider& fuzzed_data_provider) noexcept;
167168

169+
[[nodiscard]] CKey ConsumePrivateKey(FuzzedDataProvider& fuzzed_data_provider, std::optional<bool> compressed = std::nullopt) noexcept;
170+
168171
template <typename T>
169172
[[nodiscard]] bool MultiplicationOverflow(const T i, const T j) noexcept
170173
{

0 commit comments

Comments
 (0)