Project

One World

NFTDeFi

15,000 USDC

View results

Previous Next

Submission Details

Severity: medium

Invalid

Upgrading a `tierIndex` does no update mapping's count which will lead to wrong `minted` tracking count

maroutis

Summary

When users upgrade their tier in a DAO, the minted counts for tiers are not properly updated, leading to inaccurate tracking of the number of tokens minted per tier. This can result in tiers appearing full when they are not, or exceeding the maximum allowed tokens for a tier.

Vulnerability Details

In the MembershipFactory contract, the upgradeTier function allows users to upgrade to a higher tier by burning tokens from a lower tier and minting new tokens:

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);

}

However, the minted counts in daos[daoMembershipAddress].tiers are not updated when tokens are burned or minted during an upgrade. This leads to:

The lower tier's minted count remains inflated, not reflecting the burned tokens.
The higher tier's minted count does not increase, potentially allowing more tokens than the amount limit.

POC

Consider the following test :

function testUpdateDaoDoesNotUpdateIndexMintedMapping() public {

TierConfig[] memory tierConfig = new TierConfig[]();

tierConfig[0] = TierConfig({price: 6400, amount: 7, minted: 0, power: 64});

tierConfig[1] = TierConfig({price: 3200, amount: 7, minted: 0, power: 32});

tierConfig[2] = TierConfig({price: 1600, amount: 7, minted: 0, power: 16});

tierConfig[3] = TierConfig({price: 800, amount: 7, minted: 0, power: 8});

tierConfig[4] = TierConfig({price: 400, amount: 7, minted: 0, power: 4});

tierConfig[5] = TierConfig({price: 200, amount: 7, minted: 0, power: 2});

tierConfig[6] = TierConfig({price: 100, amount: 7, minted: 0, power: 1});

MockUsdc mockUsdc = new MockUsdc();

DAOInputConfig memory daoConfig = DAOInputConfig({

ensname: "testdao.eth",

daoType: DAOType.SPONSORED,

currency: address(mockUsdc),

maxMembers: 1270,

noOfTiers: 7

});

currencyManager.addCurrency(address(mockUsdc));

address dao = membershipFactory.createNewDAOMembership(daoConfig, tierConfig);

// Alice and bob both decide to join the DAO

deal(address(mockUsdc), alice, 100e18);

deal(address(mockUsdc), bob, 100e18);

vm.prank(alice);

mockUsdc.approve(address(membershipFactory), type(uint256).max);

vm.prank(bob);

mockUsdc.approve(address(membershipFactory), type(uint256).max);

vm.startPrank(alice);

membershipFactory.joinDAO(dao, 1);

membershipFactory.joinDAO(dao, 0);

vm.stopPrank();

vm.startPrank(bob);

membershipFactory.joinDAO(dao, 1);

membershipFactory.joinDAO(dao, 0);

vm.expectRevert("Tier full.");

membershipFactory.joinDAO(dao, 0);

vm.stopPrank();

TierConfig memory tier1 = membershipFactory.tiers(dao)[1];

TierConfig memory tier0 = membershipFactory.tiers(dao)[0];

assertEq(tier1.minted, 7);

assertEq(MembershipERC1155(dao).balanceOf(alice, 1) + MembershipERC1155(dao).balanceOf(bob, 1), tier1.minted);

assertEq(tier0.minted, 7);

assertEq(MembershipERC1155(dao).balanceOf(alice, 0) + MembershipERC1155(dao).balanceOf(bob, 0), tier0.minted);

// Alice decides to upgrade her tier 1 tokens

vm.startPrank(alice);

membershipFactory.upgradeTier(dao, 1);

vm.stopPrank();

// For the tiers it still says all tokens have been minted

assertEq(tier1.minted, 7);

vm.prank(alice);

vm.expectRevert("Tier full.");

membershipFactory.joinDAO(dao, 1);

vm.stopPrank();

// However only 1 token exist for tier 1

assertEq(MembershipERC1155(dao).balanceOf(alice, 1) + MembershipERC1155(dao).balanceOf(bob, 1), 1);

assertEq(tier0.minted, 7);

// While 10 tokens exist for the tier 0

assertEq(MembershipERC1155(dao).balanceOf(alice, 0) + MembershipERC1155(dao).balanceOf(bob, 0), 10);

// The max amoun to mint can be bypassed this way, especially for higher tiers

}

And the setup part :

// SPDX-License-Identifier: MIT

pragma solidity 0.8.22;

import "../lib/forge-std/src/Test.sol";

import "../contracts/dao/MembershipFactory.sol";

import "../contracts/dao/tokens/MembershipERC1155.sol";

import "../contracts/dao/CurrencyManager.sol";

import "../contracts/OWPIdentity.sol";

import {DAOType, DAOConfig, TierConfig, DAOInputConfig} from "../contracts/dao/libraries/MembershipDAOStructs.sol";

import {MockERC20} from "../lib/forge-std/src/mocks/MockERC20.sol";

contract MembershipDeploymentTest is Test {

MembershipFactory public membershipFactory;

MembershipERC1155 public membershipImplementation;

CurrencyManager public currencyManager;

OWPIdentity public owpIdentity;

address owpWalletAddress;

address owpBackendAdmin;

address rewarder;

string baseURI;

string profileURI;

address alice;

address bob;

address attacker;

function setUp() public {

// Setting up the testing environment with sample addresses and URIs

owpWalletAddress = makeAddr("OWP_PLATFORM_TREASURY_ADDRESS");

owpBackendAdmin = makeAddr("OWP_PROPOSAL_EXECUTOR");

rewarder = makeAddr("Rewarder");

baseURI = "https://example.com/baseURI/";

profileURI = "https://example.com/profileURI/";

alice = makeAddr("Alice");

bob = makeAddr("bob");

attacker = makeAddr("attacker");

// Deploy MembershipERC1155 implementation contract

membershipImplementation = new MembershipERC1155();

// Deploy CurrencyManager contract

currencyManager = new CurrencyManager();

// Deploy MembershipFactory contract

membershipFactory = new MembershipFactory(

address(currencyManager), owpWalletAddress, baseURI, address(membershipImplementation)

);

// Deploy OWPIdentity contract

owpIdentity = new OWPIdentity(owpWalletAddress, owpBackendAdmin, profileURI);

// keccak256("EXTERNAL_CALLER")

membershipFactory.grantRole(0x3124d52df17d64a7d650cea4d44356c1b56f1552895b7db0931fbeaabb38822a, owpBackendAdmin);

}

contract MockUsdc is MockERC20 {

function mint(address user, uint256 amount) external {

_mint(user, amount);

}

Impact

This will result in tiers being incorrectly marked as full, preventing users from joining tiers that actually have available slots. Conversely, it can allow over-minting in higher tiers, exceeding the intended supply and disrupting the DAO's tokenomics.

Tools Used

Foundry and manual review

Recommendations

Consider updating minted counts on tier upgrades while also enforcing tier limits.

Updates

Lead Judging Commences

0xbrivan2 Lead Judge 10 months ago

Submission Judgement Published

Invalidated

Reason: Known issue

Prize pool breakdown

Total prize

15,000 USDC

nSLOC

541

28 USDC / LOC

High Medium

14,000 USDC

Low

1,000 USDC

Live

The contest is live. Earn rewards by submitting a finding.

Judging

Submissions are being carefully reviewed by our judges.

View all submissions

Appeals

This is your time to appeal against judgements on your submissions.

Appeals Review

Appeals are being carefully reviewed by our judges.

Rewards Distribution

The contest is complete and the rewards are being distributed.

View results

Support

FAQs

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