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

Transfers are not deactivated in `Healthtoken` and `MartenitsaToken`, breaking all the protocol.

n0kto

Description

Transfers are not allowed in the protocol. Only MartenitsaMarketplace can provide MartenitsaTokens, and only this contract and the MartenitsaVoting can provide HealthTokens. However, all transfer functions are available due to inheritance. Without overriding them, any user can transfer their tokens and break the protocol:

  • MartenitsaToken::updateCountMartenitsaTokensOwner are not called in any transfer function.

  • Have HealthToken without winning them.

  • A listed token can be sent to anyone and being sell without the consent of the new owner. Money goes to the first owner.

Risk

Likelyhood: High

  • Anyone, Anytime

Impact: High

  • countMartenitsaTokensOwner won't be updated, breaking the reward mechanism.

  • MartenitsaToken can be transfered and sell without the consent of the new owner.

  • HealthToken can be owned by anyone without winning them.

Proof of Concept

Foundry PoC to add in `HealthToken.t.sol`
function testTransferToken() public {
address attacker = makeAddr("attacker");
​
deal(address(healthToken), attacker, 1e18);
​
vm.startPrank(attacker);
healthToken.transfer(address(0x01), 1e18);
}

Recommended Mitigation

Override all transfer function to revert in both tokens, except:

  • HealthToken::transferFrom which has to work only if the MartenitsaEvent is the msg.sender

  • MartenitsaToken::safeTransferFrom which has to work only if the MartenitsaMarketplace is the msg.sender

Updates

Lead Judging Commences

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

ERC721 `transferFrom` not overriden

ERC20 `transfer` not overriden

Support

FAQs

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