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

[H-1] Potential Reentrancy Vulnerability in MysteryBox::claimAllRewards Function

ghufranhassan1

Summary

The claimAllRewards function in the MysteryBox contract allows users to claim all their rewards in Ether based on the value of the rewards they have accumulated. Ether transfer operations (call) occurred before updating the contract's internal state.

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];
}

Vulnerability Details

The claimAllRewards function in the MysteryBox contract is responsible for allowing users to claim all of their accumulated rewards in Ether. In a scenario where the contract interacts with external accounts (e.g., transferring Ether via call), it becomes susceptible to a reentrancy attack if state updates are not properly handled prior to external interactions.

Reentrancy vulnerabilities occur when a contract allows external calls (such as sending Ether) before updating its internal state. An attacker can exploit this by repeatedly invoking the function before the internal state (in this case, the rewards) is cleared, potentially draining the contract's funds.

If the function does not follow the Checks-Effects-Interactions (CEI) pattern, an attacker could:

  1. Call claimAllRewards.

  2. Receive the Ether while simultaneously re-entering the contract through the callback mechanism.

  3. Claim rewards multiple times by re-entering the function before the rewards are deleted, draining the contract of its funds.

Impact

The contract could lose significant funds if an attacker manages to re-enter the function before the rewards state is updated.

Tools Used

Manual Review

Recommendations

Apply CEI pattern to mitigate reentrancy attack.

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");
​
// Effects: Update state before interacting with external contract
delete rewardsOwned[msg.sender];
​
// Interactions: Send Ether after the state has been updated
(bool success,) = payable(msg.sender).call{value: totalValue}("");
require(success, "Transfer failed");
}
​
Updates

Lead Judging Commences

inallhonesty Lead Judge
8 months ago

Appeal created

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

`claimAllRewards` reentrancy

Support

FAQs

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