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

[L-1]: `MysteryBox::openBox`, `MysteryBox::transferReward`, `MysteryBox::claimAllRewards` and `MysteryBox::claimSingleReward` functions do not follow the CEI guidelines

awacs

Summary

The MysteryBox::openBox, MysteryBox::transferReward, MysteryBox::claimAllRewards and MysteryBox::claimSingleReward functions are not aligned with the Checks-Effects-Interactions practice that ensures that all checks are done first, followed by effects (such as changing the storage variables) and then interactions (such as sending rewards). This practice is recommended to be followed to mitigate any risk of reentrancy attacks. Any changes to storage variables should be done before interacting or calling external contracts.

Proof-of-Concept

Example from the MysterxyBox::transferReward function:

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

Tools Used

Static analysis

Recommendations

Update the storage variable before transfering the reward to the caller.

Updates

Appeal created

inallhonesty Lead Judge about 1 year ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity

Support

FAQs

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