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

There can be incorrect accounting of DAO tier minted amount

h2134

Summary

There can be incorrect accounting of DAO tier minted amount after updating DAO tier configuration.

Vulnerability Details

When a DAO tier configuration is updated, if it is to remove some tiers, it only preserves the minted amount of the kept tiers, the other tiers are simply deleted without checking for if there is minted tokens of those tiers.

// Preserve minted values and adjust the length of dao.tiers
for (uint256 i = 0; i < tierConfigs.length; i++) {
if (i < dao.tiers.length) {
tierConfigs[i].minted = dao.tiers[i].minted;
}
}
​
// Reset and update the tiers array
delete dao.tiers;

Admin may check the minted amount of the tiers and ensure there is no minted amount before submitting the updating transaction, however, it's possible that users mint tokens of the to be deleted tiers before the transaction is actually executed.

In case the deleted tiers are added back in the future, the minted amount won't be accounted correctly as the origin tier info has been deleted for good.

Please follow the steps to run PoC to verify:

  1. Follow the instructions to add foundry to the project (You may need to change hardhat version to 2.17.2 in package.json);

  2. Create a test file (.t.sol) containing below in /test directory;

  3. Run forge test --mt testAudit_tierAccounting.

// SPDX-License-Identifier: UNLICENSED
pragma solidity ^0.8.22;
​
import {Test} from "forge-std/Test.sol";
import "@openzeppelin/contracts/token/ERC20/ERC20.sol";
import "../contracts/dao/MembershipFactory.sol";
import "../contracts/dao/CurrencyManager.sol";
import "../contracts/dao/tokens/MembershipERC1155.sol";
import {DAOType, DAOConfig, DAOInputConfig, TierConfig} from "../contracts/dao/libraries/MembershipDAOStructs.sol";
​
contract Audit is Test {
address admin = makeAddr("Admin");
address owpWallet = makeAddr("owpWallet");
​
ERC20 WETH = new MockERC20("Wrapped ETH", "WETH", 18);
ERC20 WBTC = new MockERC20("Wrapped BTC", "WBTC", 8);
ERC20 USDC = new MockERC20("USDC", "USDC", 6);
​
MembershipFactory membershipFactory;
CurrencyManager currencyManager;
function setUp() public {
vm.startPrank(admin);
​
// Deploy CurrencyManager
currencyManager = new CurrencyManager();
currencyManager.addCurrency(address(WETH));
currencyManager.addCurrency(address(WBTC));
currencyManager.addCurrency(address(USDC));
​
// Deploy MembershipERC1155
MembershipERC1155 membershipERC1155Implementation = new MembershipERC1155();
​
// Deploy MembershipFactory
membershipFactory = new MembershipFactory(
address(currencyManager),
owpWallet,
"https://baseuri.com/",
address(membershipERC1155Implementation)
);
​
vm.stopPrank();
}
​
function testAudit_tierAccounting() public {
// Create DAO
address creator = makeAddr("Creator");
DAOInputConfig memory daoInputConfig = DAOInputConfig({
ensname: "PUBLIC DAO",
daoType: DAOType.PUBLIC,
currency: address(USDC),
maxMembers: 127,
noOfTiers: 7
});
​
vm.startPrank(creator);
address daoMemebershipAddr = membershipFactory.createNewDAOMembership(
daoInputConfig,
createTierConfigs(
daoInputConfig.noOfTiers,
ERC20(daoInputConfig.currency).decimals()
)
);
vm.stopPrank();
​
// Alice joins DAO and mints token of the 7th tier before the tier is deleted
address alice = makeAddr("Alice");
deal(address(USDC), alice, 1e6);
vm.startPrank(alice);
USDC.approve(address(membershipFactory), 16e6);
membershipFactory.joinDAO(daoMemebershipAddr, 6);
vm.stopPrank();
​
// The 7th tier
vm.startPrank(admin);
membershipFactory.updateDAOMembership(
"PUBLIC DAO",
createTierConfigs(
daoInputConfig.noOfTiers - 1,
ERC20(daoInputConfig.currency).decimals()
)
);
vm.stopPrank();
​
// The 7th tier is added back
vm.startPrank(admin);
TierConfig[] memory tiers = createTierConfigs(
daoInputConfig.noOfTiers,
ERC20(daoInputConfig.currency).decimals()
);
// The 7th tier minted amount is set to 1 (this just won't work)
tiers[6].minted = 1;
membershipFactory.updateDAOMembership(
"PUBLIC DAO",
createTierConfigs(
daoInputConfig.noOfTiers,
ERC20(daoInputConfig.currency).decimals()
)
);
vm.stopPrank();
// The 7th tier minted amount is still 0
assertEq(membershipFactory.tiers(daoMemebershipAddr)[6].minted, 0);
// The actual minted amount is 1
assertEq(MembershipERC1155(daoMemebershipAddr).balanceOf(alice, 6), 1);
}
​
function createTierConfigs(uint noOfTiers, uint8 decimals) private returns (TierConfig[] memory tiers) {
tiers = new TierConfig[](noOfTiers);
​
uint price = 1 * 10 ** decimals;
uint power = 1;
for (int i = int(noOfTiers) - 1; i >= 0; --i) {
uint index = uint(i);
tiers[index] = TierConfig({
amount: 2 ** index,
price: price,
power: power,
minted: 0
});
​
price *= 2;
power *= 2;
}
}
}
​
contract MockERC20 is ERC20 {
uint8 _decimals;
​
constructor(
string memory name_,
string memory symbol_,
uint8 decimals_
) ERC20(name_, symbol_) {
_decimals = decimals_;
}
​
function decimals() public view override returns (uint8) {
return _decimals;
}
}

Impact

Incorrect accounting of DAO tier minted amount, and this can be prevent OWP DAO tokens from being widely adopted by DAOs.

Tools Used

Manual Review

Recommendations

When updating DAO tier configuration, if there are minted tokens in deleted tiers, burn the tokens and refund to the users, or the transaction can simply revert.

Updates

Lead Judging Commences

0xbrivan2 Lead Judge
7 months ago
0xbrivan2 Lead Judge 7 months ago
Submission Judgement Published
Invalidated
Reason: Incorrect statement

Appeal created

h2134 Submitter
7 months ago
0xbrivan2 Lead Judge
7 months ago
0xbrivan2 Lead Judge 7 months ago
Submission Judgement Published
Invalidated
Reason: Incorrect statement

Support

FAQs

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