Submission Details
Severity:
Reentrancy in MysteryBox::claimAllRewards allows to withdraw all funds from the contract
Summary
Function MysteryBox::claimAllRewards does not follow CEI pattern and this allows to re-enter to the function and claim rewards again before the rewards array for the user is deleted.
Vulnerability Details
Function MysteryBox::claimAllRewards deletes rewardsOwned array with the user's rewards after sending the rewards to the user:
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];
}
This allows to re-enter to the function and claim the rewards again.
Impact
An attacker can withdraw all funds from the contract.
Tools Used
Manual review
Recommendations
Change the function to follow CEI pattern as below:
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];
}
Updates
Appeal created
inallhonesty about 1 year ago
Submission Judgement Published Assigned finding tags:
`claimAllRewards` reentrancy
Rewards breakdown
nSLOC
86This contest has a flat reward structure with the following payouts:
100 EXP
20 EXP
2 EXP
Live
The contest is live. Earn rewards by submitting a finding.
Appeals
This is your time to appeal against judgements on your submissions.
Appeals Review
Appeals are being carefully reviewed by our judges.