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

Lack of Refund Mechanism Can Result in Buyers Funds Getting Stuck in the Marketplace

pacelli

Summary

Buyers interested in purchasing a Martenitsa call the MartenitsaMarketplace::buyMartenitsa function to do so, the function checks the value sent in the transaction is greater or equal than the listed price, but in the situation where the buyer send more ETH than the price, the function does not refund the extra amount, considering the marketplace contract does not include a function to sweep ETH these funds will remain stuck in the contract.

// does not refund if msg.value > price
function buyMartenitsa(uint256 tokenId) external payable {
Listing memory 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;
​
martenitsaToken.updateCountMartenitsaTokensOwner(buyer, "add");
martenitsaToken.updateCountMartenitsaTokensOwner(seller, "sub");
​
// Clear the listing
delete tokenIdToListing[tokenId];
​
emit MartenitsaSold(tokenId, buyer, salePrice);
​
// Transfer funds to seller
(bool sent,) = seller.call{value: salePrice}("");
require(sent, "Failed to send Ether");
​
// Transfer the token to the buyer
martenitsaToken.safeTransferFrom(seller, buyer, tokenId);
}

Impact

Buyers overpaying for a Martenitsa will lose those funds.

Tools Used

Manual review, Foundry

Recommendations

There are two mitigation that could be implemented.

  • Instead of checking if msg.value is greater of equal to the price, check the buyer is paying the exact price.

- require(msg.value >= listing.price, "Insufficient funds");
+ require(msg.value == listing.price, "Offer must be equal to price");

  • Include a refund mechanism

function buyMartenitsa(uint256 tokenId) external payable {
// rest of the function...
​
+ uint256 offer = msg.value;
​
(bool sent,) = seller.call{value: salePrice}("");
require(sent, "Failed to send Ether");
​
+ if (offer > salePrice) {
+ (bool refundSent,) = buyer.call{value: offer - salePrice}("");
+ require(refundSent, "Failed to refund Ether");
+ }
​
// rest of the function...
}
Updates

Lead Judging Commences

bube Lead Judge over 1 year ago
Submission Judgement Published
Validated
Assigned finding tags:

Excess ETH not refunded to the user

Support

FAQs

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