First Flight #13: Baba Marta
First Flight #13
Beginner FriendlyFoundryGameFi
100 EXP
View results
Previous Next
Submission Details
Severity: high
Invalid

Reentrancy in MartenitsaMarketplace.buyMartenitsa(uint256) external calls before finalizing state.

vesper

Summary

Reentrancy in MartenitsaMarketplace.buyMartenitsa(uint256) external calls before finalizing state. The function makes external calls (martenitsaToken.updateCountMartenitsaTokensOwner) to update token ownership counts before it has completed updating the state of the contract.

Vulnerability Details

The listing corresponding to the token ID is deleted after the external calls. If reentrancy occurs, the function might operate on a state that has already been changed by the reentrant call.

martenitsaToken.updateCountMartenitsaTokensOwner(buyer, "add");
martenitsaToken.updateCountMartenitsaTokensOwner(seller, "sub");
​
// Clear the listing
delete tokenIdToListing[tokenId];

Impact

This could potentially allow reentrancy if the external contract called (martenitsaToken) is tricked into calling back into buyMartenitsa.

Tools Used

Slither

Recommendations

To prevent reentrancy attacks, you can apply the following techniques:

Checks-Effects-Interactions Pattern: Always update your contract's state (checks and effects) before calling external contracts (interactions). In buyMartenitsa, this means you should handle the deletion of the listing and any updates to internal state variables before making any external calls.

Use Reentrancy Guards: Implement a reentrancy guard, a simple state variable that can prevent a function from being called again while it's still executing. OpenZeppelin provides a ReentrancyGuard contract that you can use to secure your functions.

pragma solidity ^0.8.21;
​
import "@openzeppelin/contracts/security/ReentrancyGuard.sol";
​
contract MartenitsaMarketplace is Ownable, ReentrancyGuard {
// Previous contract code...
​
function buyMartenitsa(uint256 tokenId) external payable nonReentrant {
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;
​
// Update the listing before making external calls
delete tokenIdToListing[tokenId];
​
// Perform external calls
martenitsaToken.updateCountMartenitsaTokensOwner(buyer, "add");
martenitsaToken.updateCountMartenitsaTokensOwner(seller, "sub");
​
emit MartenitsaSold(tokenId, buyer, salePrice);
​
// Safe transfer of funds
(bool sent,) = seller.call{value: salePrice}("");
require(sent, "Failed to send Ether");
​
// Transfer the token to the buyer
martenitsaToken.safeTransferFrom(seller, buyer, tokenId);
}
​
// Remainder of contract...
}
Updates

Lead Judging Commences

bube Lead Judge over 1 year ago
Submission Judgement Published
Invalidated
Reason: Incorrect statement
Assigned finding tags:

Reentrancy

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.