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

`MysteryBox.sol::claimAllRewards`, `MysteryBox.sol::claimSingleReward` reentrancy vulnerabilities

4th05

Relevant GitHub Links

https://github.com/Cyfrin/2024-09-mystery-box/blob/main/src/MysteryBox.sol#L86-L89

https://github.com/Cyfrin/2024-09-mystery-box/blob/main/src/MysteryBox.sol#L97-L100

Summary

In the `MysteryBox.sol::claimAllRewards` and `MysteryBox.sol::claimSingleReward` the user can withdraw more money than what should be permitted by reentering the same function many times

Vulnerability Details

In the `MysteryBox.sol::claimAllRewards` `MysteryBox.sol::claimSingleReward` functions the payment is made before the update of all the rewards of the user in the array called `rewardsOwned`

Impact

The user can drain the protocol funds by reentering the functions called `MysteryBox.sol::claimAllRewards` and `MysteryBox.sol::claimSingleReward`

Tools Used

Manual review

Recommendations

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];
​
+ delete rewardsOwned[msg.sender];
+ (bool success,) = payable(msg.sender).call{value: totalValue}("");
+ require(success, "Transfer failed");
}
​
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];
​
+ delete rewardsOwned[msg.sender][_index];
+ (bool success,) = payable(msg.sender).call{value: value}("");
+ require(success, "Transfer failed");
}
​
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.