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

It is possible to change `tierConfigs[].amount` to be lower than `dao.tiers[].minted` by calling `updateDAOMembership`. This also causes an issue where the new amount of `maxMembers` in a DAO will be less than members already in the DAO

ljj

Summary

When updating a DAO membership with the updateDAOMembership function there is no checks made to make sure that the new tierConfigs[].amount is not less than the already existing dao.tiers[].minted. This will cause discrepencies in the contract while also breaking the invariants that the amount of minted tokens in a tier can not bypass the amount of tokens allowed to be minted and amount of maxMembers allowed in a DAO cannot be less than the amount of members in the DAO.

Vulnerability Details

The EXTERNAL_CALLER role can update a dao with the updateDAOMembership function.

function updateDAOMembership(string calldata ensName, TierConfig[] memory tierConfigs)
external onlyRole(EXTERNAL_CALLER) returns (address) {
address daoAddress = getENSAddress[ensName];
require(tierConfigs.length <= TIER_MAX, "Invalid tier count.");
require(tierConfigs.length > 0, "Invalid tier count.");
require(daoAddress != address(0), "DAO does not exist.");
DAOConfig storage dao = daos[daoAddress];
if(dao.daoType == DAOType.SPONSORED){
require(tierConfigs.length == TIER_MAX, "Invalid tier count.");
}
​
uint256 maxMembers = 0;
​
// 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;
for (uint256 i = 0; i < tierConfigs.length; i++) {
dao.tiers.push(tierConfigs[i]);
maxMembers += tierConfigs[i].amount;
}
​
// updating the ceiling limit acc to new data
if(maxMembers > dao.maxMembers){
dao.maxMembers = maxMembers;
}
​
dao.noOfTiers = tierConfigs.length;
return daoAddress;
}

As observed from this function, there are no checks done to make sure than the new tierConfigs[].amount is greater than or equal to the already existing dao.tiers[].minted amount.

Consider the scenario where in a DAO's configuration, in Tier 3, the maximum allowed amount to be minted is 4 and members have joined this tier 4 times. Now Tier 3 is 4 out of 4 minted. The EXTERNAL_CALLER role now updates this DAO with the updateDAOMembership function and changes the Tier 3's maximum allowed minted amount to 2. Now Tier 3 will be 4 out of 2 minted and breaking the invariant where amount of minted tokens is greater than the maximum allowed amount that is mintable in this tier.

Another invariant that breaks here is the amount of maxMembers that can be in a DAO. This value is calculated by adding up the new tierConfigs[].amount values. Not making sure that the new amounts are greater than or equal to the already existing amount of minted tokens will lead to the new maxMembers value being less than the amount of members that already exist, leading to this invariant also breaking. maxMembers value will store a value that is less than the amount of members in the DAO.

The issues that are caused here are:

  • Before the update, in order for new users to join Tier 3 they would have to wait for any user in Tier 3 to call the upgradeTier function which would empty out 2 slots for new users to join. However, because the allowed amount is not validated to be greater than or equal to already existing amount , in order for new users to join this tier they will have to wait for two different upgradeTier function calls.

  • maxMembers state variable will store a value that is less than already existing number of members.

  • Maximum amount of tokens that can be minted in a tier will be less than the amount of tokens that are already minted.

Proof of Concept

1) Implement a new getter function in MembershipFactory.sol to make tests easier.

function getDAODataFromAddressFromENS(string memory ensName, uint256 tier) public view returns (uint256, uint256) {
address daoAddress = getENSAddress[ensName];
DAOConfig storage dao = daos[daoAddress];
return (dao.tiers[tier].minted, dao.tiers[tier].amount);
}

2) Implement a mock WETH contract for testing inside the test folder.

// SPDX-License-Identifier: MIT
pragma solidity 0.8.22;
​
import "@openzeppelin/contracts/token/ERC20/ERC20.sol";
​
contract MockWETH is ERC20 {
constructor(string memory name, string memory symbol) ERC20(name, symbol) {}
​
function mint(address to, uint256 amount) external {
_mint(to, amount);
}
}

3) Implement and run the following test to observe the vulnerability

