Mystery Box

Summary

The claimAllRewards function in the MysteryBox contract was identified as vulnerable to reentrancy attacks, potentially allowing malicious actors to drain contract funds. Tests demonstrated that without proper protection, this vulnerability could be exploited. Conversely, the claimSingleReward function proved to be secure against such attacks, demonstrating effective state management practices. This report emphasizes the crucial need for reentrancy protection in functions dealing with external calls.

Vulnerability Details

The claimAllRewards function lacked appropriate safeguards against reentrancy, exposing it to potential fund-draining attacks. The function allowed an attacker to repeatedly withdraw rewards without first updating the contract state, resulting in multiple payouts in a single transaction.

Details:

• State not updated before transfer: The function transferred Ether without first modifying the contract’s state, allowing an attacker to reenter and repeatedly claim rewards.

• Impact: An attacker could exploit this flaw to drain all funds from the contract over several iterations.

In contrast, the claimSingleReward function avoided this vulnerability by employing state modifications (deletion of the reward entry) before transferring funds, adhering to the "Check-Effects-Interactions" pattern.

Code Analysis

Original claimAllRewards Function:

Key Issues:

• The delete operation occurred after the transfer of Ether, making the function vulnerable to reentrancy attacks.

• An attacker could exploit this by invoking the fallback function and reentering claimAllRewards.

Secure Implementation in claimSingleReward:

Key Strength:

• The delete operation happens before the transfer of Ether, which effectively prevents a reentrancy attack. By removing the reward entry first, any attempt to reenter would find that the state has already been altered, stopping the exploit in its tracks.

Modified Function with Reentrancy Protections:

To address the vulnerabilities, we applied the following mitigations:

1. Check-Effects-Interactions Pattern: Updated the state before transferring Ether.

2. NonReentrant Modifier: Utilized OpenZeppelin’s ReentrancyGuard to prevent reentrant calls.

3. Pull-over-Push Mechanism: Adopted a safer "Pull" mechanism where the user actively withdraws their rewards.

Protection Results:

• The claimAllRewards function is now resistant to reentrancy attacks, with the state updated before any external call.

Proof of Concept (PoC)

1. Before Protection: We conducted a test to simulate a reentrancy attack on the claimAllRewards function using the reentrancyAttacker contract. The attacker initiated the attack with an initial value of 0.1 ether. As the attack proceeded, the attacker was able to repeatedly call the claimAllRewards function before the state was updated, allowing them to drain multiple rewards from the MysteryBox contract.

2. Attack Execution:

   • During the test, the attacker successfully exploited the lack of state update prior to the external transfer call, repeatedly invoking the claimAllRewards function.

   • After the attack, the logs showed that the attacker’s contract balance increased to 7 ether, while the MysteryBox contract's balance reduced to 44.1 ether, confirming the successful drain of funds.

Results:

The test passed, showing that the attacker successfully drained funds:

• Balance of MysteryBox after the attack: 44.1 ether

• Balance of ReentrancyAttacker after the attack: 7 ether

This confirmed that the claimAllRewards function was indeed vulnerable to reentrancy, as the attacker was able to drain funds before the state was updated.

1. After Protection: Applying the "Check-Effects-Interactions" pattern and using the nonReentrant modifier effectively blocked the reentrancy attack. The state was updated before transferring Ether, preventing the attacker from reentering the claimAllRewards function.

By running the same test again post-protection, the attacker was unable to execute the reentrancy attack, and the contract retained its funds, demonstrating that the vulnerability was successfully mitigated.

Impact

• Critical Fund Drain: The claimAllRewards vulnerability allowed complete draining of the contract’s Ether.

• Proof of Effective Mitigation: The function now follows best practices, preventing reentrancy and ensuring fund security.

Findings on claimSingleReward:

The claimSingleReward function was found to be secure from the beginning due to the "Check-Effects-Interactions" pattern, as it deleted the reward entry before making any external call.

Tools Used

• Manual code analysis

• Foundry for reentrancy testing

• OpenZeppelin’s ReentrancyGuard library for mitigation

Recommendations

1. Implement NonReentrant Modifier: Apply the nonReentrant modifier from OpenZeppelin’s ReentrancyGuard to all functions handling Ether transfers.

2. Follow Check-Effects-Interactions Pattern: Always update state variables before making external calls.

3. Use Pull-Over-Push Mechanism: Adopt the pull mechanism, requiring users to withdraw their rewards actively.

Test Cases

1. Test for Reentrancy Attack on claimAllRewards

2. Verification of claimSingleReward Protection

Conclusion

The claimAllRewards function was successfully protected against reentrancy attacks by implementing the "Check-Effects-Interactions" pattern, using the nonReentrant modifier, and shifting towards a "Pull-over-Push" mechanism. In contrast, the claimSingleReward function was already secure against reentrancy due to the correct use of state management practices.

These improvements ensure the MysteryBox contract’s resilience against reentrancy attacks, safeguarding user funds and contract integrity.