Project

Summary

Inside MembershipFactory::joinDAO there is the use of transferFrom to handle the costs of joining a DAO tier. transferFrom will work fine for the current stated currencies which are USDC, WETH and WBTC, however in CurrencyManager::addCurrency there is functionality to add more currencies to the protocol. The current checks in that function include a zero address check, and an "already implemented" check. This means that there is potential for other popular but non-standard ERC20 implementations such as USDT.

Vulnerability Details

The safeTransferFrom implementation prevents silent failures from happening on the transfer call. USDT a popular stable coin, and likely a currency that would be in high demand to add to the protocol can silently fail when, either the sender or receiver is blacklisted or when it returns false instead of reverting. The SafeERC20 safeTransferFrom implementation solves this issue and future proofs the protocol correctly in alignment with the CurrencyManager::addCurrency feature.

Impact

Not using safeTransferFrom could lead to issues with any new tokens such as, membership NFT minting without actual payment or the user thinking they have paid for the membership, but the transfer is actually failing. This will lead to the protocol and DAO losing funds.

Tools Used

Manual Review

Recommendations

Make use of the SafeERC20 safeTranferFrom function rather than the current transferFrom implementation.