Mystery Box

Summary

It is possible for reentrancy attack in functions claimAllRewards and claimSingleReward.

Vulnerability Details

The contract uses a low-level .call function to transfer Ether, which forwards all remaining gas to the recipient (msg.sender). After transferring Ether, the state update (delete rewardsOwned[msg.sender];) happens. This leaves the contract vulnerable to a reentrancy attack, where the recipient could call the contract again (reenter) before the state is updated, potentially claiming rewards multiple times.

Impact

If msg.sender is a contract, it could use the Ether received to call claimAllRewards() or claimSingleReward again before the delete rewardsOwned[msg.sender]; the statement is executed, allowing them to claim rewards multiple times.

An attacker could repeatedly call the function before the rewards are deleted, draining the contract's balance.

Tools Used

Manual review.

Recommendations

If msg.sender is a contract, it could use the Ether received to call claimAllRewards() again before the delete rewardsOwned[msg.sender]; a statement is executed, allowing them to claim rewards multiple times. Also, use reentrancy guards like OpenZeppelin's nonReentrant modifier to prevent reentrancy attacks.

For example, that is how you could fix it: