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

Calling `MartenitsaMarketplace::buyMartenitsa` should always fail because of missing approval.

0xm00k

Summary

MartenitsaMarketplace::buyMartenitsa should always fail because the Producer has not approved the Marketplace contract to transfer the martenitsa token on their behalf. The producers can know the exact moment when the martenitsa token is sold for them to approve the transfer.

Vulnerability Details

Whenever a buyer calls the MartenitsaMarketplace::buyMartenitsa function, because the Producer has not approved the Marketplace contract to transfer the martenitsa token on their behalf, the transaction should always fail. This is because the Marketplace contract does not have the required allowance to transfer the martenitsa token on behalf of the Producer. There's no approval in the Martenitsa::listMartenitsaForSale function.

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);
}
POC Add this test to `MartenitsaMarketplace.test.sol`:
function testBuyMartenitsaFailNoApproval() public listMartenitsa {
vm.prank(bob);
marketplace.buyMartenitsa{value: 1 wei}(0);
}

then run forge test --mt testBuyMartenitsaFailNoApproval and the result should FAIL:

Encountered 1 failing test in test/MartenitsaMarketplace.t.sol:MartenitsaMarketplace
[FAIL. Reason: ERC721InsufficientApproval(0xF62849F9A0B5Bf2913b396098F7c7019b51A820a, 0)] testBuyMartenitsaFailNoApproval() (gas: 314904)

Impact

A potential buyer can never buy a martenitsa token from the Marketplace contract because the Producer has not approved the Marketplace contract to transfer the martenitsa token on their behalf.

Tools Used

Foundry and Manual review.

Recommendations

The Producer should approve the Marketplace contract to transfer the martenitsa token on their behalf before listing the martenitsa token for sale.

add the following line to the listMartenitsaForSale function:

function listMartenitsaForSale(uint256 tokenId, uint256 price) external {
require(msg.sender == martenitsaToken.ownerOf(tokenId), "You do not own this token");
require(martenitsaToken.isProducer(msg.sender), "You are not a producer!");
require(price > 0, "Price must be greater than zero");
​
Listing memory newListing = Listing({
tokenId: tokenId,
seller: msg.sender,
price: price,
design: martenitsaToken.getDesign(tokenId),
forSale: true
});
​
tokenIdToListing[tokenId] = newListing;
emit MartenitsaListed(tokenId, msg.sender, price);
+ martenitsaToken.approve(address(this), tokenId);
}

then run forge test --mt testBuyMartenitsaFailNoApproval and the result should PASS:

Ran 1 test for test/MartenitsaMarketplace.t.sol:MartenitsaMarketplace
[PASS] testBuyMartenitsaFailNoApproval() (gas: 314904)
Updates

Lead Judging Commences

bube Lead Judge over 1 year ago
Submission Judgement Published
Validated
Assigned finding tags:

Lack of approval

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.