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

[M-2] Failed buy transactions for a valid listing, because the seller can transfer the listed token to a different address before someone buys it.

Keyword

[M-2] Failed buy transactions for a valid listing, because the seller can transfer the listed token to a different address before someone buys it.

Description: The MartenitsaMarketplace::listMartenitsaForSale function doesn't transfer the listed tokens from the seller to the contract. Because of this, a seller can list a token for sale, and then transfer it to a different address. If they don't cancel the listing, the buyer will think that this is a legitimate listing. When they will try to buy this particular token, they will get a failed transaction because the safeTransferFrom inside MartenitsaMarketplace::buyMartenitsa will fail.

Impact: Buyers will incur gas fee losses and keep getting failed transactions for what they deem as being a valid listing.

Proof of Concepts: Paste this test inside MartenitsaMarketplace.t.sol file and run it with the forge test --mt testBuyMartenitsaAfterTransfer -vvv command in order to see the logs. You will see that Chasy is able to transfer the token to Jack after listing it, and when Bob calls the buyMartenitsa function, the function will revert once it reaches the safeTransferFrom line of code because Chasy is not holding the token anymore.

Proof of Code
function testBuyMartenitsaAfterTransfer() public {
vm.startPrank(chasy);
martenitsaToken.createMartenitsa("bracelet");
marketplace.listMartenitsaForSale(0, 1 wei);
martenitsaToken.approve(address(marketplace), 0);
uint256 chasyBalanceBefore = martenitsaToken.balanceOf(chasy);
console2.log("Chasy's balance before is", chasyBalanceBefore);
martenitsaToken.transferFrom(chasy, jack, 0);
uint256 jackBalance = martenitsaToken.balanceOf(jack);
console2.log("Jack's balance is", jackBalance);
uint256 chasyBalanceAfter = martenitsaToken.balanceOf(chasy);
console2.log("Chasy's balance after is", chasyBalanceAfter);
vm.stopPrank();
vm.prank(bob);
marketplace.buyMartenitsa{value: 1 wei}(0);
}

Test output

Logs:
//..
//..
//..
@> ├─ [67871] MartenitsaMarketplace::buyMartenitsa{value: 1}(0)
│ ├─ [23585] MartenitsaToken::updateCountMartenitsaTokensOwner(bob: [0x1D96F2f6BeF1202E4Ce1Ff6Dad0c2CB002861d3e], "add")
│ │ └─ ← ()
│ ├─ [2143] MartenitsaToken::updateCountMartenitsaTokensOwner(chasy: [0x4f19A0AcB98b7aA30b0138D26e5E5F9634F32F5A], "sub")
│ │ └─ ← ()
│ ├─ emit MartenitsaSold(tokenId: 0, buyer: bob: [0x1D96F2f6BeF1202E4Ce1Ff6Dad0c2CB002861d3e], price: 1)
│ ├─ [0] chasy::fallback{value: 1}()
│ │ └─ ← ()
@> │ ├─ [3735] MartenitsaToken::safeTransferFrom(chasy: [0x4f19A0AcB98b7aA30b0138D26e5E5F9634F32F5A], bob: [0x1D96F2f6BeF1202E4Ce1Ff6Dad0c2CB002861d3e], 0)

Recommended mitigation: When listing the token this should be transferred from the seller to the Marketplace contract in order to prevent sellers from transferring them out before the actual sale or before they cancel their listing.

Tools used: Manual review

Updates

Lead Judging Commences

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

Listed MartenitsaToken can be transferred before the sale

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

Listed MartenitsaToken can be transferred before the sale

Support

FAQs

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