Submission Details
Severity:
`msg.value` not properly handled in `buyMartenitsa` lead to loss of funds
Description
MartenitsaMarketplace::buyMartenitsa allows users to buy tokens paying the related price.
Problem is that the user can send more than the price but only the listed price will be sent to the producer, resulting to stuck funds in 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");
// 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);
}
Risk
Likelyhood: Low
When a user send more than the exact price.
Impact: High
Loss of funds.
Recommended Mitigation
Transfer the entire amount sent by the seller to the producer (example below).
Alternatively, require statement can check for the exact price or the function may send back the extra value to the buyer.
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}("");
+ (bool sent, ) = seller.call{value: msg.value}("");
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.