From db42091ce3452abc3e155b5936785b931590d130 Mon Sep 17 00:00:00 2001 From: DanS Date: Mon, 31 Aug 2026 02:38:38 -0500 Subject: [PATCH] rpc/net: gate and harden stratummine, keep regtest off the live network, init nSPV filter stratummine (rpc/mining.cpp), a test-only reference miner, was registered unconditionally behind a bare #ifndef WIN32 with okSafeMode=true, so it shipped as a live RPC on every non-Windows release build. Four fixes: - Gated behind an explicit, default-off -stratummine, with regtest exempt so qa/rpc-tests can still drive it. Deliberately NOT gated on fExperimentalMode: that defaults to TRUE (init.cpp:1195), so such a gate is a no-op -- an error made and caught while testing this change. - mining.notify's three hash fields went straight into uint256(ParseHex(...)). uint256's vector ctor asserts on a wrong size (uint256.cpp:30) and NDEBUG is defined nowhere in this build, so that assert is live in release; ParseHex also truncates silently at the first non-hex character. A short or garbled field therefore ABORTED THE DAEMON. Added StratumHex256(), which requires 64 hex chars and a 32-byte result, and all three fields are validated before any is committed so a bad job is rejected rather than half-applied. - processLine ran inside the window where the RandomX cache, the VM and the socket are live, all released on the normal path only, and every get_str() throws on a type mismatch -- so a malformed message leaked 256 MB and the fd. Body wrapped in try/catch: ignore the line, keep mining. - Caller-supplied timeout clamped to [1, 3600]; it was unbounded, pinning an RPC worker and the cache indefinitely. okSafeMode -> false. Verified against a hostile stratum server on regtest: - 4 malformed mining.notify payloads (short hex, non-hex, wrong JSON type, 31 bytes) -> all rejected, daemon alive, 0 assertions. The 31-byte case is the one that previously hit the uint256 assert. - valid job first (so RandomX actually allocates) then garbage mid-mine -> RSS delta +2.2 MB, i.e. cache and VM released, not the ~256 MB a leak leaves. - regtest exemption confirmed: stratummine runs past the gate there. The non-regtest refusal path is by code reading only -- a testnet node on this host collides with the production daemon's RPC port, so it was not exercised. hush_utils.h: stop injecting the mainnet node1-node10.dragonx.is addnode seeds when -regtest or -testnet is set. regtest reuses mainnet's network magic, so a supposedly isolated node was handshaking production peers and pulling their headers into its own index. Gated at the injection site only -- isdragonx itself must stay true, because it also selects ac_private, ac_algo, blocktime and the reward/halving schedule (an earlier version of this patch gated isdragonx itself and silently turned ac_private off on regtest). hush_args() runs between ParseParameters() and ReadConfigFile() (bitcoind.cpp:115/144/158), so this sees a command-line -regtest, as qa/rpc-tests uses, but not a config-file regtest=1. hush_nSPV_fullnode.h: initialize `filter` at both sites. It was assigned only on the len-11 request form; the other two passed uninitialized stack memory to NSPV_getaddressutxos/NSPV_getaddresstxids, remotely reachable since HUSH_NSPV_FULLNODE is on by default. getblocktemplate_proposals.py still passes. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01FU87LdsJZiZkfq1eXubpeo --- src/hush_nSPV_fullnode.h | 4 +-- src/hush_utils.h | 16 +++++++++-- src/rpc/mining.cpp | 60 ++++++++++++++++++++++++++++++++++++---- 3 files changed, 70 insertions(+), 10 deletions(-) diff --git a/src/hush_nSPV_fullnode.h b/src/hush_nSPV_fullnode.h index 9287c2103..e7efd144d 100644 --- a/src/hush_nSPV_fullnode.h +++ b/src/hush_nSPV_fullnode.h @@ -660,7 +660,7 @@ void hush_nSPVreq(CNode *pfrom,std::vector request) // received a reque struct NSPV_utxosresp U; 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; + int32_t skipcount = 0; char coinaddr[64]; uint32_t filter = 0; uint8_t isCC = 0; // only assigned on the len-11 form; the other two passed it on uninitialized memcpy(coinaddr,&request[2],request[1]); // request[1] < 64 bounds the copy + the terminator write coinaddr[request[1]] = 0; if ( request[1] == len-3 ) @@ -698,7 +698,7 @@ void hush_nSPVreq(CNode *pfrom,std::vector request) // received a reque struct NSPV_txidsresp T; 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; + int32_t skipcount = 0; char coinaddr[64]; uint32_t filter = 0; uint8_t isCC = 0; // only assigned on the len-11 form; the other two passed it on uninitialized memcpy(coinaddr,&request[2],request[1]); // request[1] < 64 bounds the copy + the terminator write coinaddr[request[1]] = 0; if ( request[1] == len-3 ) diff --git a/src/hush_utils.h b/src/hush_utils.h index 6241b93ba..3ff16791a 100644 --- a/src/hush_utils.h +++ b/src/hush_utils.h @@ -1783,11 +1783,21 @@ void hush_args(char *argv0) fprintf(stderr,".oO Starting %s Full Node (Extreme Privacy!) with genproc=%d notary=%d\n",name.c_str(),HUSH_MININGTHREADS, IS_HUSH_NOTARY); vector DRAGONX_nodes = {}; - // Only DRAGONX connects to these by default, other chains must opt-in via -connect/-addnode + // Only DRAGONX connects to these by default, other chains must opt-in via -connect/-addnode. + // Never on regtest or testnet: regtest reuses mainnet's network magic, so injecting the mainnet + // seeds here makes a supposedly isolated node handshake production peers and pull their headers + // into its own index -- which is exactly what it did, and why multi-node rpc-tests were talking + // to the live chain. NOTE: hush_args() runs between ParseParameters() and ReadConfigFile() + // (bitcoind.cpp:115/144/158), so this sees a command-line -regtest (how qa/rpc-tests starts + // nodes) but NOT a bare "regtest=1" in the config file. const bool isdragonx = strncmp(name.c_str(), "DRAGONX",7) == 0 ? true : false; - LogPrint("net", "%s: isdragonx=%d\n", __func__, isdragonx); - if (isdragonx) { + // Seed injection only -- isdragonx itself must stay true here, because it also selects + // ac_private, ac_algo, blocktime and the reward/halving schedule below. + const bool isnotmainnet = GetBoolArg("-regtest", false) || GetBoolArg("-testnet", false); + + LogPrint("net", "%s: isdragonx=%d isnotmainnet=%d\n", __func__, isdragonx, isnotmainnet); + if (isdragonx && !isnotmainnet) { // node8-node10 are PLACEHOLDERS with no DNS records yet. A hostname that // does not resolve is harmless here: ThreadOpenAddedConnections just fails // to open the connection and retries on its 2-minute cycle. Reserving the diff --git a/src/rpc/mining.cpp b/src/rpc/mining.cpp index 5d6fb318e..0d9f15674 100644 --- a/src/rpc/mining.cpp +++ b/src/rpc/mining.cpp @@ -1112,6 +1112,27 @@ static bool StratumMinerRecvLines(int fd, std::string& buf, int timeout_ms, std: // authorize, receive work + the per-height RandomX key, then vary the block nNonce, hash with // RandomX (byte-identical to CheckRandomXSolution via GetRandomXInput), and submit a 32-byte // solution when the block hash meets target. Exists to validate the -stratum RandomX pool path. +//! Upper bound on how long stratummine will hold an RPC worker thread (and a 256 MB RandomX +//! cache). An unbounded caller-supplied deadline pins both indefinitely. +static const int64_t MAX_STRATUMMINE_TIMEOUT = 3600; + +//! Parse a 64-char hex field from mining.notify into a uint256. +//! These fields come from whatever host the operator pointed us at, and both primitives below are +//! unforgiving: uint256's vector constructor asserts on a wrong-size input (uint256.cpp:30, and +//! NDEBUG is never defined for this build so the assert is live in release), while ParseHex +//! silently truncates at the first non-hex character. A short or garbled field would therefore +//! abort the daemon rather than be rejected. Returns false instead. +static bool StratumHex256(const std::string& hex, uint256& out) +{ + if (hex.size() != 64 || !IsHex(hex)) + return false; + std::vector v = ParseHex(hex); + if (v.size() != 32) + return false; + out = uint256(v); + return true; +} + UniValue stratummine(const UniValue& params, bool fHelp, const CPubKey& mypk) { if (fHelp || params.size() < 2 || params.size() > 4) @@ -1129,10 +1150,24 @@ UniValue stratummine(const UniValue& params, bool fHelp, const CPubKey& mypk) if (ASSETCHAINS_ALGO != ASSETCHAINS_RANDOMX) throw JSONRPCError(RPC_MISC_ERROR, "stratummine only supports RandomX chains"); + // This is a reference miner for exercising -stratum, not a production facility: it dials an + // operator-supplied host, blocks an RPC worker for the whole run, and parses whatever that host + // chooses to send back. Require an explicit opt-in, and exempt regtest so the test suite can + // still drive it. NOTE: do NOT gate this on fExperimentalMode -- that defaults to TRUE + // (init.cpp:1195), so it would leave the RPC exposed on every node and the gate would be a no-op. + if (!GetBoolArg("-stratummine", false) && Params().NetworkIDString() != "regtest") + throw JSONRPCError(RPC_MISC_ERROR, + "stratummine is a test-only reference miner and is disabled by default; " + "restart with -stratummine to enable it"); + const std::string host = params[0].get_str(); const int port = params[1].get_int(); const std::string addr = params.size() > 2 ? params[2].get_str() : "x"; - const int64_t timeout = params.size() > 3 ? params[3].get_int64() : 120; + int64_t timeout = params.size() > 3 ? params[3].get_int64() : 120; + if (timeout < 1) + timeout = 1; + if (timeout > MAX_STRATUMMINE_TIMEOUT) + timeout = MAX_STRATUMMINE_TIMEOUT; const int64_t deadline = GetTime() + timeout; // connect (blocking TCP) @@ -1166,6 +1201,11 @@ UniValue stratummine(const UniValue& params, bool fHelp, const CPubKey& mypk) uint256 hashPrevBlock, hashMerkleRoot, hashReserved; auto processLine = [&](const std::string& line) { + // Every get_str()/get_int() below throws on a type mismatch, and this lambda runs inside the + // window where the RandomX cache and VM are allocated and the socket is open -- all of which + // are released only on the normal path. A malformed server message must therefore never + // escape from here, or it leaks 256 MB and the fd on its way out. + try { UniValue v; if (!v.read(line)) return; const UniValue& id = find_value(v, "id"); @@ -1185,16 +1225,26 @@ UniValue stratummine(const UniValue& params, bool fHelp, const CPubKey& mypk) poolTarget = UintToArith256(uint256S(p[0].get_str())); haveTarget = true; } else if (m == "mining.notify" && p.size() >= 7) { + // Validate every fixed-width field before committing any of it, so a malformed job is + // ignored outright rather than half-applied over the previous one. + uint256 prev, merkle, reserved; + if (!StratumHex256(p[2].get_str(), prev) || + !StratumHex256(p[3].get_str(), merkle) || + !StratumHex256(p[4].get_str(), reserved)) + return; jobId = p[0].get_str(); nVersion = bswap_32((uint32_t)strtoul(p[1].get_str().c_str(), NULL, 16)); - hashPrevBlock = uint256(ParseHex(p[2].get_str())); - hashMerkleRoot = uint256(ParseHex(p[3].get_str())); - hashReserved = uint256(ParseHex(p[4].get_str())); + hashPrevBlock = prev; + hashMerkleRoot = merkle; + hashReserved = reserved; timeHex = p[5].get_str(); nTime = bswap_32((uint32_t)strtoul(timeHex.c_str(), NULL, 16)); nBits = bswap_32((uint32_t)strtoul(p[6].get_str().c_str(), NULL, 16)); haveJob = true; } + } catch (const std::exception&) { + // Malformed message from the server: ignore the line and keep mining. + } }; std::string buf; @@ -1302,7 +1352,7 @@ static const CRPCCommand commands[] = { // category name actor (function) okSafeMode // --------------------- ------------------------ ----------------------- ---------- #ifndef WIN32 - { "mining", "stratummine", &stratummine, true }, + { "mining", "stratummine", &stratummine, false }, #endif { "mining", "getlocalsolps", &getlocalsolps, true }, { "mining", "getnetworksolps", &getnetworksolps, true },