Skip to content

Commit

Permalink
Fix logical race in rest_getutxos
Browse files Browse the repository at this point in the history
Calling ActiveHeight() and ActiveTip() subsequently without holding the
::cs_main lock over both calls may result in a height that does not
correspond to the tip due to a race.

Fix this by holding the lock.
  • Loading branch information
MacroFake committed Aug 16, 2022
1 parent fa97a52 commit fac15ff
Showing 1 changed file with 9 additions and 5 deletions.
14 changes: 9 additions & 5 deletions src/rest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -784,14 +784,18 @@ static bool rest_getutxos(const std::any& context, HTTPRequest* req, const std::
ChainstateManager* maybe_chainman = GetChainman(context, req);
if (!maybe_chainman) return false;
ChainstateManager& chainman = *maybe_chainman;
decltype(chainman.ActiveHeight()) active_height;
uint256 active_hash;
{
auto process_utxos = [&vOutPoints, &outs, &hits](const CCoinsView& view, const CTxMemPool* mempool) {
auto process_utxos = [&vOutPoints, &outs, &hits, &active_height, &active_hash, &chainman](const CCoinsView& view, const CTxMemPool* mempool) EXCLUSIVE_LOCKS_REQUIRED(chainman.GetMutex()) {
for (const COutPoint& vOutPoint : vOutPoints) {
Coin coin;
bool hit = (!mempool || !mempool->isSpent(vOutPoint)) && view.GetCoin(vOutPoint, coin);
hits.push_back(hit);
if (hit) outs.emplace_back(std::move(coin));
}
active_height = chainman.ActiveHeight();
active_hash = chainman.ActiveTip()->GetBlockHash();
};

if (fCheckMemPool) {
Expand Down Expand Up @@ -819,7 +823,7 @@ static bool rest_getutxos(const std::any& context, HTTPRequest* req, const std::
// serialize data
// use exact same output as mentioned in Bip64
CDataStream ssGetUTXOResponse(SER_NETWORK, PROTOCOL_VERSION);
ssGetUTXOResponse << chainman.ActiveChain().Height() << chainman.ActiveChain().Tip()->GetBlockHash() << bitmap << outs;
ssGetUTXOResponse << active_height << active_hash << bitmap << outs;
std::string ssGetUTXOResponseString = ssGetUTXOResponse.str();

req->WriteHeader("Content-Type", "application/octet-stream");
Expand All @@ -829,7 +833,7 @@ static bool rest_getutxos(const std::any& context, HTTPRequest* req, const std::

case RESTResponseFormat::HEX: {
CDataStream ssGetUTXOResponse(SER_NETWORK, PROTOCOL_VERSION);
ssGetUTXOResponse << chainman.ActiveChain().Height() << chainman.ActiveChain().Tip()->GetBlockHash() << bitmap << outs;
ssGetUTXOResponse << active_height << active_hash << bitmap << outs;
std::string strHex = HexStr(ssGetUTXOResponse) + "\n";

req->WriteHeader("Content-Type", "text/plain");
Expand All @@ -842,8 +846,8 @@ static bool rest_getutxos(const std::any& context, HTTPRequest* req, const std::

// pack in some essentials
// use more or less the same output as mentioned in Bip64
objGetUTXOResponse.pushKV("chainHeight", chainman.ActiveChain().Height());
objGetUTXOResponse.pushKV("chaintipHash", chainman.ActiveChain().Tip()->GetBlockHash().GetHex());
objGetUTXOResponse.pushKV("chainHeight", active_height);
objGetUTXOResponse.pushKV("chaintipHash", active_hash.GetHex());
objGetUTXOResponse.pushKV("bitmap", bitmapStringRepresentation);

UniValue utxos(UniValue::VARR);
Expand Down

0 comments on commit fac15ff

Please sign in to comment.