From 7221bb35d50e9bac11ce5e4b3252c34abe03c124 Mon Sep 17 00:00:00 2001 From: bitflash-maintainer Date: Wed, 2 Sep 2026 09:01:34 +0000 Subject: payout: treat a malformed ledger block hash as corruption, not as a hash SetHex() parses whatever hex prefix it finds and stops, so a corrupt blockHash loaded as a DIFFERENT hash: garbage with a hex-looking prefix became a hash no block ever had, which the orphan check reads as an orphaned block -- and the next flush voided the debt on the strength of file corruption. Garbage with no hex prefix became 0 and skipped the orphan check instead. A field that is not the 64-digit hex string this tree writes now fails the load, and the file is quarantined like any other parse error. --- src/rpc.cpp | 101 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 99 insertions(+), 2 deletions(-) (limited to 'src/rpc.cpp') diff --git a/src/rpc.cpp b/src/rpc.cpp index 310c0d4..be6c80d 100644 --- a/src/rpc.cpp +++ b/src/rpc.cpp @@ -776,6 +776,29 @@ static void PublishPendingPayouts(const VersionedBlob& v) gPayoutLedgerPublishedSeq, "pending payout ledger"); } +// Parse a persisted block-hash field, refusing to guess. SetHex() reads +// whatever hex prefix it finds and stops at the first character it does not +// recognise, so a corrupt value does not fail to parse -- it parses as a +// DIFFERENT hash. Both shapes of that are wrong in a dangerous direction: a +// value with a hex-looking prefix ("deadbeef...") becomes a hash no block +// ever had, which PayoutBlockStillOnChain reads as an orphaned block and +// FlushMaturePayouts then VOIDS the debt; a value with no hex prefix at all +// becomes 0, which is grandfathered past the orphan check on the strength +// of garbage. A field that is not the 64-digit hex string ToString() wrote +// is corruption, and corruption is a load failure like any other parse +// error -- the file is quarantined, not read around. +static uint256 ParsePayoutBlockHash(const std::string& str) +{ + if (str.size() != 64 || + str.find_first_not_of("0123456789abcdefABCDEF") != std::string::npos) + throw std::runtime_error( + strprintf("blockHash \"%s\" is not a 64-digit hex string", + str.c_str())); + uint256 h; + h.SetHex(str); + return h; +} + static void LoadPendingPayouts() { std::string path = PayoutFilePath(); @@ -797,7 +820,8 @@ static void LoadPendingPayouts() // field. Leave blockHash zero and they are grandfathered past the // orphan check rather than voided on a guess. if (obj.contains("blockHash") && obj["blockHash"].is_string()) - pp.blockHash.SetHex(obj["blockHash"].get()); + pp.blockHash = + ParsePayoutBlockHash(obj["blockHash"].get()); if (obj.contains("blockHeight") && obj["blockHeight"].is_number()) pp.blockHeight = obj["blockHeight"].get(); // Upstream's tolerance for an entry with no usable height, kept @@ -976,7 +1000,8 @@ static void LoadPaying() r.amount = obj["amount"].get(); r.matureAtHeight = obj["matureAtHeight"].get(); if (obj.contains("blockHash") && obj["blockHash"].is_string()) - r.blockHash.SetHex(obj["blockHash"].get()); + r.blockHash = + ParsePayoutBlockHash(obj["blockHash"].get()); if (obj.contains("blockHeight") && obj["blockHeight"].is_number()) r.blockHeight = obj["blockHeight"].get(); if (obj.contains("retries") && obj["retries"].is_number()) @@ -4681,6 +4706,78 @@ int RunPayoutLedgerSelfTest() strPoolRoundsFile = savedRoundsFile; } + // -- 16. A malformed block hash is corruption, not a hash ---------------- + // + // SetHex() parses whatever hex prefix it finds and stops, so a corrupt + // blockHash did not fail to load -- it loaded as a DIFFERENT hash. + // "deadbeef..." became 0xdeadbeef, a hash no block ever had, which the + // orphan check reads as an orphaned block and the next flush VOIDS the + // debt; a value with no hex prefix became 0, which is grandfathered past + // the orphan check instead. Either way a corrupt file silently changed + // what the pool owed. It is a load failure now, handled like every other + // parse error: quarantine, and never rewrite. + { + PayoutTestResetState(); + remove(ledgerPath.c_str()); + remove(payingPath.c_str()); + + const std::string badLedger = strprintf( + "[{\"amounts\":{\"%s\":100000000}," + "\"blockHash\":\"deadbeef oops, not a hash\"," + "\"blockHeight\":10,\"matureAtHeight\":10}]", addrA.c_str()); + check(PayoutTestWriteFile(ledgerPath, badLedger), + "wrote a ledger whose blockHash is not a 64-digit hex string"); + LoadPendingPayouts(); + { + std::lock_guard lk(gPayoutLedgerMutex); + check(gPendingPayouts.empty(), + "no entry is loaded with a guessed hash"); + } + std::string gone; + check(!PayoutTestReadFile(ledgerPath, gone), + "the ledger with the corrupt hash is quarantined"); + + const std::string badPaying = strprintf( + "[{\"address\":\"%s\",\"amount\":100000000," + "\"blockHash\":\"00000000cafe\",\"blockHeight\":10," + "\"matureAtHeight\":10}]", addrA.c_str()); + check(PayoutTestWriteFile(payingPath, badPaying), + "wrote an in-flight list whose blockHash is truncated"); + LoadPaying(); + { + std::lock_guard lk(gPayoutExecMutex); + check(gPaying.empty(), + "no in-flight record is loaded with a guessed hash"); + } + check(!PayoutTestReadFile(payingPath, gone), + "the in-flight list with the corrupt hash is quarantined"); + + // The two spellings every real file uses still load: the 64-digit + // hash of a keyed round, and the 64 zeros this build writes for a + // grandfathered entry. + PayoutTestResetState(); + remove(ledgerPath.c_str()); + const std::string goodLedger = strprintf( + "[{\"amounts\":{\"%s\":100000000}," + "\"blockHash\":\"00000000000000000000000000000000000000000000000000000000000000cd\"," + "\"blockHeight\":10,\"matureAtHeight\":10}," + "{\"amounts\":{\"%s\":200000000}," + "\"blockHash\":\"%s\"," + "\"blockHeight\":20,\"matureAtHeight\":20}]", + addrA.c_str(), addrB.c_str(), uint256(0).ToString().c_str()); + check(PayoutTestWriteFile(ledgerPath, goodLedger), + "wrote a ledger in both real hash spellings"); + LoadPendingPayouts(); + { + std::lock_guard lk(gPayoutLedgerMutex); + check(gPendingPayouts.size() == 2 && + gPendingPayouts[0].blockHash != 0 && + gPendingPayouts[1].blockHash == 0, + "a real hash and a serialised zero hash both still load"); + } + remove(ledgerPath.c_str()); + } + // -- clean up ------------------------------------------------------------ PayoutTestResetState(); CRITICAL_BLOCK(cs_mapWallet) -- cgit v1.2.3