Skip to content

Commit fac15ff

Browse files
author
MacroFake
committed
Fix logical race in rest_getutxos
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.
1 parent fa97a52 commit fac15ff

File tree

1 file changed

+9
-5
lines changed

1 file changed

+9
-5
lines changed

src/rest.cpp

+9-5
Original file line numberDiff line numberDiff line change
@@ -784,14 +784,18 @@ static bool rest_getutxos(const std::any& context, HTTPRequest* req, const std::
784784
ChainstateManager* maybe_chainman = GetChainman(context, req);
785785
if (!maybe_chainman) return false;
786786
ChainstateManager& chainman = *maybe_chainman;
787+
decltype(chainman.ActiveHeight()) active_height;
788+
uint256 active_hash;
787789
{
788-
auto process_utxos = [&vOutPoints, &outs, &hits](const CCoinsView& view, const CTxMemPool* mempool) {
790+
auto process_utxos = [&vOutPoints, &outs, &hits, &active_height, &active_hash, &chainman](const CCoinsView& view, const CTxMemPool* mempool) EXCLUSIVE_LOCKS_REQUIRED(chainman.GetMutex()) {
789791
for (const COutPoint& vOutPoint : vOutPoints) {
790792
Coin coin;
791793
bool hit = (!mempool || !mempool->isSpent(vOutPoint)) && view.GetCoin(vOutPoint, coin);
792794
hits.push_back(hit);
793795
if (hit) outs.emplace_back(std::move(coin));
794796
}
797+
active_height = chainman.ActiveHeight();
798+
active_hash = chainman.ActiveTip()->GetBlockHash();
795799
};
796800

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

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

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

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

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

849853
UniValue utxos(UniValue::VARR);

0 commit comments

Comments
 (0)