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

Seller in `MartenitsaMarketplace::listMartenitsaForSale` might not approve the token, leading to DoS issue in `MartenitsaMarketplace::listMartenitsaForSale`

0xlouistsai

Summary

The producer can list their Martenitsa token for sale through MartenitsaMarketplace::listMartenitsaForSale, but they do not approve their token to the MartenitsaMarketplace contract atomically in the function. When a buyer wants to buy the token through MartenitsaMarketplace::listMartenitsaForSale, the transaction will fail due to insufficient token approval, leading to denial of service issue.

Vulnerability Details

Consider the following process of listing and buying Martenitsa token.

  1. Chasy is one of the producer and create a Martenitsa token through martenitsaToken::createMartenitsa.

  2. Chasy then list the token for sale through MartenitsaMarketplace::listMartenitsaForSale, but does not approve the token to the MartenitsaMarketplace contract.

  3. Another user, Bob, notices the listing token and buys the token through MartenitsaMarketplace::buyMartenitsa

  4. The transaction will revert since the MartenitsaMarketplace does not have the token approval and thus not able to execute safeTransferFrom, from Chasy to Bob.

Add the following proof-of-concept in MartenitsaMarketplace.t.sol, and execute the command forge test --mt testDenialOfServiceWhenTokenNotApproveInListing. It will succeed, indicating no user is able to buy the token although it is currently for sale due to insufficient approval.

The proof-of-concept:

function testDenialOfServiceWhenTokenNotApproveInListing() public createMartenitsa {
// Chasy owns ID 0 martenitsa token
address owner = martenitsaToken.ownerOf(0);
assertEq(owner, chasy);
​
// Chasy list the token for sale but forget to approve
vm.prank(chasy);
marketplace.listMartenitsaForSale(0, 1 wei);
​
vm.deal(bob, 1 wei);
​
// Bob wants to buy the token but will revert
vm.prank(bob);
vm.expectRevert();
marketplace.buyMartenitsa{value: 1 wei}(0);
}

Reverts in the safeTransferFrom function due to insufficient approval:

​
│ ├─ [5735] MartenitsaToken::safeTransferFrom(chasy: [0x4f19A0AcB98b7aA30b0138D26e5E5F9634F32F5A], bob: [0x1D96F2f6BeF1202E4Ce1Ff6Dad0c2CB002861d3e], 0)
│ │ └─ ← [Revert] ERC721InsufficientApproval(0xF62849F9A0B5Bf2913b396098F7c7019b51A820a, 0)
│ └─ ← [Revert] ERC721InsufficientApproval(0xF62849F9A0B5Bf2913b396098F7c7019b51A820a, 0)
└─ ← [Stop]

Impact

The buyers are unable to buy martenitsaToken although the token is for sale, and the operation will waste their gas.

Tools Used

Manual Review, Foundry testing

Recommendations

When seller wants to list their token for sale, they should also approve their token. This feature can be updated in the MartenitsaMarketplace::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");
+ // token approval
+ martenitsaToken.approve(address(this), tokenId);
// 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:

Lack of approval

Support

FAQs

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