Christmas Dinner

Summary

The deposit function in the ChristmasDinner contract is vulnerable to reentrancy attacks. A malicious ERC20 token could exploit the external call to safeTransferFrom to re-enter the contract, causing unexpected behavior, such as balance inflation, denial of service, or fund draining.

Vulnerability Details

The deposit function processes token transfers using the safeTransferFrom method. If a malicious token is added to the whitelist, its transferFrom implementation could include external calls to re-enter the deposit function.

This re-entrance allows the malicious token to exploit the function before its previous state update is completed. The lack of a nonReentrant modifier and the placement of the external call before the state update creates a gap that can be exploited.

Relevant Code

Impact

1. Balance Inflation: The attacker can inflate their recorded balance by repeatedly calling deposit during the execution of the safeTransferFrom function.

2. Denial of Service: The recursive calls can consume all available gas, blocking legitimate users from depositing tokens.

3. Fund Draining: In scenarios where the contract allows withdrawals or rewards, an attacker could exploit inflated balances to drain funds.

Proof of Concept (PoC)

The following demonstrates how a malicious ERC20 token can exploit the vulnerability:

Malicious ERC20 Contract

Attack Execution

1. Deploy the MaliciousToken contract with the address of the ChristmasDinner contract.

2. Mint tokens to the attacker's address.

3. Whitelist the MaliciousToken in the ChristmasDinner contract.

4. Call deposit with MaliciousToken. The malicious token’s transferFrom implementation will recursively call deposit, creating a reentrancy loop.

Recommendations

1. Use the nonReentrant Modifier
   Apply the nonReentrant modifier to the deposit function to block reentrant calls:

   function deposit(address _token, uint256 _amount) external beforeDeadline nonReentrant {

   ...

   }

2. Adopt Checks-Effects-Interactions Pattern
   Reorder the function logic to update state variables before making external calls:

   function deposit(address _token, uint256 _amount) external beforeDeadline nonReentrant {

   if (!whitelisted[_token]) {

   revert NotSupportedToken();

   }

   balances[msg.sender][_token] += _amount;

   participant[msg.sender] = true;

   emit NewSignup(msg.sender, _amount, true);

   IERC20(_token).safeTransferFrom(msg.sender, address(this), _amount);

   }

3. Strict Whitelisting of Tokens
   Vet all tokens thoroughly before adding them to the whitelist to ensure they do not contain malicious code in their transferFrom implementation.