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

Incorrect Assumption about Inheritance renders `MartenitsaEvent` contract unable to interact with `MartenitsaMarketplace` or `MartenitsaToken`

codexbugmenot

Summary

The MartenitsaEvent contract is designed to work with ERC721 MartenitsaToken
but instead of initializing it in constructor the contract inherits MartenitsaToken creating a new token contract with address of MartenitsaEvent which is not the same as the original MartenitsaToken contract, rendering this contract unable to interact with MartenitsaMarketplace or MartenitsaToken

Impact

  1. The MartenistaEvent::isProducer(it has one since it inherited MartenitsaToken) is not same as MartenitsaToken::isProducer hence all the producers can join event.

  2. Users who joins the event using MartenitsaEvent::joinEventfunction cannot list martenitsas using MartenitsaNarketplace::listMartenitsaForSale function.

  3. The MartenitsaEvent::_addProducer internal function which is called inside MartenitsaEvent::joinEvent adds a user to MartenitsaEvent::isProducer mapping not MartenitsaToken::isProducer mapping.

Proof of Concept

Add the following PoC's to MartenitsaEvent.t.sol

  1. Any producer can join the event

PoC: Join Event Producer
​
function testJoinEventProducer() public activeEvent {
uint256 requirement = 10 ** 18;
​
// giving chasy a health token (assuming that she won it in vote)
vm.prank(address(marketplace));
healthToken.distributeHealthToken(chasy, requirement);
// chasy here is a producer normally she shouldnt be able to join event
// but because of wrong inheritance assumption chasy can join theEvent
vm.startPrank(chasy);
healthToken.approve(address(martenitsaEvent), 10 ** 18);
martenitsaEvent.joinEvent();
vm.stopPrank();
}
​
​

  1. Users who join event cannot list martenitsas

PoC:Even After Join Event User Cannot List Martenitsa
function testEvenAfterJoinEventUserCannotListMartenitsa() public activeEvent eligibleForReward {
uint256 requirement = 10 ** 18;
​
vm.startPrank(bob);
marketplace.collectReward();
healthToken.approve(address(martenitsaEvent), requirement);
martenitsaEvent.joinEvent();
vm.stopPrank();
​
// this isproducer belongs to martenitsa which is not same as the one in MartenitsaToken
assert(martenitsaEvent.isProducer(bob) == true);
​
vm.startPrank(bob);
​
//After joining event
​
martenitsaEvent.createMartenitsa("bracelet");
vm.expectRevert();
marketplace.listMartenitsaForSale(0, 1 wei);
​
martenitsaEvent.approve(address(marketplace), 0);
​
vm.stopPrank();
}

  1. The MartenitsaEvent::_addProducer internal function adds a user to MartenitsaEvent::isProducer mapping not MartenitsaToken::isProducer mapping

PoC:Different IsProducer
function testDifferentIsProducer() public activeEvent eligibleForReward {
uint256 requirement = 10 ** 18;
​
vm.startPrank(bob);
marketplace.collectReward();
healthToken.approve(address(martenitsaEvent), requirement);
martenitsaEvent.joinEvent();
vm.stopPrank();
​
// this isproducer belongs to martenitsa which is not same as the one in MartenitsaToken
​
// true in MartenitsaEvent contract
assert(martenitsaEvent.isProducer(bob) == true);
// False in MartenitsaToken contract
assert(martenitsaToken.isProducer(bob) == false);
}
​

Recommendations

There are two possible mitigations and both demand major changes

  1. The MartenitsaEvent contract requires an entire archhitectural rework since the assumption about the inheritance is wrongly implemented and also the test suites use MartenitsaToken as the base token contract , the changes should be implemented in such a way that MartenitsaEvent contract doesn't inherit MartenitsaToken but initialises its address in constructor as a state variable and rest of the changes in all contracts and test suites must be done with this assumption.

  2. Using MartenitsaEvent contract as the base token contract can work but it requires several changes in all other contracts and test suites rewritten.

Updates

Lead Judging Commences

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

MartenitsaToken and MartenitsaEvent have different addresses

Support

FAQs

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