Project

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

Reentrancy Vulnerability in joinDAO Function

Summary

The smart contract provided contains a critical Reentrancy Vulnerability in the joinDAO function located at line 141. Specifically, the vulnerability arises from the use of multiple calls to external contracts without ensuring proper protection against reentrancy attacks. This is a dangerous vulnerability in the context of decentralized finance (DeFi) protocols, as attackers can exploit it to drain funds, modify contract state, or trigger unintended behavior.

Vulnerability Details

The vulnerability lies in the joinDAO function of the MembershipFactory contract, which allows users to join a DAO by purchasing a membership NFT. The function involves transferring tokens using the ERC-20 transferFrom method and then minting a membership NFT with the IMembershipERC1155(daoMembershipAddress).mint method.

Here's the relevant code of the function:

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);
}

In the above code, there are two external calls made to the IERC20.transferFrom function and one external call to the IMembershipERC1155.mint function. The issue arises because the contract state is modified (specifically, daos[daoMembershipAddress].tiers[tierIndex].minted) before the external calls, and these external calls can re-enter the contract and exploit this modification.

While the state change occurs before the external calls, this function could still be susceptible to reentrancy attacks since an attacker can trick the contract into executing malicious code after sending funds, but before the mint operation is completed. This can potentially lead to double-spending, draining funds, or causing undesired behavior.

Impact

  1. Reentrancy Attacks: The primary risk is that an attacker could deploy a malicious contract that calls back into the MembershipFactory contract after the external calls are made. Specifically, the external transfer calls (to the ERC-20 token contract) are followed by a state change (minted increment), but the external minting function is not protected from reentrancy.

  2. Financial Loss: The attacker could exploit the vulnerability to manipulate the minted value, potentially allowing them to mint more NFTs than allowed, effectively bypassing the membership limits.

  3. Malicious Contract Execution: Since the callExternalContract function is also exposed with little restriction (and also uses low-level calls), an attacker could create a contract that directly exploits this vulnerability to manipulate the DAO state and achieve malicious outcomes, including token theft or arbitrary contract execution.

  4. Denial of Service (DoS): If the attacker exploits this vulnerability in a high-volume scenario (e.g., many users attempting to join at the same time), they could potentially trigger a DoS condition, preventing legitimate users from joining the DAO or interacting with the contract normally.

Tools Used

  • Slither: A static analysis tool for Solidity contracts that identifies common vulnerabilities such as reentrancy, overflow, and uninitialized storage issues.

  • Foundry: A framework for smart contract development and testing that helps run tests and analyze edge cases for vulnerabilities.

  • MythX: A security analysis platform that helps identify vulnerabilities in smart contracts.

  • Remix IDE: Used for manual inspection and interacting with the contract in a safe environment to understand its behavior.

Recommendations

  1. Reentrancy Guard: Implement a reentrancy guard in the joinDAO function to prevent reentrancy attacks. This is commonly done by using OpenZeppelin's ReentrancyGuard contract, which provides a nonReentrant modifier.

    Example:

    import "@openzeppelin/contracts/security/ReentrancyGuard.sol";
    contract MembershipFactory is AccessControl, NativeMetaTransaction, ReentrancyGuard {
    function joinDAO(address daoMembershipAddress, uint256 tierIndex) external nonReentrant {
    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);
    }
    }
  2. Change Order of Operations: Consider modifying the contract's logic so that external calls occur after the state changes (i.e., after the minted counter is updated). However, reentrancy protection is still the preferred solution.

  3. Use Safe Transfer Methods: Avoid using call{value: ...} or low-level calls unless absolutely necessary. Prefer high-level transfer methods like IERC20.transfer or IERC20.transferFrom, which are safer and do not expose the contract to arbitrary code execution risks.

  4. Limit Access: Ensure that only trusted external contracts can call functions like callExternalContract, and restrict the scope of what actions these calls can perform. Use access control and multi-signature mechanisms to enforce these restrictions.

Proof of Concept for Reentrancy Vulnerability

Overview:

The vulnerability allows an attacker to manipulate the contract state due to a lack of reentrancy protection. Specifically, the attacker can exploit the joinDAO function by re-entering the contract during the token transfer process, potentially causing a double-spend or minting more tokens than allowed.

Actors:

  • Attacker: A malicious contract that exploits the vulnerability by calling the joinDAO function and re-entering the contract to manipulate the minting process.

  • Victim: The MembershipFactory contract, which does not have reentrancy protection.

  • Protocol: The DAO membership system, which allows users to join by purchasing tokens and minting NFTs.

Working Test Case (PoC):

// Malicious contract that exploits the reentrancy vulnerability
contract MaliciousContract {
MembershipFactory victim;
// Constructor to set the target victim contract
constructor(address victimAddress) {
victim = MembershipFactory(victimAddress);
}
// Fallback function to call back into the victim contract and exploit reentrancy
fallback() external payable {
// Re-enter the victim contract to manipulate state
victim.joinDAO(address(victim), 0);
}
// Function to initiate the attack
function attack() external {
victim.joinDAO(address(victim), 0);
}
}

Explanation:

  • Line 1: The malicious contract deploys and sets the target MembershipFactory contract.

  • Line 9: The fallback function is triggered when the malicious contract is called, and it re-enters the MembershipFactory contract to exploit the vulnerability.

  • Line 13: The attacker initiates the attack, which exploits the reentrancy issue in the joinDAO function.

Example Code of the Affected Function:

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; // State change before external call
IERC20(daos[daoMembershipAddress].currency).transferFrom(_msgSender(), owpWallet, platformFees);
IERC20(daos[daoMembershipAddress].currency).transferFrom(_msgSender(), daoMembershipAddress, tierPrice - platformFees);
IMembershipERC1155(daoMembershipAddress).mint(_msgSender(), tierIndex, 1); // External call
emit UserJoinedDAO(_msgSender(), daoMembershipAddress, tierIndex);
}

Example of How It Was Infected:

  • The attacker deploy

s a malicious contract that calls joinDAO.

  • The malicious contract re-enters the victim contract during the token transfer process, exploiting the vulnerability and potentially minting more NFTs than allowed or draining funds.

Example of How to Repair:

  • Reentrancy Guard: Implementing a reentrancy guard to prevent the contract from being re-entered during the execution of the joinDAO function.

  • State Change Before External Calls: Moving the state change (like incrementing minted) after the external calls to ensure that an attacker cannot manipulate the contract's state while re-entering.

Updates

Lead Judging Commences

0xbrivan2 Lead Judge 10 months ago
Submission Judgement Published
Invalidated
Reason: Incorrect statement

Support

FAQs

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