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");
@> 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");
delete tokenIdToListing[tokenId];
emit MartenitsaSold(tokenId, buyer, salePrice);
@> (bool sent, ) = seller.call{value: salePrice}("");
require(sent, "Failed to send Ether");
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);
}