Mystery Box

Summary

The claimAllRewards function is intended to allow users to claim all their rewards by transferring the total value of rewards owed to them. However, the current implementation of the function is vulnerable to a reentrancy attack, which can allow a malicious user to exploit the contract and withdraw more funds than they are entitled to.

Vulnerability Details

The vulnerability arises from the order of operations within the function. Specifically, the contract sends funds to the caller before updating the state variable rewardsOwned[msg.sender]. This opens the possibility for an attacker to re-enter the claimAllRewards function and repeatedly drain funds before the state is updated.

Detailed Breakdown:

1. Loop Calculation of Total Rewards: The function loops through the rewardsOwned[msg.sender] array to calculate the total reward value. This is safe in itself, but the vulnerability occurs later in the code execution.

2. Funds Transfer: After checking if the totalValue is greater than 0, the contract initiates a low-level call using:

The low-level .call() method allows the recipient (in this case, msg.sender) to execute arbitrary code. If the msg.sender is a smart contract, it could include a fallback function or exploit that re-enters the claimAllRewards function before the state is updated.

• State Update Vulnerability: After transferring the funds, the contract proceeds to delete the rewardsOwned[msg.sender] array. However, if an attacker re-enters the function (via the fallback function) during the call operation, they can trigger multiple executions of the claimAllRewards function, each time receiving the full totalValue.

  This happens because the state (rewardsOwned[msg.sender]) is not updated (i.e., deleted) until after the funds transfer. Therefore, on each reentry, the array still holds the rewards, allowing the attacker to continuously withdraw.

POC:

*I Chained the bug together just to demonstrate how easy it could be to Exploit this Vulnerability *

• 1=> Deploy the MysteryBox.sol Contract In Remix IDE and Fund The Contract

• 2=> Deploy My Exploit Contract Using the MysteryBox.sol Contract Address And Fund it With any Amount (1 ether)

• 3=> call the buy function in my exploit and start calling the attack function i chained it with the randomness issue to have a quick win but when ever their is any win it triggers the `Reentrancy Vulnerability` and drain all the balance in the game contract

Impact

• Fund Drainage: An attacker can continuously withdraw funds from the contract until its balance is exhausted.

• Denial of Service: The contract may run out of funds, preventing legitimate users from claiming their rewards.

Tools Used

• Remix IDE

• gas used to avoid revert while performing the attack: 80000000

Recommendations

To mitigate the reentrancy vulnerability, the function should update the state before transferring any funds. This can be achieved by using the Checks-Effects-Interactions pattern, where the contract’s state is updated before any external calls are made. The updated function should look like this:

By deleting rewardsOwned[msg.sender] before sending the funds, the attacker cannot re-enter the function and claim rewards multiple times.

Recommendations:

1. Implement Checks-Effects-Interactions Pattern: Always update the contract’s state before making external calls to avoid reentrancy vulnerabilities.

2. Consider Using OpenZeppelin’s ReentrancyGuard: This library provides a simple modifier (nonReentrant) that can be applied to functions to prevent reentrancy attacks.

3. Limit Gas for .call(): Limit the gas available for external calls to reduce the chances of an attacker executing complex fallback functions during the call.