First Flight #13: Baba Marta

Vulnerability Details

After transferring funds to the seller using seller.call{value: salePrice}(""), the function immediately transfers the token to the buyer using martenitsaToken.safeTransferFrom(seller, buyer, tokenId). If the token transfer triggers a call to an external contract that can execute arbitrary code (a malicious fallback function), it could potentially re-enter the buyMartenitsa function before it completes, allowing the seller to re-enter and manipulate the state again

Impact

An attacker can potentially re-enter the buyMartenitsa function before it completes and manipulate the state

Recommendations

Ensure that all internal state changes are made before external calls. Transfer token to buyer first before updating their count states

function buyMartenitsa(uint256 tokenId) external payable {
Listing storage listing = tokenIdToListing[tokenId];
require(listing.forSale, "Token is not listed for sale");
require(msg.value >= listing.price, "Insufficient funds");

address seller = listing.seller;
address buyer = msg.sender;
uint256 salePrice = listing.price;

// Clear the listing
delete tokenIdToListing[tokenId];

martenitsaToken.safeTransferFrom(seller, buyer, tokenId);

// Update token counts after the transfer
martenitsaToken.updateCountMartenitsaTokensOwner(buyer, "add");
martenitsaToken.updateCountMartenitsaTokensOwner(seller, "sub");

// Transfer funds to seller after all state changes
(bool sent, ) = seller.call{value: salePrice}("");
require(sent, "Failed to send Ether");

// Emit event after all actions are completed
emit MartenitsaSold(tokenId, buyer, salePrice);

}