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

Minted tier count not updated when users leave the DAO

robertodf99

Summary

According to the sponsor team's response in section 7.3.8 of the Cyfrin audit report, users who intend to leave a DAO will notify the EXTERNAL_CALLER via off-chain methods, after which their tokens will be burned. Currently, the only way to perform this burn action is through an external call initiated by the MembershipFactory contract. While a burn transaction is sent to the MembershipERC1155 contract to reduce the user’s token balance, the DAOConfig struct in MembershipFactory remains unsynchronized, as the minted count for the tier is not decreased.

Vulnerability Details

As a result of this desynchronization, fewer members than anticipated may be eligible to join the DAO, since the minted tier count in the factory contract and the actual membership token supply on-chain are not aligned.

Impact

The following PoC in Foundry simplifies an scenario where Alice mints a tier 6 membership and then the factory relays the call to the membership NFT contract, essentially simulating the external call process.

See PoC
// SPDX-License-Identifier: MIT
pragma solidity ^0.8.20;
​
import {Vm, Test} from "forge-std/Test.sol";
import {console2} from "forge-std/console2.sol";
import {DAOConfig, DAOInputConfig, TierConfig, DAOType, TIER_MAX} from "../src/dao/libraries/MembershipDAOStructs.sol";
import {MembershipFactory} from "../src/dao/MembershipFactory.sol";
import {CurrencyManager} from "../src/dao/CurrencyManager.sol";
import {MembershipERC1155} from "../src/dao/tokens/MembershipERC1155.sol";
import {ERC20} from "@openzeppelin/contracts/token/ERC20/ERC20.sol";
​
contract MembershipFactoryTest is Test {
MembershipFactory membershipFactory;
CurrencyManager currencyManager;
MembershipERC1155 membershipImplementation;
MockToken mockToken;
address alice = makeAddr("alice");
address daoAddress;
​
function setUp() public {
vm.startPrank(alice);
mockToken = new MockToken("TOKEN", "TOKEN");
membershipImplementation = new MembershipERC1155();
currencyManager = new CurrencyManager();
membershipFactory = new MembershipFactory(
address(currencyManager),
address(this),
"baseURI",
address(membershipImplementation)
);
​
DAOInputConfig memory inputConfig = DAOInputConfig({
ensname: "TEST",
daoType: DAOType.SPONSORED,
currency: address(mockToken),
maxMembers: 700,
noOfTiers: 7
});
​
TierConfig[] memory tierConfig = new TierConfig[](); // This should be new TierConfig[](7)
​
for (uint256 i = 0; i < 7; i++) {
tierConfig[i] = TierConfig({
amount: 10,
price: (7 - i) * 10 ** 18,
power: (7 - i) * 10 ** 18,
minted: 0
});
}
currencyManager.addCurrency(address(mockToken));
daoAddress = membershipFactory.createNewDAOMembership(
inputConfig,
tierConfig
);
vm.stopPrank();
}
​
function testLeaveDAO() public {
deal(address(mockToken), alice, 2 ether);
vm.startPrank(alice);
mockToken.approve(address(membershipFactory), type(uint256).max);
membershipFactory.joinDAO(daoAddress, 6);
vm.stopPrank();
// When users want to leave the DAO EXTERNAL_CALLER burns their token
vm.prank(address(membershipFactory));
MembershipERC1155(daoAddress).burn(alice, 6, 1);
TierConfig[] memory tierConfig = membershipFactory.tiers(daoAddress);
// However, the tier minted amount has not been updated in the factory
assertEq(tierConfig[6].minted, 1);
}
}
contract MockToken is ERC20 {
constructor(
string memory name_,
string memory symbol_
) ERC20(name_, symbol_) {
_mint(msg.sender, 100 ether);
}
}
​

Tools Used

Manual review

Recommendations

I propose two options:

  • Implement a dedicated function in the factory contract that performs the external call while also updating the DAOConfig struct.

  • In the MembershipFactory::callExternalContract function, consider adding logic to check the function signature and update the configuration struct when burn functions in the MembershipERC1155 contract are called.

Updates

Lead Judging Commences

0xbrivan2 Lead Judge 10 months ago
Submission Judgement Published
Invalidated
Reason: Out of scope

Support

FAQs

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