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

Reentrancy attack in claimAllRewards(), claimSingleReward()

newspacexyz

Summary

Using call to transfer Ether opens the function to reentrancy attacks.

Vulnerability Details

The use of call to transfer Ether is potentially vulnerable to reentrancy attacks, especially since state changes (i.e., deleting rewardsOwned[msg.sender]) occur after the transfer. If an attacker can re-enter the contract in the middle of the execution, they can call claimAllRewards multiple times before the state is updated, effectively draining the contract.

This logic also works in claimSingleReward().

Impact

If an attacker successfully performs a reentrancy attack, they could claim rewards multiple times, draining the contract’s funds.

Tools Used

Manual review

Recommendations

function claimAllRewards() public nonReentrant {
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];
}
Updates

Appeal created

inallhonesty Lead Judge 9 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.