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

Dos attack via transferring claimed box

0x37

Summary

Function claimAllRewards() may be reverted if malicious uses transfer lots of claimed box.

Vulnerability Details

Function claimAllRewards() will be used to claim all boxes's rewards. If there're lots of box, the claimAllRewards() may be reverted because of out of gas.
Malicious users can claim his box's rewards and then transfer this to the victim. And we can repeat transferring claimed box to the victim to inflat array rewardsOwned's length.
When the victim wants to claim all his rewards via transferReward, this function will be reverted because of out of gas.

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

Impact

Function claimAllRewards() may be reverted because of the long array. Users have to pay more gas to claim each box's rewards.

Tools Used

Manual

Recommendations

Add some check in transferReward, if this box has already claimed, we should not transfer this box.

Updates

Lead Judging Commences

inallhonesty Lead Judge
about 1 year ago

Appeal created

inallhonesty Lead Judge about 1 year ago
Submission Judgement Published
Validated
Assigned finding tags:

A user can poison the `rewardsOwned` of another user via `transferReward` of an empty reward index

Support

FAQs

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

Give us feedback!