Project
One World
NFTDeFi
15,000 USDC
View results
Previous Next
Submission Details
Severity: medium
Invalid

Incorrect logic in `MembershipFactory::upgradeTier` deducts the user's current tier tokens and grants a token for a lower tier

safdie

Summary

The upgradeTier function in the DAO contract has incorrect logic, which may allow users to effectively "downgrade" rather than upgrade their tier due to a logical flaw in the index handling. Instead of allowing a user to advance to a higher tier, the function deducts the user's current tier tokens and grants a token for a lower tier. This behavior deviates from the intended function of allowing users to upgrade within the DAO's tier structure.

Vulnerability Details

The purpose of the upgradeTier function is to allow users to advance to a higher membership level within a DAO by upgrading their tier. However, due to a mistake in the code, the function does the opposite of what is intended—it effectively downgrades the user to a lower tier.

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

function upgradeTier(address daoMembershipAddress, uint256 fromTierIndex) external {
require(daos[daoMembershipAddress].daoType == DAOType.SPONSORED, "Upgrade not allowed.");
require(daos[daoMembershipAddress].noOfTiers >= fromTierIndex + 1, "No higher tier available.");
IMembershipERC1155(daoMembershipAddress).burn(_msgSender(), fromTierIndex, 2);
IMembershipERC1155(daoMembershipAddress).mint(_msgSender(), fromTierIndex - 1, 1); // Incorrect logic
emit UserJoinedDAO(_msgSender(), daoMembershipAddress, fromTierIndex - 1);
}

In the above code, the mint function is called with fromTierIndex - 1, which actually mints the token for a lower tier than the user currently holds. For example, if a user is at fromTierIndex = 2, they would expect an upgrade to fromTierIndex = 3. However, the code mistakenly issues a token for fromTierIndex = 1, effectively downgrading the user.

PoC:

  • Setup the contract with two DAO tiers: Tier 1 and Tier 2.

  • User joins the DAO at Tier 1.

  • User attempts to upgrade to Tier 2.

  • Observe that the user is incorrectly downgraded to a lower tier.

// SPDX-License-Identifier: MIT
pragma solidity ^0.8.0;
​
import "forge-std/Test.sol";
import "src/DAOContract.sol";
​
contract UpgradeTierTest is Test {
DAOContract daoContract;
address user;
​
function setUp() public {
// Deploy the contract and create a DAO with two tiers
daoContract = new DAOContract();
user = address(1);
daoContract.createDAO(address(this), "SampleDAO", 2, [100, 200], address(this));
// User joins DAO at Tier 1
vm.startPrank(user);
daoContract.joinDAO(address(this), 0);
vm.stopPrank();
}
​
function testUpgradeTierDowngradeBug() public {
// User attempts to upgrade to Tier 2
vm.startPrank(user);
daoContract.upgradeTier(address(this), 0);
​
// Verify user was downgraded instead of upgraded
uint256 newTier = daoContract.getUserTier(address(this), user);
assertEq(newTier, 0, "User should have been downgraded to Tier 0 due to bug");
vm.stopPrank();
}
}

Impact

The incorrect logic in the upgradeTier function significantly impacts users attempting to advance within the DAO. Instead of upgrading, users will find themselves mistakenly assigned to a lower tier, which could lead to:

  • Confusion and frustration among DAO members.

  • Potential financial loss if DAO members have paid for higher tiers only to be placed in lower tiers.

Tools Used

Manual review.

Recommendations

To correct this issue, modify the upgradeTier function to mint tokens for the next tier rather than the previous one.

function upgradeTier(address daoMembershipAddress, uint256 fromTierIndex) external {
require(daos[daoMembershipAddress].daoType == DAOType.SPONSORED, "Upgrade not allowed.");
require(daos[daoMembershipAddress].noOfTiers > fromTierIndex + 1, "No higher tier available.");
IMembershipERC1155(daoMembershipAddress).burn(_msgSender(), fromTierIndex, 2);
IMembershipERC1155(daoMembershipAddress).mint(_msgSender(), fromTierIndex + 1, 1); // Corrected logic
emit UserJoinedDAO(_msgSender(), daoMembershipAddress, fromTierIndex + 1);
}
Updates

Lead Judging Commences

0xbrivan2 Lead Judge about 1 year ago
Submission Judgement Published
Invalidated
Reason: Design choice

Support

FAQs

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

Give us feedback!