Skip to content

fuzz: BIP152 compact block deserializer harness (stacked on #351, #379) - #391

Open
xanimo wants to merge 7 commits into
dogecoinfoundation:0.1.5-devfrom
xanimo:0.1.5-dev-bip152-fuzz
Open

fuzz: BIP152 compact block deserializer harness (stacked on #351, #379)#391
xanimo wants to merge 7 commits into
dogecoinfoundation:0.1.5-devfrom
xanimo:0.1.5-dev-bip152-fuzz

Conversation

@xanimo

@xanimo xanimo commented Aug 3, 2026

Copy link
Copy Markdown
Member

Stacked on #351 (fuzz infrastructure) and #379 (BIP152). Review only the top
commit.

Adds a libFuzzer harness for the four BIP152 deserializers. All parse
peer-supplied bytes before any validation:

cmpctblock   header | nonce | vec<shortid[6]> | vec<prefilled txn>
getblocktxn  block_hash | vec<differentially-encoded index>
blocktxn     block_hash | vec<transaction>
sendcmpct    announce (1) | version (8)

cmpctblock and getblocktxn are the interesting pair — both carry
differentially-encoded indices, so the parser accumulates a running total from
attacker-controlled deltas.

2,409,938 executions   1119 new coverage units   0 crashes

What it exposed

dogecoin_compact_block_derive_sipkeys serializes the header with
dogecoin_block_header_serialize, which emits only the six 80-byte fields.
Core's FillShortTxIDSelector streams the auxpow-bearing serializer, so for any
merge-mined block the k0/k1 differ and every short ID mismatches
reconstruction fails silently rather than erroring.

dogecoin_compact_block_serialize has the same gap on the encode side.

This is not fixed here, and it is not a one-line change: the auxpow payload is
parsed into a local dogecoin_auxpow_block and freed inside
dogecoin_block_header_deserialize, so the bytes needed to reproduce Core's
hash are discarded before the compact block ever sees them.
dogecoin_block_header carries an auxpow member, but
dogecoin_block_header_copy moves only check/ctx/is — flags, not payload.
Closing it means retaining the raw header span (80 bytes plus auxpow) on the
compact block and hashing that. Tracked separately.

Two notes on the numbers

The first version of this harness was wrong, and died two executions in.
It deserialized into stack structs, but dogecoin_compact_block_free and its
siblings release the container as well as its members, so they need their
paired _new. ASAN caught it immediately — the harness working as intended,
not an API defect.

Long runs are recorded with detect_leaks=0. libFuzzer runs LeakSanitizer
periodically during a run, not only at exit, and its stop-the-world scan is
unreliable on this host: identical input and identical binary segfault roughly
three times in eight, always after the target has finished and written its
output. Bounded runs with leak detection enabled are clean and report no leaks.
The instability is the environment, not the code — recorded here rather than
omitted, so the flag is not mistaken for hiding something.

@xanimo
xanimo force-pushed the 0.1.5-dev-bip152-fuzz branch 2 times, most recently from a7e030d to 574c18d Compare August 3, 2026 18:32
xanimo added 7 commits August 5, 2026 14:04
Message types and (de)serializers for compact block relay: sendcmpct,
cmpctblock, getblocktxn and blocktxn, plus short-id derivation and the
prefilled-transaction encoding.

Dogecoin is pre-SegWit, so only BIP152 version 1 applies -- short ids are
SipHash-2-4 over the txid, not the wtxid that version 2 introduced for
witness serialization.  CMPCTBLOCK_VERSION is 1 accordingly, and a peer
announcing version 2 is not something this code can honour.

Wire counts are bounded before they size an allocation.  prefilled_count,
indices_count and txs_count are compact_size values read straight off the
wire, and feeding them to calloc() before reading any element reserves
~68 GB, ~17 GB and ~34 GB respectively at 0xFFFFFFFF -- a memory
exhaustion DoS from a tiny message, the same class as the getheaders
locator count.  Every element occupies a minimum number of bytes on the
wire, so any count the remaining buffer cannot hold is invalid by
construction and is rejected before allocating.

Nothing dispatches these messages yet; the P2P wiring follows separately
so it can be reviewed on its own.

79/79 with test_compact_block registered and running.
Announce compact block support once the handshake completes, and record
what each peer announces back.

fAnnounce is sent false: peers are asked not to push unsolicited
cmpctblocks, which suits a client that drives its own block requests.
Core does the same on its non-witness path, sending a single sendcmpct
with version 1 after fSuccessfullyConnected.

Version handling follows Core's SENDCMPCT handler, which enables compact
blocks when the announced version is 1, or 2 only when the local node has
NODE_WITNESS, and otherwise does nothing at all -- no disconnect, no
misbehaviour score.  Dogecoin has no witness serialization, so version 2
short ids (SipHash over the wtxid) cannot be computed here: a peer
announcing it simply leaves cmpct_enabled false.  That is a legitimate
announcement on a witness chain rather than a protocol violation, so
penalising it would be wrong.

fAnnounce itself is a serialized bool and Core does not reject
non-canonical encodings, so any non-zero byte is treated as true.  The
field only selects a relay preference, so rejecting a peer over it would
risk interop damage while protecting nothing.

Verified against a live 1.14.99 node: sendcmpct sent on our side, one
received from the peer, and no version rejection logged -- the peer
announced version 1, as expected on a pre-segwit chain.

Reconstruction (cmpctblock, getblocktxn, blocktxn) follows separately.
79/79.
Audit of dogecoin_compact_block_reconstruct() ahead of dispatching these
messages.  Nothing reaches this code today, so none of the below is
currently exploitable -- which is exactly why it is worth fixing now,
before the P2P wiring turns every one of them into a live path fed by an
untrusted peer.

  - available_txs leaked on two error returns (the shortid_to_txpos and
    missing_indices allocation failures), and the invalid-prefilled-index
    path freed available_txs while leaving available_txs_count non-zero.
    A caller that then reached fill_missing() would index a NULL array
    through a still-positive count.  All failures now funnel through one
    label that empties the state consistently.

  - Calling reconstruct twice on the same peer state leaked the previous
    available_txs and missing_indices.  A peer can send back-to-back
    cmpctblocks, so reset at entry.

  - short_ids_count + prefilled_count could wrap: both are attacker
    supplied, and the sum is used as an allocation size and as the bound
    for every index check below it.  Reject the overflow.

  - Duplicate prefilled indices silently overwrote a slot, desynchronising
    the short-id mapping so a slot existed that no short id could fill;
    the block could then never complete.  Rejected.

fill_missing() additionally refuses a state with no available_txs or
missing_indices rather than dereferencing them on the strength of the
counts alone.

79/79.
The Windows CI legs failed to link with:

  unittester.obj : error LNK2019: unresolved external symbol
  test_compact_block referenced in function main

test/compact_block_tests.c was listed in Makefile.am but never added to
the tests target in CMakeLists.txt, so under CMake unittester.c called a
function that was never compiled. The Linux legs build with autotools and
passed; only the three x86_64-win-native legs use CMake, which is why
this looked platform-specific when it is really build-system-specific.

Also moves src/compact_block.c out of IF(WITH_NET) to match where
Makefile.am already has it. It includes only compact_block.h, mem, sha2,
utils and portable_endian, and references no node, nodegroup or
event_base symbol, so the guard was not buying anything -- and with the
test now registered unconditionally, a -DWITH_NET=OFF build would have
reproduced the same link error.

CMake WITH_NET=ON: 79/79 (was 78, test_compact_block now runs)
CMake WITH_NET=OFF: 73/73, previously would not have linked
… all

Core puts a CBlockHeader in the cmpctblock, not a CPureBlockHeader:

  blockencodings.h:146    CBlockHeader header;
  blockencodings.h:161    READWRITE(header);

and CBlockHeader::SerializationOp (primitives/block.h) reads the pure
header and then, conditionally, the AuxPoW:

  READWRITE(*(CPureBlockHeader*)this);
  if (this->IsAuxpow()) { ...; READWRITE(*auxpow); }

with IsAuxpow() being exactly nVersion & VERSION_AUXPOW (pureheader.h:129).
So the header field is 80 bytes for a non-merge-mined block and 80 bytes
plus a full CAuxPow for everything else -- which, since height 371337, is
effectively every block. It has to be: PartiallyDownloadedBlock::FillBlock
runs CheckBlock, and the proof of work of a merge-mined block lives in the
AuxPoW. A bare 80-byte header would be unverifiable.

Match that dispatch: hand the whole buffer to
dogecoin_block_header_deserialize and let it branch on 0x100 the way
SerializationOp does. A peer that sets the bit without supplying AuxPoW
fails in the AuxPoW sub-parsers, which is what Core does -- the stream read
throws and the message is rejected.

The larger divergence is in the keys. Core derives them over the header as
it streams it:

  FillShortTxIDSelector():  stream << header << nonce;

through the AuxPoW-bearing serializer. libdogecoin derived them from
dogecoin_block_header_serialize, which emits only the six base fields. For
any merge-mined block the preimage differs, so k0/k1 differ, so every short
ID differs and reconstruction fails against every Core peer -- silently, at
the reconstruct step, with nothing pointing back here.

The parsed header cannot repair this. dogecoin_block_header_deserialize
parses AuxPoW into a local dogecoin_auxpow_block and frees it at cleanup,
and dogecoin_block_header_copy carries only auxpow->check, ->ctx and ->is:
flags, not payload. By the time the keys are derived the bytes are gone.

So retain them. dogecoin_compact_block gains header_raw/header_raw_len,
holding the header exactly as it arrived; the deserializer measures the
span by how far dogecoin_block_header_deserialize advanced the buffer.
Keys come from dogecoin_compact_block_derive_sipkeys_raw over that span,
which is Core's preimage by other means. The old
dogecoin_compact_block_derive_sipkeys stays as a wrapper over the 80 base
bytes, documented as correct only when !(version & 0x100).

Serialization emits the retained span. Without one it emits the 80 base
bytes if there is no AuxPoW to lose, and otherwise fails: there is no
AuxPoW serializer in this tree, and putting 80 bytes on the wire for a
merge-mined header produces a message Core rejects. Failing is better than
failing silently, so dogecoin_compact_block_serialize now returns
dogecoin_bool and dogecoin_p2p_msg_cmpctblock propagates it as NULL. This
is an API break, but BIP152 has not shipped in a release.

Tests. The height-371338 mainnet vector moves out of block_tests.c into
test/data/auxpow_block_371338.h so both suites can use it; a real
merge-mined header is the only honest fixture for this. The new round-trip
builds a cmpctblock from that header and asserts the nonce is found behind
the AuxPoW, that header_raw matches the wire bytes, that the keys equal an
independently computed SHA256(span || nonce), that the 80-byte derivation
gives different keys, and that re-serialization is byte-identical. A
0x100-without-AuxPoW message must be rejected. A third test covers the
serialize refusal.

Bounding the header to 80 bytes fails both new tests, the round-trip at the
nonce:
expect 81985529216486895, receive 4294967297 -- four bytes of the parent
coinbase.

check_auxpow running inside the deserializer, so that parsing a cmpctblock
does scrypt work before any peer-level gating, is a separate problem and is
left alone here. Core defers it to CheckBlock.

WITH_NET=OFF: 72/72, valgrind --leak-check=full clean, 0 errors from 0
contexts. WITH_NET=ON: 74/74.
Adds a libFuzzer harness for the four BIP152 deserializers, on the
infrastructure from dogecoinfoundation#351. All parse peer-supplied bytes before validation:

  cmpctblock   header | nonce | vec<shortid[6]> | vec<prefilled txn>
  getblocktxn  block_hash | vec<differentially-encoded index>
  blocktxn     block_hash | vec<transaction>
  sendcmpct    announce (1) | version (8)

cmpctblock and getblocktxn are the interesting pair: both carry indices
that are differentially encoded, so the parser accumulates a running total
from attacker-controlled deltas.

The harness exposed a divergence from Core that is tracked separately:
dogecoin_compact_block_derive_sipkeys serializes the header with
dogecoin_block_header_serialize, which emits only the six 80-byte fields,
while Core's FillShortTxIDSelector streams the auxpow-bearing serializer.
For any merge-mined block the k0/k1 differ and every short ID mismatches,
so reconstruction fails silently. The header struct cannot currently
express the difference: the auxpow payload is parsed into a local
dogecoin_auxpow_block and freed, so the bytes needed to reproduce Core's
hash are discarded before the compact block ever sees them.

Result: 2,409,938 executions, 1119 new coverage units, no crashes.

Two notes on how the numbers were obtained.

The first version of this harness deserialized into stack structs and
died two executions in: dogecoin_compact_block_free and friends release
the container as well as its members, so they need their paired _new.
ASAN caught it immediately, which is the harness working as intended
rather than a defect in the API.

Long runs are recorded with detect_leaks=0. libFuzzer runs LeakSanitizer
periodically during a run, and its stop-the-world scan is unreliable on
this host -- identical input and identical binary segfault roughly three
times in eight, always after the target has finished and written its
output. Bounded runs with leak detection enabled are clean and report no
leaks; the instability is the environment, not the code.
A cmpctblock header is 80 bytes plus a full AuxPoW whenever version bit
0x100 is set, so the harness needs an input where that blob is well formed,
not merely present.

The seed is a cmpctblock built from the height-371338 mainnet vector the
BIP152 tests use: the 535-byte header span (80 base + 455 AuxPoW) measured
by parsing it, followed by a known nonce and empty short-id and prefilled
vectors. Same construction as
test_compact_block_real_auxpow_header_roundtrip, so the fixture stays
honest -- a synthetic AuxPoW would only prove the parser accepts what this
repository invents.

It is not about reaching the AuxPoW parser. That takes one version bit and
the fuzzer finds it immediately: from an empty corpus it enters
deserialize_dogecoin_auxpow_block 209105 times in 746529 runs. What random
input does not produce is a *valid* AuxPoW, so those executions die in the
first sub-parser.

The seed gives the mutator a working structure to vary. Throughput shows
the difference: 396923 runs seeded against 746529 unseeded over the same 60
seconds, roughly half the speed per execution, because inputs derived from
the seed survive the early rejects and reach the parent coinbase, the
merkle branches, the parent header, and check_auxpow's scrypt work.

Placed under test/fuzz_corpus/compact_block/ to match the layout the PSBT
harness established.
@xanimo
xanimo force-pushed the 0.1.5-dev-bip152-fuzz branch from 574c18d to e647a73 Compare August 5, 2026 21:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant