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

Inefficient and Costly Tier Upgrade Mechanism

nourelden

Summary

The tier upgrade mechanism in the MembershipFactory contract is poorly designed, causing users to pay significantly more than necessary when upgrading tiers and potentially losing funds in the process.

Vulnerability Details

The current upgrade mechanism has two major flaws:

  • Users must burn their existing tier token and pay the full price of a new tier instead of just paying the difference

    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);
    emit UserJoinedDAO(_msgSender(), daoMembershipAddress, fromTierIndex - 1);
    }

If the DAO Tiers price equal:

Tier 0 = 7$

Tier 1 = 6$

Tier 2 = 5$

Tier 3 = 4$

Tier 4 = 3$

Tier 5 = 2$

Tier 6 = 1$

function-upgradeTier()

Impact

Financial Loss For The Users/Members Scenarios:

Scenario 1: Overpaying for Upgrades

  • User buys Tier 6 for $6

  • To upgrade to Tier 7 ($7), user must:

  • Burn current Tier 6 token (losing $6)

  • Pay full $7 for Tier 7

  • Total cost: $13 (instead of just paying $1 difference)

  • Loss: $6 in unnecessary costs

Cause a loss of Funds for the user.

Proof Of Concept

Run the Following Foundry Test test_SponsoredDAOMaxMemberLimit() :

// SPDX-License-Identifier: MIT
pragma solidity 0.8.22;
​
import {Test, console} from "forge-std/Test.sol";
import {MembershipERC1155} from "../contracts/dao/tokens/MembershipERC1155.sol";
import {MembershipFactory} from "../contracts/dao/MembershipFactory.sol";
import {IMembershipERC1155} from "../contracts/dao/interfaces/IERC1155Mintable.sol";
import {
DAOConfig,
DAOInputConfig,
TierConfig,
DAOType,
TIER_MAX
} from "../contracts/dao/libraries/MembershipDAOStructs.sol";
import "@openzeppelin/contracts/token/ERC20/IERC20.sol";
import "@openzeppelin/contracts/proxy/transparent/TransparentUpgradeableProxy.sol";
import "@openzeppelin/contracts/proxy/transparent/ProxyAdmin.sol";
​
contract MockERC20 {
mapping(address => uint256) public balanceOf;
mapping(address => mapping(address => uint256)) public allowance;
​
function mint(address to, uint256 amount) public {
balanceOf[to] += amount;
}
​
function approve(address spender, uint256 amount) public returns (bool) {
allowance[msg.sender][spender] = amount;
return true;
}
​
function transfer(address to, uint256 amount) public returns (bool) {
require(balanceOf[msg.sender] >= amount, "Insufficient balance");
balanceOf[msg.sender] -= amount;
balanceOf[to] += amount;
return true;
}
​
function transferFrom(address from, address to, uint256 amount) public returns (bool) {
require(allowance[from][msg.sender] >= amount, "Insufficient allowance");
require(balanceOf[from] >= amount, "Insufficient balance");
allowance[from][msg.sender] -= amount;
balanceOf[from] -= amount;
balanceOf[to] += amount;
return true;
}
}
​
contract MembershipTest is Test {
// Contract instances
MembershipERC1155 public membershipImpl;
MembershipFactory public factory;
IERC20 public mockToken;
​
// Test addresses
address public admin = makeAddr("admin");
address public DAOCreator = makeAddr("DAOCreator");
address public user1 = makeAddr("user1");
address public user2 = makeAddr("user2");
address public owpWallet = makeAddr("owpWallet");
address public currencyManager = makeAddr("currencyManager");
​
// Constants
string constant BASE_URI = "ipfs://QmTest/";
uint256 constant PLATFORM_FEE = 20; // 20%
​
// Test data
DAOInputConfig daoInputConfig;
TierConfig[] _tierConfigs;
​
// Add at the top with other constants
bytes32 constant BURNER_ROLE = keccak256("BURNER_ROLE");
bytes32 constant ADMIN_ROLE = keccak256("ADMIN_ROLE");
​
// Helper function to grant any role
function grantRole(address daoAddress, address to, bytes32 role) internal {
vm.prank(address(factory));
MembershipERC1155(daoAddress).grantRole(role, to);
}
​
// Modifier version if you prefer
modifier withRole(address daoAddress, address to, bytes32 role) {
vm.prank(address(factory));
MembershipERC1155(daoAddress).grantRole(role, to);
_;
}
​
function setUp() public {
vm.startPrank(admin);
​
// Deploy mock token for testing
mockToken = IERC20(deployMockERC20());
​
// Deploy the membership implementation
membershipImpl = new MembershipERC1155();
​
// Deploy the factory
factory = new MembershipFactory(currencyManager, owpWallet, BASE_URI, address(membershipImpl));
​
// Add this: Mock the currency manager to whitelist the mock token
vm.mockCall(
currencyManager,
abi.encodeWithSelector(bytes4(keccak256("isCurrencyWhitelisted(address)"))),
abi.encode(true)
);
​
vm.stopPrank();
}
​
function deployMockERC20() internal returns (address) {
MockERC20 token = new MockERC20();
// Mint some tokens to all test users
token.mint(user1, 100 ether);
token.mint(user2, 100 ether);
// Mint to all the loop addresses we'll use
for (uint256 i = 0; i < 50; i++) {
address user = address(uint160(1000 + i));
token.mint(user, 100 ether);
}
return address(token);
}
​
function setupTierConfigs(uint256 numberOfTiers) internal pure returns (TierConfig[] memory) {
TierConfig[] memory configs = new TierConfig[]();
​
if (numberOfTiers >= 1) {
configs[6] = TierConfig({amount: 100, price: 1 ether, power: 1, minted: 0});
}
​
if (numberOfTiers >= 2) {
configs[5] = TierConfig({amount: 50, price: 2 ether, power: 2, minted: 0});
}
​
if (numberOfTiers >= 3) {
configs[4] = TierConfig({amount: 25, price: 3 ether, power: 5, minted: 0});
}
​
if (numberOfTiers >= 4) {
configs[3] = TierConfig({amount: 10, price: 4 ether, power: 10, minted: 0});
}
​
if (numberOfTiers >= 5) {
configs[2] = TierConfig({amount: 5, price: 5 ether, power: 20, minted: 0});
}
​
if (numberOfTiers >= 6) {
configs[1] = TierConfig({amount: 3, price: 6 ether, power: 50, minted: 0});
}
​
if (numberOfTiers >= 7) {
configs[0] = TierConfig({amount: 1, price: 7 ether, power: 100, minted: 0});
}
​
return configs;
}
function test_UpgradeTier_Inefficiencies_SCENARIO1() public {
// Setup a sponsored DAO with multiple tiers
DAOInputConfig memory daoConfig = DAOInputConfig({
ensname: "sponsoredDAO",
daoType: DAOType.SPONSORED,
currency: address(mockToken),
maxMembers: 194, // sum of all amounts
noOfTiers: TIER_MAX
});
​
// Setup tiers with increasing prices
TierConfig[] memory tierConfigs = setupTierConfigs(TIER_MAX);
​
vm.startPrank(DAOCreator);
address daoAddress = factory.createNewDAOMembership(daoConfig, tierConfigs);
vm.stopPrank();
​
// SCENARIO 1: Overpaying for Upgrades
vm.startPrank(user1);
​
// Buy two Tier 1
mockToken.approve(address(factory), 20 ether);
factory.joinDAO(daoAddress, 1); // tier1 price = 6 ether
factory.joinDAO(daoAddress, 1); // tier1 price = 6 ether
​
// Verify user has 2 tokens of Tier 1
assertEq(MembershipERC1155(daoAddress).balanceOf(user1, 1), 2, "Should have 2 tokens of Tier 1");
uint256 tier1Cost = tierConfigs[1].price * 2;
​
console.log("Cost for two Tier 1:", tier1Cost);
​
// upgrade to Tier 0
factory.upgradeTier(daoAddress, 1);
​
getUserTierBalances(daoAddress, user1);
​
console.log("The function burns two tier1 that cost:", tier1Cost);
console.log("To upgrade to tier 0 that cost:", tierConfigs[0].price);
vm.stopPrank();
}
function getUserTierBalances(address daoAddress, address user) public view {
MembershipERC1155 dao = MembershipERC1155(daoAddress);
​
for (uint256 i = 0; i < TIER_MAX; i++) {
uint256 balance = dao.balanceOf(user, i);
if (balance > 0) {
console.log("Tier %s: %s tokens", i, balance);
}
}
}
}

