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

[M-1] Buyers can lose money if they call the `MartenitsaMarketplace::buyMartenitsa` function with a `msg.value` amount that is higher than the `listing.price`.

Keyword

[M-1] Buyers can lose money if they call the MartenitsaMarketplace::buyMartenitsa function with a msg.value amount that is higher than the listing.price.

Description: The MartenitsaMarketplace::buyMartenitsa function requires msg.value >= listing.price. If Chasy lists one item with a price of 1 Wei, and Bob wants to buy the item, but he accidentaly sends 5 Wei instead of 1, the transaction will not revert. Chasy will sell the item and she will receive 1 Wei, Bob will pay 5 Wei instead of 1, and the MartenitsaMarketplace contract will receive the extra 4.

Impact: Users can accidentally lose funds because the transaction will not revert if they call the MartenitsaMarketplace::buyMartenitsa with a msg.value amount that is > listing.price.

Proof of Concepts: Paste this test inside MartenitsaMarketplace.t.sol file.

Proof of Code
function testBuyMartenitsaWrongAmount() public {
vm.startPrank(chasy);
martenitsaToken.createMartenitsa("bracelet");
marketplace.listMartenitsaForSale(0, 1 wei);
martenitsaToken.approve(address(marketplace), 0);
vm.stopPrank();
uint256 balance = bob.balance;
console2.log("Bob's balance before sale is", balance);
uint256 marketplaceBalanceBefore = address(marketplace).balance;
console2.log("Marketplace balance before sale is", marketplaceBalanceBefore);
vm.prank(bob);
marketplace.buyMartenitsa{value: 5 wei}(0);
uint256 balance2 = bob.balance;
console2.log("Bob's balance after sale is", balance2);
uint256 marketplaceBalanceAfter = address(marketplace).balance;
console2.log("Marketplace balance after sale is", marketplaceBalanceAfter);
​
assert(martenitsaToken.ownerOf(0) == bob);
assert(martenitsaToken.getCountMartenitsaTokensOwner(bob) == 1);
assert(martenitsaToken.getCountMartenitsaTokensOwner(chasy) == 0);
}

Test output

Logs:
Bob's balance before sale is 5000000000000000000
Marketplace balance before sale is 0
Bob's balance after sale is 4999999999999999995
Marketplace balance after sale is 4

Recommended mitigation: To prevent this from happening, the require statement inside the MartenitsaMarketplace::buyMartenitsa function should be changed to strict equality.

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");
+ require(msg.value == listing.price, "Insufficient funds");
//..
//..
//..
}

Tools used: Manual review

Updates

Lead Judging Commences

bube Lead Judge about 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.