MyCut
First Flight #23
Beginner FriendlyFoundry
100 EXP
View results
Previous Next
Submission Details
Severity: high
Invalid

In closePot()::Pot.sol, "playersToRewards" should be used instead of "claimants"

0xziin

Summary

In closePot()::Pot.sol, playersToRewards should be used instead of claimants.

Vulnerability Details

In closePot()::Pot.sol, claimants is used but it is the wrong variable :

...
for (uint256 i = 0; i < claimants.length; i++) {
_transferReward(claimants[i], claimantCut);
}
...

Because, as seen in claimCut()::Pot.sol function :

function claimCut() public {
address player = msg.sender;
uint256 reward = playersToRewards[player];
if (reward <= 0) {
revert Pot__RewardNotFound();
}
playersToRewards[player] = 0;
remainingRewards -= reward;
claimants.push(player); // <======= Here
_transferReward(player, reward);
}

https://github.com/Cyfrin/2024-08-MyCut/blob/main/src/Pot.sol#L45


==> claimants are the players who have already claimed their cut. It is not the appropriate variable.

Impact

It will lead to a false track of who claimed and who is going to be rewarded after 90 minus a cut.

Users who have already claimed will be rewarded again, with a cut of 10 percent.
==> And users who haven't claimed
before 90 days, won't ever be rewarded !

Tools Used

Github, VisualCode.

Recommendations

Use playersToRewards instead of claimants.

mapping(address => uint256) private playersToRewards;
Updates

Lead Judging Commences

equious Lead Judge 12 months ago
Submission Judgement Published
Invalidated
Reason: Design choice

Support

FAQs

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