Christmas Dinner

Summary

**The nonReentrant modifier is not implemented properly, the **locked **variable is not updated to **true inside the modifier. Allowing recursive calls to the function.

Vulnerability Details

The nonReentrant modifier isn't implemented properly and doesn't prevent reentrant calls, the function is vulnerable to reentrancy attacks, which allows an attacker to repeatedly call the refund() function before the state updates to prevent further withdrawals.

Attack Scenario

Here’s how an attacker might exploit the reentrancy vulnerability for ETH:

1. First Call to refund:

   • The attacker calls refund and enters _refundETH.

   • ETH is sent to the attacker’s contract via _to.transfer(refundValue).

   • The attacker’s fallback/receive function is triggered.

2. Reentrant Call:

   • In the fallback function, the attacker calls refund again.

   • Since locked hasn't been updated the modifier doesn't prevent reentrancy.

   • Because etherBalance[_to] has not yet been set to 0, the attacker can withdraw ETH again.

3. Repeat:

   • The attacker repeats this process until all ETH in the contract is drained

ERC20 tokens are safe because they are immediately updated to 0 once they are transferred in_refundERC20

Impact

The enitre ether of the contract can be drained by a bad actor, by repeatedly calling the refund() function using a fallback()/receive() function call before the etherBalance mapping is updated.

Tools Used

Manual Review

Recommendations

Update the nonReentrant modifier to ensure it works as intended.