Submission Details
Severity:
The excess amount of listing.price is not refunded to the user in `buyMartenitsa()` function
Summary
The excess amount of msg.value is not refunded to the user in buyMartenitsa() function, Even worse, the protocol lacks a method to withdraw these funds.
Vulnerability Details
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);
}
If msg.value > listing.price, the excess amount of listing.price will lost.
POC:
function testExcessFundsBuyMartenitsa() public listMartenitsa {
vm.prank(chasy);
martenitsaToken.approve(address(marketplace), 0);
//check balance before
assert(chasy.balance == 0);
assert(bob.balance == 5 ether);
assert(address(marketplace).balance== 0);
//using 5 ether to buy , price is 1 wei
vm.prank(bob);
marketplace.buyMartenitsa{value: 5 ether}(0);
//check balance after
assert(chasy.balance == 1 wei);
assert(bob.balance == 0);
assert(address(marketplace).balance== (5 ether - 1 wei));
assert(martenitsaToken.ownerOf(0) == bob);
assert(martenitsaToken.getCountMartenitsaTokensOwner(bob) == 1);
assert(martenitsaToken.getCountMartenitsaTokensOwner(chasy) == 0);
}
Impact
High
Tools Used
Foundry
Recommendations
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");
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 over 1 year ago
Submission Judgement Published Assigned finding tags:
Excess ETH not refunded to the user
Rewards breakdown
nSLOC
239This contest has a flat reward structure with the following payouts:
100 EXP
20 EXP
2 EXP
Live
The contest is live. Earn rewards by submitting a finding.
Appeals
This is your time to appeal against judgements on your submissions.
Appeals Review
Appeals are being carefully reviewed by our judges.