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

`MysteryBox::claimAllRewards` and `MysteryBox::claimSingleReward` functions are vulnerable to reentrancy attack.

SyncAudit

Summary

MysteryBox::claimAllRewards and MysteryBox::claimSingleReward make state changes only after transfering ether to the destination address. This allows the destinationation address (contract) to make several callbacks to either of the functions and drain off all the funds in the contract.

Vulnerability Details

MysteryBox::claimAllRewards and MysteryBox::claimSingleRewarddid not make state changes before making external call, making them vulnerable to a reentrancy attack. The called address (contract) can callback the MysteryBox::claimAllRewards or MysteryBox::claimSingleReward in the same transaction and drain all the funds in the contract.

Impact

All the funds in the contract will be stolen.

Tools Used

manual review.

Recommendations

Refactor the MysteryBox::claimAllRewards and MysteryBox::claimSingleReward as shown 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];
}
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 about 1 year 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.

Give us feedback!