fix(nspv/wallet): bound nSPV request buffers + fix uninitialized fee / null-deref
hush_nSPV_fullnode.h: bound the REMOTERPC method strcpy and json memcpy to their fixed buffers (method[64], json[11000]); add lower-length and memcpy-source bounds to the UTXOS/TXIDS coinaddr[64] copies and the MEMPOOL handler. These paths deserialize attacker-controlled request bytes -> stack overflow / OOB read. The nSPV server is opt-in via -nspv_msg (off by default; DragonX uses lightwalletd). rpc/blockchain.cpp: getchaintxstats null-checks pwalletMain (crash under -disablewallet). wallet/rpcwallet.cpp: z_sendmany initializes nFee to the default miners fee (was read uninitialized when no fee param supplied). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
@@ -417,7 +417,9 @@ int32_t NSPV_remoterpc(struct NSPV_remoterpcresp *ptr,char *json,int n)
|
|||||||
{
|
{
|
||||||
request.read(json,n);
|
request.read(json,n);
|
||||||
jreq.parse(request);
|
jreq.parse(request);
|
||||||
strcpy(ptr->method,jreq.strMethod.c_str());
|
// SECURITY (stack overflow): strMethod is attacker-controlled; bound the copy to the fixed buffer.
|
||||||
|
strncpy(ptr->method,jreq.strMethod.c_str(),sizeof(ptr->method)-1);
|
||||||
|
ptr->method[sizeof(ptr->method)-1] = '\0';
|
||||||
len+=sizeof(ptr->method);
|
len+=sizeof(ptr->method);
|
||||||
std::map<std::string, bool>::iterator it = nspv_remote_commands.find(jreq.strMethod);
|
std::map<std::string, bool>::iterator it = nspv_remote_commands.find(jreq.strMethod);
|
||||||
if (it==nspv_remote_commands.end())
|
if (it==nspv_remote_commands.end())
|
||||||
@@ -438,8 +440,10 @@ int32_t NSPV_remoterpc(struct NSPV_remoterpcresp *ptr,char *json,int n)
|
|||||||
{
|
{
|
||||||
rpc_result = JSONRPCReplyObj(result, NullUniValue, jreq.id);
|
rpc_result = JSONRPCReplyObj(result, NullUniValue, jreq.id);
|
||||||
response=rpc_result.write();
|
response=rpc_result.write();
|
||||||
memcpy(ptr->json,response.c_str(),response.size());
|
// SECURITY (stack overflow): clamp to the fixed json buffer.
|
||||||
len+=response.size();
|
size_t rlen = response.size(); if ( rlen > sizeof(ptr->json) ) rlen = sizeof(ptr->json);
|
||||||
|
memcpy(ptr->json,response.c_str(),rlen);
|
||||||
|
len+=rlen;
|
||||||
return (len);
|
return (len);
|
||||||
}
|
}
|
||||||
else throw JSONRPCError(RPC_MISC_ERROR, "Error in executing RPC on remote node");
|
else throw JSONRPCError(RPC_MISC_ERROR, "Error in executing RPC on remote node");
|
||||||
@@ -459,8 +463,10 @@ int32_t NSPV_remoterpc(struct NSPV_remoterpcresp *ptr,char *json,int n)
|
|||||||
rpc_result = JSONRPCReplyObj(NullUniValue,JSONRPCError(RPC_PARSE_ERROR, e.what()), jreq.id);
|
rpc_result = JSONRPCReplyObj(NullUniValue,JSONRPCError(RPC_PARSE_ERROR, e.what()), jreq.id);
|
||||||
response=rpc_result.write();
|
response=rpc_result.write();
|
||||||
}
|
}
|
||||||
memcpy(ptr->json,response.c_str(),response.size());
|
// SECURITY (stack overflow): the error path echoes attacker-controlled jreq.id; clamp to the buffer.
|
||||||
len+=response.size();
|
size_t rlen = response.size(); if ( rlen > sizeof(ptr->json) ) rlen = sizeof(ptr->json);
|
||||||
|
memcpy(ptr->json,response.c_str(),rlen);
|
||||||
|
len+=rlen;
|
||||||
return (len);
|
return (len);
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -651,10 +657,10 @@ void hush_nSPVreq(CNode *pfrom,std::vector<uint8_t> request) // received a reque
|
|||||||
if ( timestamp > pfrom->prevtimes[ind] )
|
if ( timestamp > pfrom->prevtimes[ind] )
|
||||||
{
|
{
|
||||||
struct NSPV_utxosresp U;
|
struct NSPV_utxosresp U;
|
||||||
if ( len < 64+5 && (request[1] == len-3 || request[1] == len-7 || request[1] == len-11) )
|
if ( 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;
|
int32_t skipcount = 0; char coinaddr[64]; uint8_t filter; uint8_t isCC = 0;
|
||||||
memcpy(coinaddr,&request[2],request[1]);
|
memcpy(coinaddr,&request[2],request[1]); // request[1] < 64 bounds the copy + the terminator write
|
||||||
coinaddr[request[1]] = 0;
|
coinaddr[request[1]] = 0;
|
||||||
if ( request[1] == len-3 )
|
if ( request[1] == len-3 )
|
||||||
isCC = (request[len-1] != 0);
|
isCC = (request[len-1] != 0);
|
||||||
@@ -691,10 +697,10 @@ void hush_nSPVreq(CNode *pfrom,std::vector<uint8_t> request) // received a reque
|
|||||||
if ( timestamp > pfrom->prevtimes[ind] )
|
if ( timestamp > pfrom->prevtimes[ind] )
|
||||||
{
|
{
|
||||||
struct NSPV_txidsresp T;
|
struct NSPV_txidsresp T;
|
||||||
if ( len < 64+5 && (request[1] == len-3 || request[1] == len-7 || request[1] == len-11) )
|
if ( 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; uint8_t isCC = 0;
|
||||||
memcpy(coinaddr,&request[2],request[1]);
|
memcpy(coinaddr,&request[2],request[1]); // request[1] < 64 bounds the copy + the terminator write
|
||||||
coinaddr[request[1]] = 0;
|
coinaddr[request[1]] = 0;
|
||||||
if ( request[1] == len-3 )
|
if ( request[1] == len-3 )
|
||||||
isCC = (request[len-1] != 0);
|
isCC = (request[len-1] != 0);
|
||||||
@@ -732,7 +738,7 @@ void hush_nSPVreq(CNode *pfrom,std::vector<uint8_t> request) // received a reque
|
|||||||
if ( timestamp > pfrom->prevtimes[ind] )
|
if ( timestamp > pfrom->prevtimes[ind] )
|
||||||
{
|
{
|
||||||
struct NSPV_mempoolresp M; char coinaddr[64];
|
struct NSPV_mempoolresp M; char coinaddr[64];
|
||||||
if ( len < sizeof(M)+64 )
|
if ( len >= 40 && len < sizeof(M)+64 ) // SECURITY: lower bound guards the fixed-offset reads request[1..39]
|
||||||
{
|
{
|
||||||
int32_t vout; uint256 txid; uint8_t funcid,isCC = 0;
|
int32_t vout; uint256 txid; uint8_t funcid,isCC = 0;
|
||||||
n = 1;
|
n = 1;
|
||||||
@@ -741,7 +747,7 @@ void hush_nSPVreq(CNode *pfrom,std::vector<uint8_t> request) // received a reque
|
|||||||
n += dragon_rwnum(0,&request[n],sizeof(vout),&vout);
|
n += dragon_rwnum(0,&request[n],sizeof(vout),&vout);
|
||||||
n += dragon_rwbignum(0,&request[n],sizeof(txid),(uint8_t *)&txid);
|
n += dragon_rwbignum(0,&request[n],sizeof(txid),(uint8_t *)&txid);
|
||||||
slen = request[n++];
|
slen = request[n++];
|
||||||
if ( slen < 63 )
|
if ( slen < 63 && n + slen <= len ) // SECURITY: bound the memcpy source read within request
|
||||||
{
|
{
|
||||||
memcpy(coinaddr,&request[n],slen), n += slen;
|
memcpy(coinaddr,&request[n],slen), n += slen;
|
||||||
coinaddr[slen] = 0;
|
coinaddr[slen] = 0;
|
||||||
|
|||||||
@@ -1654,7 +1654,7 @@ UniValue getchaintxstats(const UniValue& params, bool fHelp, const CPubKey& mypk
|
|||||||
ret.pushKV("deshielding_payments", (int64_t)pindex->nChainDeshieldingPayments);
|
ret.pushKV("deshielding_payments", (int64_t)pindex->nChainDeshieldingPayments);
|
||||||
ret.pushKV("shielding_payments", (int64_t)pindex->nChainShieldingPayments);
|
ret.pushKV("shielding_payments", (int64_t)pindex->nChainShieldingPayments);
|
||||||
|
|
||||||
int64_t nullifierCount = pwalletMain->NullifierCount();
|
int64_t nullifierCount = pwalletMain ? pwalletMain->NullifierCount() : 0; // null under -disablewallet
|
||||||
//TODO: this is unreliable, is only a cache or subset of total nullifiers
|
//TODO: this is unreliable, is only a cache or subset of total nullifiers
|
||||||
ret.pushKV("nullifiers", (int64_t)nullifierCount);
|
ret.pushKV("nullifiers", (int64_t)nullifierCount);
|
||||||
ret.pushKV("shielded_pool_size", (int64_t)(pindex->nChainShieldedOutputs - pindex->nChainShieldedSpends));
|
ret.pushKV("shielded_pool_size", (int64_t)(pindex->nChainShieldedOutputs - pindex->nChainShieldedSpends));
|
||||||
|
|||||||
@@ -5146,7 +5146,7 @@ UniValue z_sendmany(const UniValue& params, bool fHelp, const CPubKey& mypk)
|
|||||||
}
|
}
|
||||||
|
|
||||||
//GOAL: choose one random zaddress with enough funds
|
//GOAL: choose one random zaddress with enough funds
|
||||||
CAmount nFee;
|
CAmount nFee = ASYNC_RPC_OPERATION_DEFAULT_MINERS_FEE; // default when params.size()<=3 (was uninitialized)
|
||||||
if (params.size() > 3) {
|
if (params.size() > 3) {
|
||||||
if (params[3].get_real() == 0.0) {
|
if (params[3].get_real() == 0.0) {
|
||||||
nFee = 0;
|
nFee = 0;
|
||||||
|
|||||||
Reference in New Issue
Block a user