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

Unchecked transfer in `MembershipFactory::joinDAO`: Ignoring Return Value of `transferFrom` Allows Exploitation

gosbu

Summary

The joinDAO function in the MembershipFactory contract fails to handle the return value of the transferFrom function calls, which are used to transfer tokens for platform fees and DAO membership payments. This oversight can allow an attacker to bypass token transfers and still execute the logic of joining a DAO, minting an NFT without actually paying the required fees.

Vulnerability Details

The joinDAO function includes two calls to transferFrom for transferring tokens:

IERC20(daos[daoMembershipAddress].currency).transferFrom(_msgSender(), owpWallet, platformFees);
IERC20(daos[daoMembershipAddress].currency).transferFrom(_msgSender(), daoMembershipAddress, tierPrice - platformFees);

However, the return value of these transferFrom calls is ignored. According to the ERC20 standard, transferFrom returns a boolean value indicating success or failure. Ignoring this return value can result in unintended behavior, such as not transferring the necessary tokens, but still proceeding with the minting of the membership NFT and emitting the UserJoinedDAO event.

An attacker can exploit this issue by approving tokens without transferring them. This allows them to bypass the payment logic and still call joinDAO, effectively receiving a DAO membership without paying for it.

Impact

  • Financial Loss: Attackers can mint DAO membership NFTs without paying the required tier price, resulting in a loss of platform fees and DAO revenue.

  • Unfair Access: Attackers can gain access to the DAO without contributing the required funds, undermining the integrity of the membership process.

  • Security Breach: The exploit allows users to join DAOs without any actual payment, potentially leading to further attacks on the system.

Tools Used

Manual Analysis

Proof of code

describe("--- ATACK joinDAO ---", function () {
beforeEach(async function () {
await currencyManager.addCurrency(testERC20.address);
await membershipFactory.createNewDAOMembership(DAOConfig, TierConfig);
const ensAddress = await membershipFactory.getENSAddress("testdao.eth");
membershipERC1155 = await MembershipERC1155.attach(ensAddress);
});
it("Should exploit unchecked token transfer in joinDAO", async function () {
// Setup
await testERC20.mint(addr1.address, ethers.utils.parseEther("200"));
await testERC20.connect(addr1).approve(membershipFactory.address, ethers.utils.parseEther("200"));
const tierIndex = 0;
await expect(membershipFactory.connect(addr1).joinDAO(membershipERC1155.address, tierIndex))
.to.emit(membershipFactory, "UserJoinedDAO")
.withArgs(addr1.address, membershipERC1155.address, tierIndex);
const daoData = await membershipFactory.daos(membershipERC1155.address);
const attackerBalance = await testERC20.balanceOf(addr1.address);
const expectedMinimumBalance = ethers.utils.parseEther("198"); // Assuming some gas fee has been deducted
expect(attackerBalance).to.be.gt(expectedMinimumBalance);
});
});

Recommendations

Check Return Value of transferFrom: Modify the joinDAO function to properly handle the return values of the transferFrom calls:

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);
+ require(IERC20(daos[daoMembershipAddress].currency).transferFrom(_msgSender(), owpWallet, platformFees), "Platform fee transfer failed.");
+ require(IERC20(daos[daoMembershipAddress].currency).transferFrom(_msgSender(), daoMembershipAddress, tierPrice - platformFees), "Tier price transfer failed.");
IMembershipERC1155(daoMembershipAddress).mint(_msgSender(), tierIndex, 1);
emit UserJoinedDAO(_msgSender(), daoMembershipAddress, tierIndex);
}
Updates

Lead Judging Commences

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

Support

FAQs

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