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

Improper Upgrade Logic in upgradeTier Function

bryanconquer

Summary

The upgradeTier function in the MembershipFactory contract contains improper logic that causes users to be downgraded to a lower membership tier instead of being upgraded to a higher tier. This issue is due to incorrect index manipulation, which leads to unintended tier changes for users in sponsored DAOs.

Vulnerability Details

The upgradeTier function is intended to move a user from their current membership tier (fromTierIndex) to the next higher tier. However, the logic mistakenly subtracts 1 from the fromTierIndex when calling the mint() function, which results in a downgrade rather than an upgrade.

The line that mints a new tier subtracts 1 from the current tier index (fromTierIndex - 1), which results in a downgrade.

  • Users attempting to upgrade are mistakenly given a lower-tier membership.

Affected Code:

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.");
// Incorrect logic: downgrades user instead of upgrading
IMembershipERC1155(daoMembershipAddress).burn(_msgSender(), fromTierIndex, 2);
IMembershipERC1155(daoMembershipAddress).mint(_msgSender(), fromTierIndex - 1, 1);
emit UserJoinedDAO(_msgSender(), daoMembershipAddress, fromTierIndex - 1);
}
​

poc

it("Should upgrade to the correct next tier", async function () {
const fromTierIndex = 1;
await testERC20.mint(addr1.address, ethers.utils.parseEther("1000000"));
await testERC20.connect(addr1).approve(membershipFactory.address, ethers.utils.parseEther("1000000"));
await membershipFactory.connect(addr1).joinDAO(membershipERC1155.address, fromTierIndex);
// Store balance before the upgrade
const beforeBalance = await membershipERC1155.balanceOf(addr1.address, fromTierIndex);
// Call upgradeTier function
await membershipFactory.connect(addr1).upgradeTier(membershipERC1155.address, fromTierIndex);
// Check if the user has been upgraded to the next tier (fromTierIndex + 1)
const afterBalance = await membershipERC1155.balanceOf(addr1.address, fromTierIndex + 1);
// Verify that the user has been upgraded to the next tier
expect(afterBalance).to.equal(beforeBalance.add(1)); // The user should have one more token in the next tier
// Ensure no tokens remain in the old tier
const afterOldTierBalance = await membershipERC1155.balanceOf(addr1.address, fromTierIndex);
expect(afterOldTierBalance).to.equal(0); // The user should not have any tokens in the old tier
});
​

Impact

  • Downgraded Membership: Users trying to upgrade their membership are downgraded to a lower tier, losing access to higher-tier benefits.

  • User Frustration: The issue can lead to user dissatisfaction, as users may end up with unintended tier downgrades, especially if they paid for an upgrade.

  • Potential Financial Loss: If there is a fee associated with upgrading membership tiers, users may lose funds without receiving the expected higher-tier benefits.

Tools Used

Manual Code Review

Recommendations

Fix the Tier Upgrade Logic: Update the mint() function call to correctly upgrade users to the next higher tier:

IMembershipERC1155(daoMembershipAddress).mint(_msgSender(), fromTierIndex + 1, 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.