fix: guard BuildWitnessCache against an off-active-chain pindex (heap overflow)

BuildWitnessCache sizes its blockCms buffer from pindex->GetHeight() but the
Phase-1 loop walks the active chain (chainActive.Next), terminating only on
pbi==pindex. If a reorg moved pindex off the active chain while the notify
thread lagged (cs_main is released between per-block ChainTip calls) and the new
active tip is taller, the loop never reaches pindex and, once past pindex's
height, writes blockCms[h-startHeight] out of bounds -- a heap overflow.
Rebuilding witnesses for an abandoned block is meaningless anyway, so bail early
when pindex is not on the active chain; cs_main is held for the whole function,
so the check cannot race the loop.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

src/hush/tlsmanager.cpp

@@ -593,6 +593,7 @@ int TLSManager::threadSocketHandler(CNode* pnode, fd_set& fdsetRecv, fd_set& fds
                 int64_t nPassBytes = 0;
                 bool fKeepReading = true;
                 while (fKeepReading) {
+                    int nSSLPending = 0;
                     if (nRecvBase + nPassBytes > (int64_t)ReceiveFloodSize())
                         break;
                     {
@@ -609,6 +610,10 @@ int TLSManager::threadSocketHandler(CNode* pnode, fd_set& fdsetRecv, fd_set& fds
                             wolfSSL_ERR_clear_error(); // clear the error queue, otherwise we may be reading an old error that occurred previously in the current thread
                             nBytes = wolfSSL_read(pnode->ssl, pchBuf, sizeof(pchBuf));
                             nRet = wolfSSL_get_error(pnode->ssl, nBytes);
+                            // Capture TLS buffered-byte count while still under cs_hSocket; the drain-continuation
+                            // check below runs unlocked, so touching pnode->ssl there would race with
+                            // SocketSendData / CloseSocketDisconnect (which free ssl) -> data race / use-after-free.
+                            nSSLPending = wolfSSL_pending(pnode->ssl);
                         } else {
                             nBytes = recv(pnode->hSocket, pchBuf, sizeof(pchBuf), MSG_DONTWAIT);
                             nRet = WSAGetLastError();
@@ -628,7 +633,7 @@ int TLSManager::threadSocketHandler(CNode* pnode, fd_set& fdsetRecv, fd_set& fds
                         // buffer, or TLS has buffered decrypted bytes) and within the per-pass cap.
                         // The flood ceiling is enforced pre-read at the top of the loop.
                         if (fKeepReading) {
-                            bool fMore = (nBytes == (int)sizeof(pchBuf)) || (bIsSSL && wolfSSL_pending(pnode->ssl) > 0);
+                            bool fMore = (nBytes == (int)sizeof(pchBuf)) || (bIsSSL && nSSLPending > 0);
                             if (!fMore || ++nDrainReads >= MAX_DRAIN_READS)
                                 fKeepReading = false;
                         }

src/init.cpp

@@ -1087,8 +1087,8 @@ static const int64_t g_nMinCoinCacheMB = 256; // never thrash below this working
 
 // Scheduled task: nudge nCoinCacheUsage toward "use all RAM except the reserve". If free RAM is below
 // the reserve we shrink the target (the next per-block flush releases the excess); if there is spare
-// RAM we grow it back toward the startup ceiling. Lock-free: it only reads system memory and writes
+// RAM we grow it back toward the startup ceiling. nCoinCacheUsage is std::atomic<size_t>, so this
-// the aligned size_t threshold that the flush path reads.
+// cross-thread write (vs the cs_main-held reads in FlushStateToDisk/VerifyDB) is well-defined, no lock needed.
 static void AdjustCoinCacheForMemoryPressure()
 {
     if (g_nMaxCoinCacheUsage == 0)
@@ -2072,7 +2072,25 @@ bool AppInit2(boost::thread_group& threadGroup, CScheduler& scheduler)
                 pcoinsdbview   = new CCoinsViewDB(nCoinDBCache, false, fReindex);
                 pcoinscatcher  = new CCoinsViewErrorCatcher(pcoinsdbview);
                 pcoinsTip      = new CCoinsViewCache(pcoinscatcher);
-                pnotarizations = new NotarizationDB(100*1024*1024, false, fReindex);
+                try {
+                    pnotarizations = new NotarizationDB(100*1024*1024, false, fReindex);
+                } catch (const std::exception& e) {
+                    // The notarizations (dPoW) DB is non-essential and node-regenerable. It has been seen to
+                    // snapshot/flush torn (0-byte log -> leveldb "Database I/O error" on reopen), which
+                    // previously aborted startup with a spurious "Error opening block database" and forced a
+                    // full resync. Wipe and recreate it instead of failing hard.
+                    LogPrintf("%s: notarizations DB failed to open (%s); moving aside and regenerating (non-fatal)\n", __FUNCTION__, e.what());
+                    // Move (do NOT delete) the old DB aside, so a transient open failure (fd
+                    // exhaustion, disk full, permissions) cannot permanently destroy notarization
+                    // history. If the recreate below also fails it propagates as fatal and the old
+                    // data survives in notarizations.corrupt for recovery.
+                    {
+                        boost::filesystem::path ndir = GetDataDir() / "notarizations";
+                        boost::filesystem::remove_all(ndir.string() + ".corrupt");
+                        boost::filesystem::rename(ndir, ndir.string() + ".corrupt");
+                    }
+                    pnotarizations = new NotarizationDB(100*1024*1024, false, true);
+                }
 
 
                 if (fReindex) {

src/main.cpp

@@ -109,7 +109,7 @@ bool fIsBareMultisigStd = true;
 bool fCheckBlockIndex = false;
 bool fCheckpointsEnabled = true;
 bool fCoinbaseEnforcedProtectionEnabled = true;
-size_t nCoinCacheUsage = 5000 * 300;
+std::atomic<size_t> nCoinCacheUsage(5000 * 300);
 uint64_t nPruneTarget = 0;
 // If the tip is older than this (in seconds), the node is considered to be in initial block download.
 int64_t nMaxTipAge = DEFAULT_MAX_TIP_AGE;

src/main.h

@@ -43,6 +43,7 @@
 #include "txmempool.h"
 #include "uint256.h"
 
+#include <atomic>
 #include <algorithm>
 #include <exception>
 #include <map>
@@ -189,7 +190,7 @@ extern bool fCheckpointsEnabled;
 // TODO: remove this flag by structuring our code such that
 // it is unneeded for testing
 extern bool fCoinbaseEnforcedProtectionEnabled;
-extern size_t nCoinCacheUsage;
+extern std::atomic<size_t> nCoinCacheUsage;
 extern CFeeRate minRelayTxFee;
 extern int64_t nMaxTipAge;
 

src/pow.cpp

@@ -739,8 +739,18 @@ bool RandomXValidationRequired(int32_t height)
     if (HUSH_LOADINGBLOCKS != 0)
         return false;
     extern bool fCheckpointsEnabled;
-    if (fCheckpointsEnabled && height < Checkpoints::GetTotalBlocksEstimate(Params().Checkpoints()))
+    // Gate the RandomX skip on the last checkpoint actually LOCKED INTO this node's block index
-        return false;
+    // (GetLastCheckpoint), NOT the static top checkpoint (GetTotalBlocksEstimate). The fork-rejection
+    // guard uses this same in-index boundary, so a block below it is provably on the checkpoint-pinned
+    // chain and cannot be a forged fork. Using the static boundary would, once checkpoints extend above
+    // the RandomX activation height, leave a gap (in-index checkpoint .. static top) during IBD/eclipse
+    // where a no-hashpower peer could get SHA256-grinded, RandomX-forged blocks accepted. Safe to walk
+    // mapBlockIndex here: called only under cs_main (ActivateBestChainStep + inline CheckRandomXSolution).
+    if (fCheckpointsEnabled) {
+        CBlockIndex* pcheckpoint = Checkpoints::GetLastCheckpoint(Params().Checkpoints());
+        if (pcheckpoint != NULL && height < pcheckpoint->GetHeight())
+            return false;
+    }
     return true;
 }
 

src/validationinterface.cpp

@@ -156,10 +156,17 @@ void ThreadNotifyWallets(CBlockIndex *pindexLastTip)
                     assert(pcoinsTip->GetSaplingAnchorAt(SaplingMerkleTree::empty_root(), oldSaplingTree));
                 }
 
+                // Use operator[] (empty-list default), NOT .at(): a block's conflict entry can be
+                // absent if it was drained in an earlier cycle whose connect loop hit the IBD
+                // ReadBlockFromDisk retry below (that break leaves pindexLastTip behind, so the block
+                // gets rebuilt here but its conflicts were already cleared and are never re-inserted).
+                // .at() would throw std::out_of_range which, uncaught under cs_main in this boost
+                // thread, aborts the node (and crash-loops since pindexLastTip cannot advance). A
+                // missing entry simply means no conflict notifications for this block (best-effort).
                 blockStack.emplace_back(
                     pindex,
                     std::make_pair(oldSproutTree, oldSaplingTree),
-                    recentlyConflicted.first.at(pindex));
+                    recentlyConflicted.first[pindex]);
 
                 pindex = pindex->pprev;
             }
@@ -174,7 +181,11 @@ void ThreadNotifyWallets(CBlockIndex *pindexLastTip)
         // network message processing thread.
         //
 
-        // Notify block disconnects
+        // Notify block disconnects. If an IBD read fails mid-disconnect we must NOT fall through
+        // to the connect loop (which advances pindexLastTip to the new tip and permanently abandons
+        // the remaining disconnects -> wallet witness/anchor desync); skip connects this cycle and
+        // retry the whole disconnect next cycle, exactly as the connect-side break retries.
+        bool fDisconnectIncomplete = false;
         while (pindexLastTip && pindexLastTip != pindexFork) {
             // Read block from disk.
             CBlock block;
@@ -184,6 +195,7 @@ void ThreadNotifyWallets(CBlockIndex *pindexLastTip)
                     // Sleep briefly and retry on the next cycle instead of crashing.
                     LogPrintf("%s: block at height %d not yet readable, will retry\n",
                               __func__, pindexLastTip->GetHeight());
+                    fDisconnectIncomplete = true;
                     break;
                 }
                 LogPrintf("*** %s\n", "Failed to read block while notifying wallets of block disconnects");
@@ -205,8 +217,9 @@ void ThreadNotifyWallets(CBlockIndex *pindexLastTip)
             pindexLastTip = pindexLastTip->pprev;
         }
 
-        // Notify block connections
+        // Notify block connections (skipped this cycle if the disconnect loop broke to retry an
-        while (!blockStack.empty()) {
+        // unreadable block, so pindexLastTip is not advanced past the un-disconnected blocks).
+        while (!fDisconnectIncomplete && !blockStack.empty()) {
             auto blockData = blockStack.back();
             blockStack.pop_back();
 

src/wallet/wallet.cpp

@@ -1229,6 +1229,20 @@ void CWallet::BuildWitnessCache(const CBlockIndex* pindex, bool witnessOnly)
 
   LOCK2(cs_main, cs_wallet);
 
+  // The Phase-1 loop below walks the ACTIVE chain (chainActive.Next) and sizes blockCms from
+  // pindex->GetHeight(), terminating only on pbi==pindex. If pindex was reorged OFF the active
+  // chain (a reorg landed while ThreadNotifyWallets drained its connect backlog with cs_main
+  // released), the loop never reaches pindex and, once the active tip passes pindex's height, it
+  // writes blockCms[h-startHeight] out of bounds -> heap overflow. Rebuilding witnesses for an
+  // abandoned block is meaningless; the ChainTip for the new active tip re-drives this. cs_main is
+  // held for the whole function, so this check cannot race the loop below.
+  if (pindex != chainActive[pindex->GetHeight()]) {
+    if (fZdebug)
+      LogPrintf("%s: pindex height=%d not on active chain (reorg); skipping witness rebuild\n",
+                __func__, pindex->GetHeight());
+    return;
+  }
+
   int startHeight = VerifyAndSetInitialWitness(pindex, witnessOnly) + 1;
 
   if (startHeight > pindex->GetHeight() || witnessOnly) {