Submission Details
Severity:
Membership token mints are not accounted in upgradeTier(address,uint256)
Summary
The amount of minted membership tokens are accounted in the DAO config struct. We can describe the protocol invariant:
tier[i].minted == sum(balanceOf(address, i))
I.e. the amount of minted tokens accounted in the struct must be equal to the sum of the actual existing supply for a given tier. This invariant can be broken.
Vulnerability Details
The accounting is executed properly in joinDAO(address,uint256):
function joinDAO(address daoMembershipAddress, uint256 tierIndex) external {
...
daos[daoMembershipAddress].tiers[tierIndex].minted += 1;
...
}
However, the same accounting is not done in upgradeTier(address,uint256):
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);
}
The minted amounts need to be adjusted two times, once for the newly minted tokens, once for the burned tokens. Neither is done in this case.
The following PoC demonstrates the issue.
import { expect } from "chai";
import { ethers } from "hardhat";
describe("NativeMetaTransaction", function () {
let owner: any, user: any, attacker: any;
let currencyManager, membershipImplementation, membershipFactory: any, testERC20:any;
let DAOType:any, DAOConfig:any, TierConfig:any;
beforeEach(async function () {
// init signers
[owner, user, attacker] = await ethers.getSigners();
// deploy CurrencyManager
const CurrencyManager = await ethers.getContractFactory("CurrencyManager");
currencyManager = await CurrencyManager.deploy();
await currencyManager.deployed();
console.log("CurrencyManager deployed at:", currencyManager.address);
// deploy MembershipERC1155
const MembershipERC1155 = await ethers.getContractFactory("MembershipERC1155");
membershipImplementation = await MembershipERC1155.deploy();
await membershipImplementation.deployed();
console.log("MembershipERC1155 deployed at:", membershipImplementation.address);
// deploy MembershipFactory
const MembershipFactory = await ethers.getContractFactory("MembershipFactory");
membershipFactory = await MembershipFactory.deploy(currencyManager.address, owner.address, "https://baseuri.com/", membershipImplementation.address);
await membershipFactory.deployed();
console.log("MembershipFactory deployed at:", membershipFactory.address);
// deploy testERC20
const ERC20 = await ethers.getContractFactory("OWPERC20");
testERC20 = await ERC20.deploy('OWP', 'OWP');
await testERC20.deployed();
console.log("TestERC20 deployed at:", testERC20.address);
// add currencies: WETH, WBTC, USDC, USDC.e, OWPERC20
await currencyManager.connect(owner).addCurrency("0x7ceb23fd6bc0add59e62ac25578270cff1b9f619"); // WETH
await currencyManager.connect(owner).addCurrency("0x1BFD67037B42Cf73acF2047067bd4F2C47D9BfD6"); // WBTC
await currencyManager.connect(owner).addCurrency("0x3c499c542cEF5E3811e1192ce70d8cC03d5c3359"); // USDC
await currencyManager.connect(owner).addCurrency("0x2791Bca1f2de4661ED88A30C99A7a9449Aa84174"); // USDC.e
await currencyManager.connect(owner).addCurrency(testERC20.address); // OWPERC20
});
it("Breaks invariant minted == sum of balances", async function () {
DAOType = { PUBLIC: 0, PRIVATE: 1, SPONSORED: 2, };
DAOConfig = { ensname: "test", daoType: DAOType.SPONSORED, currency: testERC20.address, maxMembers: 5000, noOfTiers: 7 };
TierConfig = [
{ price: 0, amount: 2, minted: 0, power: 12 }, // tier 0
{ price: 0, amount: 4, minted: 0, power: 6 }, // tier 1
{ price: 0, amount: 8, minted: 0, power: 3 }, // tier 2
{ price: 0, amount: 16, minted: 0, power: 12 }, // tier 3
{ price: 0, amount: 32, minted: 0, power: 6 }, // tier 4
{ price: 0, amount: 64, minted: 0, power: 3 }, // tier 5
{ price: 0, amount: 128, minted: 0, power: 12 }, // tier 6
];
const tx = await membershipFactory.createNewDAOMembership(DAOConfig, TierConfig);
const receipt = await tx.wait();
const event = receipt.events.find((event:any) => event.event === "MembershipDAONFTCreated");
const ensName = event.args[2][0];
const nftAddress = event.args[1];
const ensToAddress = await membershipFactory.getENSAddress("test")
expect(ensName).to.equal("test");
expect(ensToAddress).to.equal(nftAddress);
console.log("Sponsored DAO created.")
// user buys tier 0 (lowest tier) nft
const tx_join_dao = await membershipFactory.connect(user).joinDAO(nftAddress, 6);
const tx_join_dao_receipt = await tx_join_dao.wait();
// user buys tier 0 (lowest tier) nft again, because 2 same tier tokens are needed for an upgrade
const tx_join_dao_2 = await membershipFactory.connect(user).joinDAO(nftAddress, 6);
const tx_join_dao_2_receipt = await tx_join_dao_2.wait();
// user calls upgrade tier, minting "invalid" token
const tx_upgrade_tier = await membershipFactory.connect(user).upgradeTier(nftAddress, 6);
const receipt_upgrade_tier = await tx_upgrade_tier.wait();
const nftContract = await ethers.getContractAt("MembershipERC1155", nftAddress);
const userBalance = await nftContract.balanceOf(user.address, 5);
expect(userBalance).to.equal(1);
console.log("Call to upgradeTier() successful.");
const tiers = await membershipFactory.tiers(nftContract.address);
console.log(tiers);
});
});
The result is printed to the console:
[
...,
[
BigNumber { value: "64" },
BigNumber { value: "0" },
BigNumber { value: "3" },
BigNumber { value: "0" },
amount: BigNumber { value: "64" },
price: BigNumber { value: "0" },
power: BigNumber { value: "3" },
minted: BigNumber { value: "0" } // tier 5 minted is 0, but our test just proved we minted 1 token!
],
[
BigNumber { value: "128" },
BigNumber { value: "0" },
BigNumber { value: "12" },
BigNumber { value: "2" },
amount: BigNumber { value: "128" },
price: BigNumber { value: "0" },
power: BigNumber { value: "12" },
minted: BigNumber { value: "2" } // tier 6 tokens were burnt but not removed
]
]
Likelihood
High - any user can execute the sequence of action for any DAO where daoType == SPONSORED.
Impact
Medium - protocol invariant is broken, leading to unexpected behaviour.
Tools Used
Manual analysis
Hardhat
Foundry
Recommendations
Perform the accounting in upgradeTier(address,uint256)as well:
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.");
daos[daoMembershipAddress].tiers[tierIndex].minted += 1; // account newly minted token
daos[daoMembershipAddress].tiers[tierIndex-1].minted -= 2; // account burned token
IMembershipERC1155(daoMembershipAddress).burn(_msgSender(), fromTierIndex, 2);
IMembershipERC1155(daoMembershipAddress).mint(_msgSender(), fromTierIndex - 1, 1);
emit UserJoinedDAO(_msgSender(), daoMembershipAddress, fromTierIndex - 1);
}
Prize pool breakdown
Total prize
15,000 USDC
nSLOC
54128
USDC / LOC
14,000 USDC
1,000 USDC
Live
The contest is live. Earn rewards by submitting a finding.
Appeals
This is your time to appeal against judgements on your submissions.
Appeals Review
Appeals are being carefully reviewed by our judges.