Project

Summary

upgradeTier function should upgrade user from fromTierIndex to fromTierIndex + 1 but it wrongly updates to fromTierIndex -1

Vulnerability Details

user joins a Dao by mentioning daoMembershipAddress of Dao and tierIndex in which tier they wants to join,

then they should pay price of that paticular tier

https://github.com/Cyfrin/2024-11-one-world/blob/1e872c7ab393c380010a507398d4b4caca1ae32b/contracts/dao/MembershipFactory.sol#L143-L147

Then if they want to upgrade to higher tier then they use upgradeTier function,

which check weather the given dao has any tier higher than current tierIndex or not

https://github.com/Cyfrin/2024-11-one-world/blob/1e872c7ab393c380010a507398d4b4caca1ae32b/contracts/dao/MembershipFactory.sol#L157

which clearly states that the intension was to upgrade to a higher tier, means to (fromTierIndex+1) not to (fromTierIndex -1)

but the upgradeTier function mistakenly upgrades to fromTierIndex -1

https://github.com/Cyfrin/2024-11-one-world/blob/1e872c7ab393c380010a507398d4b4caca1ae32b/contracts/dao/MembershipFactory.sol#L159

Impact

As each tier has it's own price and as we can see when we are upgrading, it was burning 2 from current tier(fromTierIndex) and minting only 1, that means it indicates higher tiers have higher prices.

so user lossing his value which indirectly comes under lossing of user funds.

for example,

If

If a user has 2 tokens in tier 3 and he wants to upgrade,

initially value of those 2 tokens will be 40

case 1) If he upgrades to tier 4 then he has 1 token in tier 4 so value remains same(40)

case 2) if he upgrades to tier 2 then he has 1 token in tier 2 so value he gets is 10 (40-10=30) ,30 loss.

so in case 2 he loss his funds.

Tools Used

manual review

Recommendation

upgrade to fromTierIndex +1 not to fromTierIndex -1

at,

https://github.com/Cyfrin/2024-11-one-world/blob/1e872c7ab393c380010a507398d4b4caca1ae32b/contracts/dao/MembershipFactory.sol#L159

add this line

and delete this line