fuzz: Cover block filter P2P messages #36337

maflcko commented at 5:34 PM on September 25, 2026: member

The BIP157 P2P messages are not covered by any fuzz targets at all.

Fix this to cover them via the P2P targets.

Should be possible to test by verifying the integer sanitizer is triggered (https://github.com/bitcoin/bitcoin/pull/36321#issuecomment-5810977529):

$ echo 'FwAAAAEAAABcX2dldGNmY2hlY2twdAAftOvdD0sFqXEx6US5VIknlsOJqZ5goMwtmwZBPdCxQVxf
AfkGKP39/f0v/Q/9E1AP/f2ybAT1Af39A/399wEAuXEAAAAAAAAAAAB1z3QAbgAAQAHm/f39/f39
/f39/S8/D/0B/f3IAAcBBWNmaf8DAAAA' | base64 --decode > /tmp/msg.bin

$ UBSAN_OPTIONS="suppressions=$(pwd)/test/sanitizer_suppressions/ubsan:print_stacktrace=0:halt_on_error=0:report_error_type=1" FUZZ=process_messages ./bld-cmake/bin/fuzz /tmp/msg.bin 

./bld-cmake/bin/fuzz: Running 1 inputs 1 time(s) each.
/src/net_processing.cpp:3662:33: runtime error: unsigned integer overflow: 0 - 1 cannot be represented in type 'size_type' (aka 'unsigned long')
/src/net_processing.cpp:3662:18: runtime error: implicit conversion from type 'size_type' (aka 'unsigned long') of value 18446744073709551615 (64-bit, unsigned) to type 'int' changed the value to -1 (32-bit, signed)

DrahtBot added the label Fuzzing on Sep 25, 2026

DrahtBot commented at 5:34 PM on September 25, 2026: contributor

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

External sites

Code coverage & benchmarks: https://corecheck.dev/bitcoin/bitcoin/pulls/36337
Ralph review <img src=https://review.fish.foo/assets/bitcoin/bitcoin/36337.png alt="Ralph review status"> https://git.fish.foo/bitcoin/bitcoin/pulls/36337#author-ralph

Reviews

See the guideline and AI policy for information on the review process.

Type	Reviewers
ACK	marcofleon, Crypt-iQ

If your review is incorrectly listed, please copy-paste <code></code> into the comment that the bot should ignore.

Conflicts

No conflicts as of last run.

LLM Linter (✨ experimental)

Possible places where named args for integral literals may be used (e.g. func(x, /*named_arg=*/0) in C++, and func(x, named_arg=0) in Python):

src/test/util/validation.cpp: InitBlockFilterIndex(..., BlockFilterType::BASIC, 1_MiB, /*f_memory=*/true, /*f_wipe=*/true)

2026-10-08 05:10:07

DrahtBot added the label CI failed on Sep 25, 2026

DrahtBot commented at 6:34 PM on September 25, 2026: contributor

🚧 At least one of the CI tasks failed. Task MSan, fuzz: https://github.com/bitcoin/bitcoin/actions/runs/36168019330/job/108180422242 LLM reason (✨ experimental): MemorySanitizer (fuzz test) reported a use of an uninitialized value in BaseIndex::Stop() (base.cpp:479), causing exit code 1 and the CI failure.

<details><summary>Hints</summary>

Try to run the tests locally, according to the documentation. However, a CI failure may still happen due to a number of reasons, for example:

Possibly due to a silent merge conflict (the changes in this pull request being incompatible with the current code in the target branch). If so, make sure to rebase on the latest commit of the target branch.
A sanitizer issue, which can only be found by compiling with the sanitizer and running the affected test.
An intermittent issue.

Leave a comment here, if you need help tracking down a confusing failure.

</details>

Crypt-iQ commented at 3:37 PM on September 26, 2026: contributor

I think CI failures are because in process_message(s) the static testing_setup is destructed before static g_filter_indexes so BaseIndex::Stop will try to access testing_setup.m_node.

in src/test/util/validation.cpp:150 in fa488f53c3 outdated

 145 | +        Assert(index.Init());
 146 | +        index.Sync();
 147 | +        // The index serves the static setup chain only. Leaving it registered
 148 | +        // deadlocks with ImmediateBackgroundTaskRunner when a fuzzed block
 149 | +        // connects under cs_main.
 150 | +        index.Stop();

Crypt-iQ commented at 4:28 PM on September 26, 2026:

Can you explain why it deadlocks? I couldn't figure this bit out

maflcko commented at 4:29 PM on September 29, 2026:

Sure. I guess the "fuzzed block" explanation is a bit confusing, because this is a general unit test utility, not limited to fuzzing.

The idea is:

block connection events happens under cs_main: https://github.com/bitcoin/bitcoin/blob/3b9768a07aee6851de24f9c23bcdeeac2d872d23/src/validation.cpp#L3367-L3408
With ImmediateBackgroundTaskRunner a new thread is spawned to handle it, which obviously can't take cs_main again.
Thus, https://github.com/bitcoin/bitcoin/blob/3b9768a07aee6851de24f9c23bcdeeac2d872d23/src/kernel/chain.cpp#L25 will deadlock via callback thread: BaseIndex::BlockConnected() → ProcessBlock() → kernel::MakeBlockInfo() → tries to lock cs_main

I guess in theory one could use ImmediateTaskRunner (reverting #36046), but that would bring back the issues from there.

maflcko commented at 4:30 PM on September 29, 2026:

I think CI failures are because in process_message(s) the static testing_setup is destructed before static g_filter_indexes so BaseIndex::Stop will try to access testing_setup.m_node.

Thx, fixed by destroying the indexes in the testing dtor, which seems useful generally, unrelated to the changes here.

fanquake commented at 12:24 PM on September 29, 2026: member

cc @marcofleon

DrahtBot removed the label CI failed on Sep 29, 2026

Crypt-iQ commented at 5:37 PM on October 1, 2026: contributor

crACK fa47c22f7a9e148c6e67ac726ccefb7ceda44cef

Checked this didn't introduce non-determinism using qa-assets corpus. I think new seeds for process_message(s) are needed since no seeds currently reach past PrepareBlockFilterRequest.

maflcko commented at 9:20 AM on October 2, 2026: member

I think new seeds for process_message(s) are needed since no seeds currently reach past PrepareBlockFilterRequest.

I had my LLM-based fuzz engine (based on inverse fdp: https://github.com/maflcko/DrahtBot/blob/main/inverse_fdp/build.rs#L1) create the one in the pull description, and I can submit some more later

Crypt-iQ commented at 12:42 PM on October 2, 2026: contributor

I had my LLM-based fuzz engine (based on inverse fdp: https://github.com/maflcko/DrahtBot/blob/main/inverse_fdp/build.rs#L1)

Might be off-topic, but could you explain how this works? I've been trying to come up with ways to have LLMs help with fuzzing and haven't come up with anything that I like. Does the LLM see coverage is stuck and then generate a test case with inverse-fdp to unblock the fuzzer?

maflcko commented at 1:34 PM on October 2, 2026: member

Might be off-topic, but could you explain how this works?

It is pretty primitive. I selected a fuzz input manually to start with and then asked the LLM to modify it.

<details><summary>LLM prompt (experimental)</summary>

› Ok, so the goal is to reach coverage in net_processing (see the diff, I've marked it, the goal is to reach that)


  I've also applied the inverse fdp from DrahtBot (you can read the repo there)



  Compiling and running the fuzz clang exe will print the fuzz seed.


  A good candidate, which hits the prior line is:


  ifdp.push_integral_in_range(1i32, 1i32, 3i32, );
  ifdp.push_integral_in_range(4395519861509i64, 0i64, 9223372036854775807i64, );
  ifdp.push_integral_in_range(4294967295u32, 4294967294u32, 4294967295u32, );
  ifdp.push_integral_in_range(7u8, 0u8, 255u8, );
  ifdp.push_integral_in_range(0u64, 0u64, 5u64, );
  ifdp.push_bytes(&[0x17,0x0,0x0,0x0,]); // (len=4)
  ifdp.push_integral_in_range(51453u16, 0u16, 65535u16, );
  ifdp.push_integral_in_range(253u8, 0u8, 255u8, );
  ifdp.push_integral_in_range(1u64, 0u64, 6u64, );
  ifdp.push_integral_in_range(4245634863u32, 0u32, 4294967295u32, );
  ifdp.push_integral_in_range(18302063728033398269u64, 0u64, 18446744073709551615u64, );
  ifdp.push_integral_in_range(18302037353746661486u64, 0u64, 18446744073709551615u64, );
  ifdp.push_integral_in_range(0u64, 0u64, 5u64, );
  ifdp.push_bytes(&[0x1,0x0,0x0,0x0,]); // (len=4)
  ifdp.push_integral_in_range(29903u16, 0u16, 65535u16, );
  ifdp.push_integral_in_range(117u8, 0u8, 255u8, );
  ifdp.push_integral_in_range(0u64, 0u64, 6u64, );
  ifdp.push_integral_in_range(0u32, 0u32, 4294967295u32, );
  ifdp.push_str_u8(&[]); // (len=0), Limit: 64
  ifdp.push_integral_in_range(0u64, 0u64, 6u64, );
  ifdp.push_integral_in_range(0u8, 0u8, 255u8, );
  ifdp.push_integral_in_range(125039383017469u64, 0u64, 18446744073709551615u64, );
  ifdp.push_integral_in_range(253u8, 0u8, 255u8, );
  ifdp.push_integral_in_range(3u64, 0u64, 9u64, );
  ifdp.push_integral_in_range(253u8, 0u8, 255u8, );
  ifdp.push_integral_in_range(253u8, 0u8, 255u8, );
  ifdp.push_integral_in_range(1u64, 0u64, 6u64, );
  ifdp.push_integral_in_range(245u8, 0u8, 255u8, );
  ifdp.push_integral_in_range(4u64, 0u64, 6u64, );
  ifdp.push_integral_in_range(1823701557i32, 31800i32, 2147483647i32, );
  ifdp.push_integral_in_range(15u8, 0u8, 255u8, );
  ifdp.push_integral_in_range(80u8, 0u8, 255u8, );
  ifdp.push_integral_in_range(18i64, -1i64, 24i64, );
  ifdp.push_integral_in_range(253u8, 0u8, 255u8, );
  ifdp.push_integral_in_range(15u8, 0u8, 255u8, );
  ifdp.push_integral_in_range(253u8, 0u8, 255u8, );
  ifdp.push_integral_in_range(47u8, 0u8, 255u8, );
  ifdp.push_integral_in_range(253u8, 0u8, 255u8, );
  ifdp.push_integral_in_range(253u8, 0u8, 255u8, );
  ifdp.push_integral_in_range(253u8, 0u8, 255u8, );
  ifdp.push_integral_in_range(253u8, 0u8, 255u8, );
  ifdp.push_bytes(&[0x67,0x65,0x74,0x63,0x66,0x63,0x68,0x65,0x63,0x6b,0x70,0x74,]); // (len=12)
  ifdp.push_integral_in_range(1618230402i64, 946684801i64, 4133980799i64, );
  ifdp.push_str_u8(&[0x0,0x0,0x0,0x0,0x1,0x0,0x0,0x40,0x0,0x95,0x0,0x1,0x0,0x40,0x1,0x0,0x0,0xc0,0x73,0x65,0x72,0x63,0x27,0x8,0x46,0xd7,0xfd,0xfd,0x50,0xfd,0x78,0x6e,0x0,]); // (len=33), Limit: 4000000; not terminated!
  consider ifdp.push_str_u8_no_term instead?







  Use the inverse fdp, compile and run it, to reach the prior line, then smartly modify it, to reach the goal line.



  Don't modify the fuzz target itself. However, you can add as many prints (std::cout) as you like. For example, if you think it would be useful to print the block header hashes, to use them in the inverse FDP, you may do so.
  You can also print the filter hashes, etc ...

</details>

Maybe if someone is bored they can generalize it, so that the LLM decides itself where to start and what coverage to add, but this seems too open ended for me to work on.

sedited requested review from marcofleon on Oct 7, 2026

DrahtBot added the label Needs rebase on Oct 7, 2026

maflcko force-pushed on Oct 7, 2026

maflcko commented at 2:16 PM on October 7, 2026: member

rebased. Should be trivial to re-ack via git range-diff bitcoin-core/master fa47c22f7a fa09c6c140

DrahtBot removed the label Needs rebase on Oct 7, 2026

in src/test/util/validation.cpp:152 in fa82dc4ba8

 147 | +        Assert(index.Init());
 148 | +        index.Sync();
 149 | +        // The index serves the static setup chain only. Leaving it registered
 150 | +        // deadlocks with ImmediateBackgroundTaskRunner when a fuzzed block
 151 | +        // connects under cs_main.
 152 | +        index.Stop();

marcofleon commented at 8:16 PM on October 7, 2026:

nit: Could use the new StopIndex() here.

maflcko commented at 5:09 AM on October 8, 2026:

thx, done.

marcofleon commented at 8:36 PM on October 7, 2026: contributor

ACK fa09c6c14033cd7d235c0484632749573e51ba0d

Coverage from this PR running from scratch: process message process messages

I didn't run these for super long (only a couple days) but you can see new coverage easily in blockfilterindex.cpp.

DrahtBot requested review from Crypt-iQ on Oct 7, 2026

fuzz: Cover block filter P2P messages fadc2c93d3

test: DestroyAllBlockFilterIndexes in ~ChainTestingSetup()

DestroyAllBlockFilterIndexes is a no-op when no indexes exist, but
calling it here avoids a use-after-free: The validation signals are
destructed in ~ChainTestingSetup, so there is a UAF when
BaseIndex::~BaseIndex() later calls Stop(), and Stop() dereferences
m_chain->context()->validation_signals.

The shutdown order follows the one in init.cpp:
DestroyAllBlockFilterIndexes() after FlushBackgroundCallbacks().

fab64c032c

maflcko force-pushed on Oct 8, 2026

marcofleon commented at 4:46 PM on October 8, 2026: contributor

re-ACK fab64c032c1a0c997c6ec398cc5578df36b33b7d

Crypt-iQ commented at 3:51 PM on October 9, 2026: contributor

re-ACK fab64c032c1a0c997c6ec398cc5578df36b33b7d

sedited merged this on Oct 9, 2026

sedited closed this on Oct 9, 2026

maflcko deleted the branch on Oct 9, 2026