fix(wallet): opreturn_burn return change + widen txfee to CAmount

#10 (HIGH) opreturn_burn selected UTXOs for nAmount+txfee but pushed only the
burn vout and returned - so the entire selected-input surplus was silently paid
as miner fee (e.g. a 500-coin UTXO burning 10 lost ~490). Push a change output
for (inputs - nAmount - txfee). Also widen the int32_t txfee (which truncated
large CAmount fees) to CAmount and MoneyRange-validate.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

src/hush.h

@@ -591,6 +591,14 @@ int32_t hush_voutupdate(bool fJustCheck,int32_t *isratificationp,int32_t notaryi
             opretlen += (scriptbuf[len++] << 8);
         }
         opoffset = len;
+        // SECURITY (Finding #4): opretlen is attacker-controlled (up to 65535 via OP_PUSHDATA2)
+        // and was previously used with no bounds check. scriptbuf is a fixed DRAGON_MAXSCRIPTSIZE
+        // stack buffer in hush_connectblock, so an oversized opretlen drives out-of-bounds reads in
+        // the downstream 'K'/KV and notarization paths (persisted to disk, leaked via kvsearch RPC,
+        // reliable crash on block connect). Reject any opret claiming more bytes than actually
+        // remain in the real script; this mirrors the no-OP_RETURN fall-through so nothing valid changes.
+        if ( opretlen < 0 || opretlen > scriptlen - len )
+            return(notaryid);
         matched = 0;
         if ( SMART_CHAIN_SYMBOL[0] == 0 )
         {
@@ -933,7 +941,7 @@ int32_t hush_connectblock(bool fJustCheck, CBlockIndex *pindex,CBlock& block)
                         if ( len >= sizeof(uint32_t) && len <= sizeof(scriptbuf) )
                         {
                             memcpy(scriptbuf,(uint8_t *)&block.vtx[i].vout[j].scriptPubKey[0],len);
-                            if ( len == 35 && scriptbuf[0] == 33 && scriptbuf[34] == 0xac )
+                            if ( len == 35 && scriptbuf[0] == 33 && scriptbuf[34] == 0xac && numvalid < (int32_t)(sizeof(pubkeys)/sizeof(pubkeys[0])) )
                             {
                                 memcpy(pubkeys[numvalid++],scriptbuf+1,33);
                                 for (k=0; k<33; k++)

src/hush_nSPV_fullnode.h

@@ -324,7 +324,7 @@ int32_t NSPV_mempoolfuncs(bits256 *satoshisp,int32_t *vindexp,std::vector<uint25
                 CScript scriptPubKey = tx.vout[tx.vout.size()-1].scriptPubKey;
                 if ( GetOpReturnData(scriptPubKey,vopret) != 0 )
                 {
-                    if ( vopret[0] == evalcode && vopret[1] == func )
+                    if ( vopret.size() >= 2 && vopret[0] == evalcode && vopret[1] == func )
                     {
                         txids.push_back(hash);
                         num++;
@@ -657,7 +657,7 @@ void hush_nSPVreq(CNode *pfrom,std::vector<uint8_t> request) // received a reque
             if ( timestamp > pfrom->prevtimes[ind] )
             {
                 struct NSPV_utxosresp U;
-                if ( len < 64+5 && request[1] < 64 && (request[1] == len-3 || request[1] == len-7 || request[1] == len-11) )
+                if ( len >= 2 && len < 64+5 && request[1] < 64 && (request[1] == len-3 || request[1] == len-7 || request[1] == len-11) )
                 {
                     int32_t skipcount = 0; char coinaddr[64]; uint8_t filter; uint8_t isCC = 0;
                     memcpy(coinaddr,&request[2],request[1]);   // request[1] < 64 bounds the copy + the terminator write
@@ -697,7 +697,7 @@ void hush_nSPVreq(CNode *pfrom,std::vector<uint8_t> request) // received a reque
             if ( timestamp > pfrom->prevtimes[ind] )
             {
                 struct NSPV_txidsresp T;
-                if ( len < 64+5 && request[1] < 64 && (request[1] == len-3 || request[1] == len-7 || request[1] == len-11) )
+                if ( len >= 2 && len < 64+5 && request[1] < 64 && (request[1] == len-3 || request[1] == len-7 || request[1] == len-11) )
                 {
                     int32_t skipcount = 0; char coinaddr[64]; uint32_t filter; uint8_t isCC = 0;
                     memcpy(coinaddr,&request[2],request[1]);   // request[1] < 64 bounds the copy + the terminator write
@@ -730,7 +730,7 @@ void hush_nSPVreq(CNode *pfrom,std::vector<uint8_t> request) // received a reque
                         }
                         NSPV_txidsresp_purge(&T);
                     }
-                } else fprintf(stderr,"len.%d req1.%d\n",len,request[1]);
+                } else fprintf(stderr,"len.%d\n",len);
             }
         }
         else if ( request[0] == NSPV_MEMPOOL )
@@ -767,7 +767,7 @@ void hush_nSPVreq(CNode *pfrom,std::vector<uint8_t> request) // received a reque
                             NSPV_mempoolresp_purge(&M);
                         }
                     }
-                } else fprintf(stderr,"len.%d req1.%d\n",len,request[1]);
+                } else fprintf(stderr,"len.%d\n",len);
             }
         }
         else if ( request[0] == NSPV_NTZS )

