Mystery Box

First Flight #25
Beginner FriendlyFoundry
100 EXP
View results
Submission Details
Severity: high
Valid

Reentrancy Vulnerabilities in reward claiming functions

Summary

The claimAllRewards and claimSingleReward functions in the MysteryBox contract are vulnerable to reentrancy attacks. These functions transfer ETH to the user before updating the contract state, potentially allowing an attacker to recursively call the functions and drain funds from the contract.

Vulnerability Details

The vulnerability exists in both the claimAllRewards and claimSingleReward functions:

function claimAllRewards() public {
uint256 totalValue = 0;
for (uint256 i = 0; i < rewardsOwned[msg.sender].length; i++) {
totalValue += rewardsOwned[msg.sender][i].value;
}
require(totalValue > 0, "No rewards to claim");
@> (bool success,) = payable(msg.sender).call{value: totalValue}("");
require(success, "Transfer failed");
@> delete rewardsOwned[msg.sender];
}
function claimSingleReward(uint256 _index) public {
require(_index <= rewardsOwned[msg.sender].length, "Invalid index");
uint256 value = rewardsOwned[msg.sender][_index].value;
require(value > 0, "No reward to claim");
@> (bool success,) = payable(msg.sender).call{value: value}("");
require(success, "Transfer failed");
@> delete rewardsOwned[msg.sender][_index];
}

In both functions, the ETH transfer occurs before the state is updated (i.e., before deleting the claimed rewards). This "checks-effects-interactions" pattern violation allows a malicious contract to make a reentrant call back into these functions before the state is updated, potentially claiming the same rewards multiple times.

Impact

The impact of this vulnerability is severe:

  1. Fund Drainage: An attacker could potentially drain all available ETH from the contract by repeatedly claiming the same rewards before the state is updated.

  2. Reward Duplication: Users could claim the same rewards multiple times, unfairly multiplying their earnings at the expense of the contract and other users.

  3. Contract Insolvency: If exploited, this vulnerability could lead to the contract becoming insolvent, unable to pay out legitimate rewards to other users.

  4. Economic Imbalance: The ability to claim rewards multiple times could disrupt the intended economic balance of the reward system, potentially making the game unfair or unsustainable.

The severity of this impact is heightened by the fact that the contract handles real ETH, meaning any exploit would result in direct financial losses.

Tools Used

  • Manual review of the smart contract code

Recommendations

To address this vulnerability, we recommend implementing the checks-effects-interactions pattern by updating the contract state before making external calls. Here's how the functions should be modified:

function claimAllRewards() public {
uint256 totalValue = 0;
for (uint256 i = 0; i < rewardsOwned[msg.sender].length; i++) {
totalValue += rewardsOwned[msg.sender][i].value;
}
require(totalValue > 0, "No rewards to claim");
+ delete rewardsOwned[msg.sender];
(bool success,) = payable(msg.sender).call{value: totalValue}("");
require(success, "Transfer failed");
- delete rewardsOwned[msg.sender];
}
function claimSingleReward(uint256 _index) public {
require(_index <= rewardsOwned[msg.sender].length, "Invalid index");
uint256 value = rewardsOwned[msg.sender][_index].value;
require(value > 0, "No reward to claim");
+ delete rewardsOwned[msg.sender][_index];
(bool success,) = payable(msg.sender).call{value: value}("");
require(success, "Transfer failed");
- delete rewardsOwned[msg.sender][_index];
}

By implementing these changes, the contract updates its state before making external calls, significantly reducing the risk of reentrancy attacks while maintaining the core functionality of the reward claiming process.

Updates

Appeal created

inallhonesty Lead Judge 11 months ago
Submission Judgement Published
Validated
Assigned finding tags:

`claimAllRewards` reentrancy

`claimSingleReward` reentrancy

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.