From 75f6348510f78f6391177c35219a834471d3e465 Mon Sep 17 00:00:00 2001 From: bluezr Date: Mon, 3 Aug 2026 16:56:53 -0700 Subject: [PATCH] block: let a header own its AuxPoW proof dogecoin_block_header carried an auxpow member holding only the validation hook -- check, ctx and is -- never the proof itself. The proof was built into a local dogecoin_auxpow_block inside dogecoin_block_header_deserialize and freed at cleanup, so by the time the caller had its header the parent coinbase, merkle branches and parent header were gone. dogecoin_block_header_copy made that worse by looking complete: it copied the three hook fields, so a copied merge-mined header was silently missing its proof with nothing to indicate it. Anything needing the proof after the parse therefore had to re-parse the wire bytes, or retain them separately. That is why BIP152 keeps a header_raw span on dogecoin_compact_block: not because a compact block wants raw bytes, but because the parsed header could not answer for itself. Add dogecoin_auxpow_payload, owned by the header. The deserializer moves the parsed fields onto it and nulls them on the scratch block, so ownership transfers rather than duplicating; _free releases it; _copy deep-copies it. The payload deliberately has no back-pointer to its header, unlike dogecoin_auxpow_block, which owns both its header and its parent_header and frees them. A header holding one of those would own the thing that owns it. The payload owns only parent_header, a plain 80-byte header whose own payload is NULL, so ownership terminates. auxpow.check / auxpow.ctx are untouched. That is a validation hook whose context the caller supplies at call time -- validation.c passes the block directly -- not a reference to this data. dogecoin_block_header_copy assigns the copied payload rather than freeing what dest held. Every other field there is a plain overwrite: the function treats dest as raw memory, and callers pass uninitialised stack headers to it -- net_tests.c does, through dogecoin_block_header_deserialize. Freeing dest->auxpow_payload dereferenced whatever the stack contained, which is a SEGV on eight platforms and the reason ASAN caught this and a local build did not: every local caller happened to use dogecoin_block_header_new, where the pointer is NULL. A dest that already owns a payload is the caller's to release, as with every other member. The test extends the height-371338 vector: the proof survives the parse, a copy owns an independent one, and freeing the source leaves the copy intact, which it can only do if nothing is shared. Disabling just the deep copy fails it at line 351. WITH_NET=ON with -DBUILD_SHARED_LIBS=1: 78/78. WITH_NET=OFF: 72/72. ASAN+UBSAN: 78/78, no leaks. --- include/dogecoin/block.h | 36 ++++++++++++++++ src/block.c | 88 ++++++++++++++++++++++++++++++++++++++++ test/block_tests.c | 29 +++++++++++++ 3 files changed, 153 insertions(+) diff --git a/include/dogecoin/block.h b/include/dogecoin/block.h index af3a316aa..cffe9e5b1 100644 --- a/include/dogecoin/block.h +++ b/include/dogecoin/block.h @@ -48,6 +48,30 @@ typedef struct _auxpow { void *ctx; } auxpow; +/* The AuxPoW proof carried by a merge-mined header, owned by the header it + belongs to. + * + * This deliberately has no back-pointer to its owning header, unlike + * dogecoin_auxpow_block. That struct owns both its header and its parent_header + * and frees them, so a header could not hold one without the two owning each + * other. The payload owns only parent_header, which is a plain 80-byte header + * with no payload of its own, so ownership terminates. + * + * The auxpow.check / auxpow.ctx hook on dogecoin_block_header is unaffected: + * it is a validation hook whose context the caller supplies at call time, not a + * reference to this data. */ +typedef struct dogecoin_auxpow_payload_ { + dogecoin_tx* parent_coinbase; + uint256_t parent_hash; + uint8_t parent_merkle_count; + uint256_t* parent_coinbase_merkle; + uint32_t parent_merkle_index; + uint8_t aux_merkle_count; + uint256_t* aux_merkle_branch; + uint32_t aux_merkle_index; + struct dogecoin_block_header_* parent_header; +} dogecoin_auxpow_payload; + typedef struct dogecoin_block_header_ { int32_t version; uint256_t prev_block; @@ -56,8 +80,20 @@ typedef struct dogecoin_block_header_ { uint32_t bits; uint32_t nonce; auxpow auxpow[1]; + /** AuxPoW proof for a merge-mined header, NULL otherwise. Retained so the + header can reproduce the bytes it was parsed from: the deserializer used + to parse this into a local dogecoin_auxpow_block and free it, and + dogecoin_block_header_copy carried only the auxpow hook fields, so the + proof was discarded and anything needing it had to re-parse. */ + dogecoin_auxpow_payload* auxpow_payload; } dogecoin_block_header; +/** Free an AuxPoW payload and everything it owns. */ +LIBDOGECOIN_API void dogecoin_auxpow_payload_free(dogecoin_auxpow_payload* payload); + +/** Deep-copy an AuxPoW payload. Returns NULL if src is NULL. */ +LIBDOGECOIN_API dogecoin_auxpow_payload* dogecoin_auxpow_payload_copy(const dogecoin_auxpow_payload* src); + typedef struct dogecoin_auxpow_block_ { dogecoin_block_header* header; dogecoin_tx* parent_coinbase; diff --git a/src/block.c b/src/block.c index b1b780727..84df5c70a 100644 --- a/src/block.c +++ b/src/block.c @@ -175,6 +175,7 @@ dogecoin_block_header* dogecoin_block_header_new() { header->auxpow->check = check; header->auxpow->ctx = header; header->auxpow->is = false; + header->auxpow_payload = NULL; return header; } @@ -207,8 +208,61 @@ dogecoin_auxpow_block* dogecoin_auxpow_block_new() { * * @return Nothing. */ +void dogecoin_auxpow_payload_free(dogecoin_auxpow_payload* payload) { + if (!payload) return; + dogecoin_tx_free(payload->parent_coinbase); + dogecoin_free(payload->parent_coinbase_merkle); + dogecoin_free(payload->aux_merkle_branch); + /* parent_header is a plain 80-byte header: its own auxpow_payload is NULL, + so this does not recurse. */ + dogecoin_block_header_free(payload->parent_header); + dogecoin_free(payload); + } + +dogecoin_auxpow_payload* dogecoin_auxpow_payload_copy(const dogecoin_auxpow_payload* src) { + if (!src) return NULL; + dogecoin_auxpow_payload* dst = dogecoin_calloc(1, sizeof(*dst)); + if (!dst) return NULL; + + memcpy_safe(dst->parent_hash, src->parent_hash, sizeof(uint256_t)); + dst->parent_merkle_count = src->parent_merkle_count; + dst->parent_merkle_index = src->parent_merkle_index; + dst->aux_merkle_count = src->aux_merkle_count; + dst->aux_merkle_index = src->aux_merkle_index; + + if (src->parent_coinbase) { + dst->parent_coinbase = dogecoin_tx_new(); + if (!dst->parent_coinbase) goto fail; + dogecoin_tx_copy(dst->parent_coinbase, src->parent_coinbase); + } + if (src->parent_merkle_count && src->parent_coinbase_merkle) { + size_t n = (size_t)src->parent_merkle_count * sizeof(uint256_t); + dst->parent_coinbase_merkle = dogecoin_malloc(n); + if (!dst->parent_coinbase_merkle) goto fail; + memcpy_safe(dst->parent_coinbase_merkle, src->parent_coinbase_merkle, n); + } + if (src->aux_merkle_count && src->aux_merkle_branch) { + size_t n = (size_t)src->aux_merkle_count * sizeof(uint256_t); + dst->aux_merkle_branch = dogecoin_malloc(n); + if (!dst->aux_merkle_branch) goto fail; + memcpy_safe(dst->aux_merkle_branch, src->aux_merkle_branch, n); + } + if (src->parent_header) { + dst->parent_header = dogecoin_block_header_new(); + if (!dst->parent_header) goto fail; + dogecoin_block_header_copy(dst->parent_header, src->parent_header); + } + return dst; + +fail: + dogecoin_auxpow_payload_free(dst); + return NULL; + } + void dogecoin_block_header_free(dogecoin_block_header* header) { if (!header) return; + dogecoin_auxpow_payload_free(header->auxpow_payload); + header->auxpow_payload = NULL; header->version = 0; dogecoin_mem_zero(&header->prev_block, DOGECOIN_HASH_LENGTH); dogecoin_mem_zero(&header->merkle_root, DOGECOIN_HASH_LENGTH); @@ -342,6 +396,29 @@ int dogecoin_block_header_deserialize(dogecoin_block_header* header, struct cons goto cleanup; } dogecoin_block_header_copy(header, block->header); + /* Move the proof onto the header instead of letting cleanup free it. + Ownership transfers: the fields are nulled on the block so + dogecoin_auxpow_block_free does not release what the header now owns. */ + dogecoin_auxpow_payload* payload = dogecoin_calloc(1, sizeof(*payload)); + if (!payload) goto cleanup; + payload->parent_coinbase = block->parent_coinbase; + memcpy_safe(payload->parent_hash, block->parent_hash, sizeof(uint256_t)); + payload->parent_merkle_count = block->parent_merkle_count; + payload->parent_coinbase_merkle = block->parent_coinbase_merkle; + payload->parent_merkle_index = block->parent_merkle_index; + payload->aux_merkle_count = block->aux_merkle_count; + payload->aux_merkle_branch = block->aux_merkle_branch; + payload->aux_merkle_index = block->aux_merkle_index; + payload->parent_header = block->parent_header; + block->parent_coinbase = NULL; + block->parent_coinbase_merkle = NULL; + block->aux_merkle_branch = NULL; + block->parent_header = NULL; + /* No free of a prior payload here: dogecoin_block_header_copy above has + already overwritten the pointer, and header may have arrived as an + uninitialised stack struct. Callers own dest's prior contents, the + same contract every other field in this function follows. */ + header->auxpow_payload = payload; } ret = true; cleanup: @@ -510,6 +587,17 @@ void dogecoin_block_header_copy(dogecoin_block_header* dest, const dogecoin_bloc dest->auxpow->check = src->auxpow->check; dest->auxpow->ctx = src->auxpow->ctx; dest->auxpow->is = src->auxpow->is; + /* Deep-copy the proof. Carrying only the hook fields is what discarded it + before, so a copied merge-mined header could not be re-serialized or + re-validated without going back to the wire bytes. + + Assign, do not free what dest held. Every other field here is a plain + overwrite: this function treats dest as raw memory, and callers pass + uninitialised stack headers to it -- net_tests.c does, via + dogecoin_block_header_deserialize. Freeing dest->auxpow_payload would + dereference whatever the stack happened to contain. A dest that already + owns a payload is the caller's to release, as with every other member. */ + dest->auxpow_payload = dogecoin_auxpow_payload_copy(src->auxpow_payload); } /** diff --git a/test/block_tests.c b/test/block_tests.c index 3d27c9e76..861d1e20f 100644 --- a/test/block_tests.c +++ b/test/block_tests.c @@ -333,7 +333,36 @@ void test_auxpow_deserialize_real_vector() { u_assert_uint32_eq(header->bits, 456184976); u_assert_uint32_eq(header->nonce, 0); + /* The AuxPoW proof must survive the parse. It used to be built into a local + dogecoin_auxpow_block and freed at cleanup, so a caller holding the header + could not re-serialize or re-validate it without going back to the wire. */ + u_assert_not_null(header->auxpow_payload); + u_assert_not_null(header->auxpow_payload->parent_coinbase); + u_assert_not_null(header->auxpow_payload->parent_header); + u_assert_int_eq(header->auxpow_payload->parent_merkle_count > 0, 1); + u_assert_not_null(header->auxpow_payload->parent_coinbase_merkle); + + /* And it must copy deeply. dogecoin_block_header_copy carried only the + auxpow hook fields, so a copied merge-mined header silently lost its + proof; now the copy owns its own, and freeing one must not disturb the + other. */ + dogecoin_block_header* dup = dogecoin_block_header_new(); + dogecoin_block_header_copy(dup, header); + u_assert_not_null(dup->auxpow_payload); + u_assert_int_eq(dup->auxpow_payload != header->auxpow_payload, 1); + u_assert_int_eq(dup->auxpow_payload->parent_coinbase + != header->auxpow_payload->parent_coinbase, 1); + u_assert_int_eq(dup->auxpow_payload->parent_merkle_count + == header->auxpow_payload->parent_merkle_count, 1); + u_assert_mem_eq(dup->auxpow_payload->parent_hash, + header->auxpow_payload->parent_hash, DOGECOIN_HASH_LENGTH); + + /* Free the source first: the copy must still be intact, which it can only + be if nothing is shared. */ dogecoin_block_header_free(header); + u_assert_not_null(dup->auxpow_payload); + u_assert_not_null(dup->auxpow_payload->parent_header); + dogecoin_block_header_free(dup); dogecoin_free(buf); }