src/main.cpp

@@ -5457,7 +5457,24 @@ bool AcceptBlockHeader(int32_t *futureblockp,const CBlockHeader& block, CValidat
     // SECURITY (header-flood DoS): once synced, verify PoW at header-accept time so a peer cannot
     // flood unbounded PoW-less headers into mapBlockIndex (they now fail RandomX -> DoS-ban). During
     // IBD keep fCheckPOW=0 for fast header sync; the full RandomX/target check runs at block connect.
-    if (!CheckBlockHeader(futureblockp,*ppindex!=0?(*ppindex)->GetHeight():0,*ppindex, block, state, IsInitialBlockDownload() ? 0 : 1)) {
+    // Resolve the parent up-front so CheckBlockHeader receives THIS header's own (still-NULL) pindex
+    // and its CORRECT height (parent height + 1) — never the caller's reused *ppindex, which in a
+    // HEADERS batch aliases the PREVIOUS header. Passing *ppindex here made (a) the height a stale
+    // value (0 for a fresh header, or the prior header's height when aliased) so post-IBD
+    // RandomXValidationRequired() saw a below-activation height and CheckRandomXSolution returned true
+    // WITHOUT verifying, and (b) the (pindex && pindex->fRandomXVerified) short-circuit fire on an
+    // as-yet-unverified header — both silently defeating the post-IBD header-flood PoW gate. The
+    // authoritative parent validation (prev-not-found / prev-invalid) still runs unchanged below; this
+    // lookup is read-only and under cs_main, so it cannot disagree with it. IBD stays fast: fCheckPOW
+    // is still 0 during IBD, so no RandomX is computed here regardless of the height.
+    CBlockIndex* pindexPrevForHeight = NULL;
+    {
+        BlockMap::iterator miPrev = mapBlockIndex.find(block.hashPrevBlock);
+        if (miPrev != mapBlockIndex.end())
+            pindexPrevForHeight = miPrev->second;
+    }
+    int32_t nHeaderHeight = (pindexPrevForHeight != NULL) ? pindexPrevForHeight->GetHeight() + 1 : 0;
+    if (!CheckBlockHeader(futureblockp,nHeaderHeight,pindex, block, state, IsInitialBlockDownload() ? 0 : 1)) {
         if ( *futureblockp == 0 ) {
             LogPrintf("%s: CheckBlockHeader futureblock=0\n", __func__);
             return false;
@@ -7587,6 +7604,15 @@ bool static ProcessMessage(CNode* pfrom, string strCommand, CDataStream& vRecv,
         uint256 hashStop;
         vRecv >> locator >> hashStop;
 
+        // Bound the locator before FindForkInGlobalIndex() scans it linearly under cs_main. An honest
+        // GetLocator() never exceeds MAX_LOCATOR_SZ, so this cannot reject a valid peer; an oversized
+        // vHave (~130k hashes fit in one message) is a message-thread liveness DoS. Ban like the
+        // adjacent vInv > MAX_INV_SZ path.
+        if (locator.vHave.size() > MAX_LOCATOR_SZ) {
+            Misbehaving(pfrom->GetId(), 20);
+            return true;
+        }
+
         LOCK(cs_main);
 
         // Find the last block the caller has in the main chain
@@ -7619,6 +7645,15 @@ bool static ProcessMessage(CNode* pfrom, string strCommand, CDataStream& vRecv,
         uint256 hashStop;
         vRecv >> locator >> hashStop;
 
+        // Bound the locator before FindForkInGlobalIndex() scans it linearly under cs_main. An honest
+        // GetLocator() never exceeds MAX_LOCATOR_SZ, so this cannot reject a valid peer; an oversized
+        // vHave (~130k hashes fit in one message) is a message-thread liveness DoS. Ban like the
+        // adjacent vInv > MAX_INV_SZ path.
+        if (locator.vHave.size() > MAX_LOCATOR_SZ) {
+            Misbehaving(pfrom->GetId(), 20);
+            return true;
+        }
+
         LOCK(cs_main);
 
 

src/main.h

@@ -130,6 +130,12 @@ static const unsigned int BLOCK_STALLING_TIMEOUT = 2;
  *  peer's 160-header reply as "tip reached" and stall header sync. Raise only as a coordinated
  *  network upgrade (with a protocol-version bump). */
 static const unsigned int MAX_HEADERS_RESULTS = 160;
+/** Maximum number of entries we accept in a CBlockLocator.vHave (GETBLOCKS / GETHEADERS). An honest
+ *  CChain::GetLocator() emits ~10 linear hashes then exponentially-spaced ones, so even a chain of
+ *  2^91 blocks stays well under this bound (GetLocator reserves 32). Matches upstream Bitcoin Core's
+ *  MAX_LOCATOR_SZ. A larger vHave is a peer trying to make FindForkInGlobalIndex() linearly scan a
+ *  huge list under cs_main (message-thread liveness DoS). */
+static const unsigned int MAX_LOCATOR_SZ = 101;
 /** Size of the "block download window": how far ahead of our current height do we fetch?
  *  Larger windows tolerate larger download speed differences between peer, but increase the potential
  *  degree of disordering of blocks on disk (which make reindexing and in the future perhaps pruning

src/rpc/misc.cpp

@@ -560,7 +560,7 @@ UniValue z_validateaddress(const UniValue& params, bool fHelp, const CPubKey& my
 
 
 #ifdef ENABLE_WALLET
-    LOCK2(cs_main, pwalletMain->cs_wallet);
+    LOCK2(cs_main, pwalletMain ? &pwalletMain->cs_wallet : NULL);
 #else
     LOCK(cs_main);
 #endif

src/wallet/rpcwallet.cpp

@@ -6380,7 +6380,7 @@ void RegisterWalletRPCCommands(CRPCTable &tableRPC)
 
 UniValue opreturn_burn(const UniValue& params, bool fHelp, const CPubKey& mypk)
 {
-    std::vector<uint8_t> vHexStr; CScript opret; int32_t txfee = 10000;CPubKey myPubkey;
+    std::vector<uint8_t> vHexStr; CScript opret; CAmount txfee = 10000;CPubKey myPubkey;
     if (fHelp || (params.size() < 2) || (params.size() > 4) )
     {
         throw runtime_error(
@@ -6413,6 +6413,9 @@ UniValue opreturn_burn(const UniValue& params, bool fHelp, const CPubKey& mypk)
     if ( params.size() > 2 )
         txfee = AmountFromValue(params[2]);
 
+    if ( !MoneyRange(nAmount) || !MoneyRange(txfee) || !MoneyRange(nAmount + txfee) )
+        throw JSONRPCError(RPC_TYPE_ERROR, "burn_amount + txfee out of range.");
+
     if (!EnsureWalletIsAvailable(fHelp))
         throw JSONRPCError(RPC_TYPE_ERROR, "wallet is locked or unavailable.");
     EnsureWalletIsUnlocked();
@@ -6425,12 +6428,17 @@ UniValue opreturn_burn(const UniValue& params, bool fHelp, const CPubKey& mypk)
 	CMutableTransaction mtx = CreateNewContextualCMutableTransaction(Params().GetConsensus(), hush_nextheight());
 
 	int64_t normalInputs = AddNormalinputs(mtx, myPubkey, nAmount+txfee, 60);
-	if (normalInputs < nAmount)
+	if (normalInputs < nAmount+txfee)
 		throw runtime_error("insufficient funds\n");
     
 	opret << OP_RETURN << E_MARSHAL(ss << vHexStr);
     
     mtx.vout.push_back(CTxOut(nAmount,opret));
+    // Return the unspent surplus (selected inputs - burn amount - txfee) as change to a
+    // wallet-owned address; without this the entire surplus is silently paid as miner fee.
+    CAmount change = normalInputs - nAmount - txfee;
+    if ( change > 0 )
+        mtx.vout.push_back(CTxOut(change, GetScriptForDestination(myPubkey.GetID())));
     ret.push_back(Pair("hex", EncodeHexTx(mtx)));
 	return(ret);
 }