// SPDX-License-Identifier: MIT
pragma solidity 0.8.22;
​
import { Test, console } from "forge-std/Test.sol";
import { MembershipFactory } from "../contracts/dao/MembershipFactory.sol";
import { CurrencyManager } from "../contracts/dao/CurrencyManager.sol";
import { MembershipERC1155 } from "../contracts/dao/tokens/MembershipERC1155.sol";
import { OWPIdentity } from "../contracts/OWPIdentity.sol";
import "../contracts/dao/libraries/MembershipDAOStructs.sol";
import { MockWETH } from "./MockWETH.sol";
import { IERC20 } from "@openzeppelin/contracts/token/ERC20/IERC20.sol";
​
contract TestOneWorld is Test {
// actors
address OWPTeam = makeAddr("1WPTeam");
address OWPWallet = makeAddr("OWPWallet");
address DAOCreator = makeAddr("DAOCreator");
address DAOMember = makeAddr("DAOMember");
address DAOMember2 = makeAddr("DAOMember2");
address DAOMember3 = makeAddr("DAOMember3");
address WETHDeployer = makeAddr("WETHDeployer");
​
//contracts
CurrencyManager currencyManager;
MembershipERC1155 membershipERC1155;
MembershipFactory membershipFactory;
MockWETH weth;
// addresses
address daoSponsoredProxy;
address daoPublicProxy;
​
function setUp() public {
// Deploy necessary contract
vm.prank(WETHDeployer);
weth = new MockWETH("Wrapped ETHER", "WETH");
​
vm.startPrank(OWPTeam);
currencyManager = new CurrencyManager();
membershipERC1155 = new MembershipERC1155();
membershipFactory = new MembershipFactory(
address(currencyManager),
OWPWallet,
"https://images.com/{id}",
address(membershipERC1155)
);
​
//Whitelist weth
currencyManager.addCurrency(address(weth));
​
vm.stopPrank();
​
weth.mint(DAOMember, 1000e18);
weth.mint(DAOMember2, 1000e18);
weth.mint(DAOMember3, 1000e18);
}
​
function testUpdateDAOAmountLessThanMinted() public {
//DAO config with 7 tiers
TierConfig[] memory tiersInput = new TierConfig[]();
tiersInput[0] = TierConfig({ amount: 1, price: 64, power: 64, minted: 0 });
tiersInput[1] = TierConfig({ amount: 2, price: 32, power: 32, minted: 0 });
tiersInput[2] = TierConfig({ amount: 4, price: 16, power: 16, minted: 0 });
tiersInput[3] = TierConfig({ amount: 8, price: 8, power: 8, minted: 0 });
tiersInput[4] = TierConfig({ amount: 16, price: 4, power: 4, minted: 0 });
tiersInput[5] = TierConfig({ amount: 32, price: 2, power: 2, minted: 0 });
tiersInput[6] = TierConfig({ amount: 64, price: 100, power: 1, minted: 0 });
​
// DAO config to be updated to
TierConfig[] memory tiersInput2 = new TierConfig[]();
tiersInput2[0] = TierConfig({ amount: 1, price: 64, power: 64, minted: 0 });
tiersInput2[1] = TierConfig({ amount: 2, price: 32, power: 32, minted: 0 });
tiersInput2[2] = TierConfig({ amount: 4, price: 16, power: 16, minted: 0 });
tiersInput2[3] = TierConfig({ amount: 8, price: 8, power: 8, minted: 0 });
tiersInput2[4] = TierConfig({ amount: 16, price: 4, power: 4, minted: 0 });
tiersInput2[5] = TierConfig({ amount: 32, price: 2, power: 2, minted: 0 });
tiersInput2[6] = TierConfig({ amount: 1, price: 1, power: 1, minted: 0 });
​
DAOInputConfig memory daoPublicInputConfig = DAOInputConfig({
ensname: "daoSponsored",
daoType: DAOType.SPONSORED,
currency: address(weth),
maxMembers: 300,
noOfTiers: 7
});
​
// create DAO
vm.prank(DAOCreator);
daoPublicProxy = membershipFactory.createNewDAOMembership(
daoPublicInputConfig,
tiersInput
);
// join DAO at tier 7 twice
vm.startPrank(DAOMember);
IERC20(weth).approve(address(membershipFactory), 1000e18);
membershipFactory.joinDAO(daoPublicProxy, 6);
membershipFactory.joinDAO(daoPublicProxy, 6);
vm.stopPrank();
​
// update DAO
vm.prank(OWPTeam);
membershipFactory.updateDAOMembership("daoSponsored", tiersInput2);
​
// cache values
(uint256 mintedAmount, uint256 allowedAmount) = membershipFactory.getDAODataFromAddressFromENS("daoSponsored", 6);
// assert to prove mintedAmount is greater than allowedAmount
assertGt(mintedAmount, allowedAmount);
}
}

Impact

Impact: Medium
Likelihood: Low, requires the EXTERNAL_CALLER role to update the DAO with a new amount that is less than the existing minted amount.

Tools Used

Manual review, foundry

Recommendations

Implement a new check in the for loop in the updateDAOMembership function to make sure that the new tierConfigs[].amount is greater than or equal to the already existing dao.tiers[].minted.

function updateDAOMembership(string calldata ensName, TierConfig[] memory tierConfigs)
//rest of the function
​
// Preserve minted values
for (uint256 i = 0; i < tierConfigs.length; i++) {
if (i < dao.tiers.length) {
uint256 mintedAmount = dao.tiers[i].minted;
++ require(tierConfigs[i].amount >= mintedAmount, "New tier amount is less than minted amount.");
tierConfigs[i].minted = mintedAmount;
}
}
//rest of the function
Updates

Lead Judging Commences

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.