Beginner FriendlyFoundryGameFi
100 EXP
View results
Submission Details
Severity: high
Valid

`ETH` may become permanently locked in the `MartenitsaMarketplace` contract

Summary

The function MartenitsaMarketplace::buyMartenitsa accepts ETH as payment for the token. However, users can inadvertently send more
ETH than required, leading to the excess ETH becoming permanently trapped in the contract.

The documentation does not specify whether the contract should refund the excess ETH, only accept the exact amount for the token purchase,
or allocate the remaining ETH to the protocol (There is no function provided to withdraw the trapped ETH).

Vulnerability Details

The function MartenitsaMarketplace::buyMartenitsa accepts ETH in exchange for tokens, requiring that the sent amount is >= the price
of the token. However, users can inadvertently send more ETH than required for the purchase. Only the token's price in ETH is
transferred to the seller, while the excess remains trapped inside the contract.

function buyMartenitsa(uint256 tokenId) external payable {
Listing memory listing = tokenIdToListing[tokenId];
require(listing.forSale, "Token is not listed for sale");
//@Audit: a user by mistake can send more eth
@> 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
//@Audit: only the price in ETH of the token is transfered
@> (bool sent, ) = seller.call{value: salePrice}("");
require(sent, "Failed to send Ether");
// Transfer the token to the buyer
martenitsaToken.safeTransferFrom(seller, buyer, tokenId);
}

A test is included to demonstrate a token purchase scenario, where a token listed at a price of 1 ETH is mistakenly bought with 2 ETH.
In this case, 1 ETH is correctly sent to the seller, while the remaining 1 ETH becomes trapped within the contract.

function testLockedETH() public {
vm.startPrank(chasy);
martenitsaToken.createMartenitsa("bracelet");
marketplace.listMartenitsaForSale(0, 1 ether);
martenitsaToken.approve(address(marketplace), 0);
vm.stopPrank();
vm.deal(bob, 2 ether);
vm.prank(bob);
marketplace.buyMartenitsa{value: 2 ether}(0);
assert(martenitsaToken.ownerOf(0) == bob);
console.log("::Marketplace ETH Balance : ", address(marketplace).balance / 10**18);
}

Output

Logs:
::Marketplace ETH Balance : 1

Impact

Excess ETH after token purchase remains indefinitely trapped in the contract

Tools Used

Foundry and manual review

Recommendations

It's not clear from the documentation whether a withdraw function should exist. If its necessity is uncertain, consider adding a check
to ensure users send the exact amount required for the purchase.

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, "Incorrect price, send the exact amount");
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);
}
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.