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

Incompatibility with Non-Standard ERC20 Tokens in joinDAO Function Causes Transaction Failures

cheatc0d33

Summary

The current implementation's direct use of ERC20 transferFrom function calls presents a critical incompatibility issue with non-standard ERC20 tokens, particularly those like USDT that don't return boolean values. This vulnerability can cause all membership purchases to fail when using such tokens, effectively breaking core protocol functionality and potentially leading to significant user friction and lost revenue opportunities. The impact is severe as USDT is one of the most widely used stablecoins in the ecosystem, and its incompatibility would severely limit the protocol's usability and adoption potential. Furthermore, this issue extends to other tokens that don't strictly follow the ERC20 standard's return value specifications.

The root cause stems from Solidity's strict type checking and the assumption that all ERC20 tokens follow the standard implementation of returning a boolean value for transfers. When interacting with tokens like USDT that don't return any value for transfers, the EVM attempts to decode a non-existent return value as a boolean, causing the transaction to revert. This occurs because the contract directly calls transferFrom without any safe handling of return values, making it incompatible with tokens that deviate from the standard ERC20 implementation.

https://github.com/Cyfrin/2024-11-one-world/blob/main/contracts/dao/MembershipFactory.sol#L140

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;
// These calls will revert with non-compliant tokens like USDT
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);
}

Recommended Fix

Implement OpenZeppelin's SafeERC20 library to safely handle transfers for all ERC20 tokens, including non-standard implementations:

import "@openzeppelin/contracts/token/ERC20/utils/SafeERC20.sol";
​
contract MembershipFactory {
using SafeERC20 for IERC20;
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;
require(tierPrice > 0, "Invalid tier price");
uint256 platformFees = (20 * tierPrice) / 100;
uint256 daoAmount = tierPrice - platformFees;
IERC20 token = IERC20(daos[daoMembershipAddress].currency);
// SafeERC20 handles non-standard token implementations safely
token.safeTransferFrom(_msgSender(), owpWallet, platformFees);
token.safeTransferFrom(_msgSender(), daoMembershipAddress, daoAmount);
daos[daoMembershipAddress].tiers[tierIndex].minted += 1;
IMembershipERC1155(daoMembershipAddress).mint(_msgSender(), tierIndex, 1);
emit UserJoinedDAO(_msgSender(), daoMembershipAddress, tierIndex);
}
}
Updates

Lead Judging Commences

0xbrivan2 Lead Judge 8 months ago
Submission Judgement Published
Invalidated
Reason: Known issue
0xbrivan2 Lead Judge 8 months ago
Submission Judgement Published
Invalidated
Reason: Known issue

Support

FAQs

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