test: add BLS operations and IES encryption fuzz targets#7167
test: add BLS operations and IES encryption fuzz targets#7167thepastaclaw wants to merge 5 commits intodashpay:developfrom
Conversation
Add two new fuzz targets for Dash-specific BLS cryptographic operations: bls_operations: Fuzzes BLS key generation, signing, verification, aggregation (pubkeys, signatures, secret keys), threshold operations (key shares, signature recovery), DH key exchange, and aggregated signature verification. Tests both legacy and basic BLS schemes. bls_ies: Fuzzes BLS Integrated Encryption Scheme operations including CBLSIESEncryptedBlob and CBLSIESMultiRecipientBlobs decrypt with fuzzed inputs, and encrypt-then-decrypt roundtrip with matching keys. Both targets carefully filter fuzzed inputs to avoid UB from accessing invalid BLS object internals (e.g., AggregateInsecure and VerifySecureAggregated access .impl without validity checks). Part of the Dash Core Fuzzing Initiative (Phase 3).
|
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. WalkthroughA new fuzz test source is added to the build and a new fuzz test file implements comprehensive fuzzing for BLS and IES code paths. The file provides helpers to construct fuzzed CBLSSecretKey, CBLSPublicKey, CBLSSignature, and uint256 values, an initialization function that calls BLSInit(), and two fuzz targets: one exercising key generation, signing, verification, aggregation, threshold schemes, and DH-derived operations, and another exercising CBLSIES* serialization, encryption, and decryption flows with fuzzed inputs. Sequence Diagram(s)sequenceDiagram
participant Fuzz as Fuzz Harness
participant Init as BLSInit
participant BLS as BLS Library
participant IES as IES Module
Fuzz->>Init: call initialize_bls_operations()
Init->>BLS: BLSInit()
Fuzz->>BLS: MakeSecretKey / MakePublicKey / MakeSignature / MakeHash
alt bls_operations flow
Fuzz->>BLS: KeyGen / Sign
BLS-->>Fuzz: Signature
Fuzz->>BLS: Verify / AggregateKeys / AggregateSigs / ThresholdOps
BLS-->>Fuzz: Verification results / Aggregated blobs
end
alt bls_ies flow
Fuzz->>IES: Deserialize CBLSIESEncryptedBlob / MultiRecipientBlobs
IES-->>Fuzz: Parsed blobs
Fuzz->>IES: Decrypt (single / multi) / Encrypt roundtrip
IES-->>Fuzz: Plaintext / Re-encrypted blobs
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
thepastaclaw
changed the title
Required for clang-format CI enforcement on Dash-specific files.
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
| CBLSSecretKey MakeSecretKey(FuzzedDataProvider& fuzzed_data_provider) | ||
| { | ||
| auto bytes = fuzzed_data_provider.ConsumeBytes<uint8_t>(CBLSSecretKey::SerSize); | ||
| bytes.resize(CBLSSecretKey::SerSize); |
There was a problem hiding this comment.
isn't this resize redundant? don't we consume the amount requested above?
test/util/data/non-backported.txt
Outdated
| src/stats/*.h | ||
| src/test/block_reward_reallocation_tests.cpp | ||
| src/test/bls_tests.cpp | ||
| src/test/fuzz/bls_operations.cppsrc/test/bls_tests.cpp |
There was a problem hiding this comment.
missing newlines / corrupted
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@test/util/data/non-backported.txt`:
- Line 53: The entry "src/test/fuzz/bls_operations.cppsrc/test/bls_tests.cpp" is
a concatenated invalid path; split it into two separate lines
"src/test/fuzz/bls_operations.cpp" and "src/test/bls_tests.cpp" in
test/util/data/non-backported.txt, placing "src/test/fuzz/bls_operations.cpp" in
the alphabetically correct spot (after the existing src/test/evo* entries) so
clang-format picks up the new fuzz target and preserve the existing
src/test/bls_tests.cpp entry.
459af32 to
6d30217
Compare
|
Re: missing newlines — fixed in 4ac72a0, pushed right around when you commented. Re: |
2 participants
Summary
Add two new fuzz targets for Dash-specific BLS cryptographic operations (Phase 3 of the Dash Core Fuzzing Initiative).
bls_operationstargetFuzzes the core BLS operations that underpin Dash's LLMQ threshold signing, ChainLocks, and InstantSend:
CBLSSecretKeyfrom fuzzed bytes +GetPublicKey()derivationSign()+VerifyInsecure()with both legacy and basic schemesAggregateInsecure()for public keys, signatures, and secret keysSecretKeyShare(),PublicKeyShare(), signatureRecover()DHKeyExchange()with fuzzed inputsVerifyInsecureAggregated()andVerifySecureAggregated()Tests both legacy (
LegacySchemeMPL) and basic (BasicSchemeMPL) BLS schemes viabls::bls_legacy_scheme.bls_iestargetFuzzes the BLS Integrated Encryption Scheme used for encrypted DKG contributions:
CBLSIESEncryptedBlob— decrypt with fuzzed keys and ciphertextCBLSIESMultiRecipientBlobs— decrypt multiple fuzzed blobsSafety
Both targets carefully filter fuzzed inputs to avoid triggering assertions or UB from accessing invalid BLS object internals — specifically:
AggregateInsecure(Span<..>)accesses.implwithout validity checks → filter to valid keys/sigsVerifySecureAggregated()accessespk.impldirectly → filter to valid public keysVerifyInsecureAggregated()hasassert(!pubKeys.empty())→ ensure non-empty after filteringSecretKeyShare(),PublicKeyShare(),Recover()validate internally → safe with any inputValidation
Environment: Guix VM (Ubuntu 24.04, aarch64, clang 18)
Build:
./configure --enable-fuzz --with-sanitizers=address,fuzzer,undefined --without-gui --disable-wallet --disable-stacktraces && make -j10bls_ies— ✅ PASSbls_operations—The leak appears to be in libdashbls internals (
std::vector<CBLSSecretKey>::push_back→ realloc path), not in the fuzz harness. This is a pre-existing leak in the BLS library, not introduced by this PR. The fuzz target successfully surfaces it for investigation.Context
Part of the Dash Core Fuzzing Initiative — Phase 3 (Functional Fuzz Targets). BLS operations are priority #1 in Phase 3 because they are consensus-critical: every ChainLock, InstantSend lock, and LLMQ commitment depends on BLS signatures.
Related PRs: