Project

Summary

The MembershipERC1155 contract contains a critical reentrancy vulnerability that exists in the sendProfit and claimProfit functions. This vulnerability arises because both functions involve transferring ERC-20 tokens (currency), and the contract does not implement any form of protection against reentrancy attacks. Specifically:

• The sendProfit function updates the totalProfit and totalSupply state variables, followed by an external call to transfer the ERC-20 tokens.

• Similarly, the claimProfit function updates the savedProfit and lastProfit mappings and calls IERC20(currency).safeTransfer(msg.sender, profit), which can trigger a reentrancy attack.

These functions are vulnerable to being called recursively, allowing an attacker to exploit the external call (ERC-20 transfer) to re-enter the contract before the state variables are properly updated, leading to incorrect balances or multiple profit claims.

Vulnerable lines:

• sendProfit function (Line 141–143): Transfers funds before finalizing all state updates.

• claimProfit function (Line 106–110): Claims profit and then transfers ERC-20 tokens, enabling reentrancy.

Vulnerability Details

Reentrancy Attack Scenario

A reentrancy vulnerability occurs when external calls (like token transfers) allow the contract to be re-entered before it finishes executing. In this contract:

1. claimProfit function:

   • This function first calls saveProfit(msg.sender) to calculate and update the profit for the sender.

   • The state is updated (savedProfit[msg.sender] = 0;), then a transfer to the msg.sender is initiated.

   • An attacker can exploit the transfer by re-entering the contract before the state updates are complete, allowing the attacker to claim more profit than they are entitled to.

2. sendProfit function:

   • This function calculates the share of profits for all token holders and then makes a transfer from the sender to the contract.

   • An attacker who has control over the currency token can use a malicious contract to call sendProfit and re-enter the contract via the token transfer, allowing for an unbounded amount of profit manipulation.

The key issue here is the lack of reentrancy protection. Both claimProfit and sendProfit functions make external calls (safeTransfer), which can trigger malicious behavior, such as re-entering the contract before the state changes are fully committed.

Reentrancy Exploit

The attacker can create a malicious contract that triggers the claimProfit or sendProfit function and uses a fallback or receive function to re-enter the contract, causing the state variables to become inconsistent and allowing the attacker to:

• Claim more profits than allowed.

• Potentially steal funds or manipulate the profit distribution logic.

For example, the attacker could re-enter claimProfit multiple times in one transaction, draining the available profit pool.

Impact

• Financial Exploitation: Attackers can exploit this vulnerability to claim more than their fair share of profits, causing the contract to lose funds and potentially draining the profit pool.

• Protocol Disruption: If the profit distribution is an essential feature of the protocol, this could lead to unfair distribution of profits and the disruption of the membership system.

• State Manipulation: Reentrancy could result in the manipulation of the savedProfit and lastProfit mappings, allowing for arbitrary profit claims.

Tools Used

• Foundry: A framework for smart contract testing used to simulate reentrancy attacks.

• Slither: A static analysis tool used to detect reentrancy and other vulnerabilities.

• MythX: A security platform used to scan for common vulnerabilities such as reentrancy, integer overflow, and other security issues.

Recommendations

1. Implement Reentrancy Guard:

   • Use the ReentrancyGuard modifier from OpenZeppelin or implement a custom reentrancy guard to prevent recursive calls to the sendProfit and claimProfit functions.

   Example of a ReentrancyGuard implementation:

   bool private _locked = false;

   ​

   modifier nonReentrant() {

   require(!_locked, "ReentrancyGuard: reentrant call");

   _locked = true;

   _;

   _locked = false;

   }

   ​

   // Apply the modifier to vulnerable functions

   function claimProfit() external nonReentrant returns (uint256 profit) {

   profit = saveProfit(msg.sender);

   require(profit > 0, "No profit available");

   savedProfit[msg.sender] = 0;

   IERC20(currency).safeTransfer(msg.sender, profit);

   emit Claim(msg.sender, profit);

   }

   ​

   function sendProfit(uint256 amount) external nonReentrant {

   uint256 _totalSupply = totalSupply;

   if (_totalSupply > 0) {

   totalProfit += (amount * ACCURACY) / _totalSupply;

   IERC20(currency).safeTransferFrom(msg.sender, address(this), amount);

   emit Profit(amount);

   } else {

   IERC20(currency).safeTransferFrom(msg.sender, creator, amount); // Redirect profit to creator if no supply

   }

   }

2. Reorder State Changes and External Calls:

   • Always modify the state first, and then perform external calls (token transfers). This ensures that the state is finalized before any external contract can interact with it.

   Example of corrected state and transfer ordering:

   function claimProfit() external returns (uint256 profit) {

   profit = saveProfit(msg.sender);

   require(profit > 0, "No profit available");

   savedProfit[msg.sender] = 0; // Update state before transfer

   IERC20(currency).safeTransfer(msg.sender, profit); // External call after state change

   emit Claim(msg.sender, profit);

   }

3. Audit External Token Contracts:

   • Ensure that any external contracts interacting with this contract (e.g., the ERC-20 currency contract) are also secure and cannot be used to exploit reentrancy.

4. Test for Reentrancy:

   • Use testing frameworks like Foundry to simulate reentrancy attacks and ensure that the contract functions as expected under different scenarios.

Proof of Concept for Reentrancy Attack

Overview:

This vulnerability occurs when an attacker creates a malicious contract that exploits the external token transfer in the claimProfit and sendProfit functions, allowing them to recursively call these functions and claim more profits than they are entitled to.

Actors:

• Attacker: A malicious contract that exploits reentrancy to claim more profits.

• Victim: The contract itself, which suffers from reentrancy.

• Protocol: The DAO or contract using the profit-sharing mechanism.

Working Test Case (PoC):

Explanation:

• Line 7-9: The malicious contract stores the address of the vulnerable MembershipERC1155 contract.

• Line 12-14: The attacker triggers claimProfit() from their own contract, which calls the claimProfit function.

• Line 17-19: The fallback function allows the attacker to re-enter the claimProfit function recursively before the state changes are finalized.

Expected Outcome:

• The attacker is able to call claimProfit multiple times in one transaction, draining the profit pool by exploiting the reentrancy vulnerability.

Example of Affected Code:

Example of How It Was Infected:

• The vulnerability occurs because the state (savedProfit[msg.sender] = 0;) is modified after the external call (safeTransfer), which allows a malicious contract to re-enter the function before the state is finalized.