1 files changed, 99 insertions, 2 deletions

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<std::string>());
+                pp.blockHash =
+                    ParsePayoutBlockHash(obj["blockHash"].get<std::string>());
             if (obj.contains("blockHeight") && obj["blockHeight"].is_number())
                 pp.blockHeight = obj["blockHeight"].get<int>();
             // Upstream's tolerance for an entry with no usable height, kept
@@ -976,7 +1000,8 @@ static void LoadPaying()
             r.amount         = obj["amount"].get<int64>();
             r.matureAtHeight = obj["matureAtHeight"].get<int>();
             if (obj.contains("blockHash") && obj["blockHash"].is_string())
-                r.blockHash.SetHex(obj["blockHash"].get<std::string>());
+                r.blockHash =
+                    ParsePayoutBlockHash(obj["blockHash"].get<std::string>());
             if (obj.contains("blockHeight") && obj["blockHeight"].is_number())
                 r.blockHeight = obj["blockHeight"].get<int>();
             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<std::mutex> 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<std::mutex> 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<std::mutex> 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)