Tools Used

Recommendations

Implement price difference calculation:

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.");
uint256 currentTierPrice = daos[daoMembershipAddress].tiers[fromTierIndex].price;
uint256 newTierPrice = daos[daoMembershipAddress].tiers[fromTierIndex + 1].price;
uint256 priceDifference = newTierPrice - currentTierPrice;
// Calculate platform fees for the difference
uint256 platformFees = (20 * priceDifference) / 100;
// Transfer only the price difference
IERC20(daos[daoMembershipAddress].currency).transferFrom(_msgSender(), owpWallet, platformFees);
IERC20(daos[daoMembershipAddress].currency).transferFrom(
_msgSender(),
daoMembershipAddress,
priceDifference - platformFees
);
// Burn 1 token instead of 2
IMembershipERC1155(daoMembershipAddress).burn(_msgSender(), fromTierIndex, 1);
IMembershipERC1155(daoMembershipAddress).mint(_msgSender(), fromTierIndex + 1, 1);
emit UserJoinedDAO(_msgSender(), daoMembershipAddress, fromTierIndex + 1);
}
Updates

Lead Judging Commences

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

Appeal created

nourelden Submitter
about 1 year ago
0xbrivan2 Lead Judge
about 1 year ago
nourelden Submitter
about 1 year ago
0xbrivan2 Lead Judge
about 1 year ago
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!