Blast the balloon SC Audit
User won prizes can be claimed using the claimPrize function. This function will validate that guaranteed prizes exist before allowing the claim to be executed. Prizes such as prizes from previously won rounds and current minor jackpot wins. There are rewards that can change during a round, the major jackpot wins: blast master and king blast. These rewards will only be claimable (guaranteed) after the round has ended.
The claim mechanism has several issues originating from the way pending rewards are managed and marked as collected. The most severe issue is that a user that has claimed rewards in between rounds, can again claim the pending rewards in the next round, effectively doubling his major jackpot rewards.
The situation appears because the claimPrize retrieves both the committed prizes and the pending prizes but does not mark the pending rewards as claimed.
require(claimableAmounts[_msgSender()] > 0, BlastTheBalloonErrors.CLAIMED);
uint allPrize = claimableAmounts[_msgSender()] + pendingAmount;
claimableAmounts[_msgSender()] = 0;
(bool success, ) = payable(_msgSender()).call{value: allPrize}("");
The claim call clears the committed (claimableAmounts) rewards only. Any new claim without claimable rewards, will fail.
A secondary issue from this design is that a user, with no prior rewards pending, with no current claimable rewards (minor jackpot wins) but with major jackpot wins cannot withdraw his prizes until a new round has started.
Coming back to the claimPrize function, a second adjutant issue but less relevant is that, since claimPrize does not mark the pending rewards as claimed, the getCurrentClaimableAmounts and getClaimableAmount functions incorrectly show that the pending amount is still claimable in between rounds.
After a claim during rounds, which extracted both the pending and the claimable amounts, after a new round has started, a new claim will trigger the function responsible with updated the pending rewards _calculateLastPrize. This function then marks as claimable the incorrect getActualClaimableAmounts result, which includes the previous already-collected pending amounts, thus doubling rewards.
if (playerInfo.lastPlayRound >= nonce) {
return;
}
claimableAmounts[player] = getActualClaimableAmounts(player);
playerInfo.lastPlayRound = nonce;
As the system is right now, there are not enough data points to be able to identify if a user is in between rounds and has already claimed. The available information is the claimable amount (that is safe to extract), the separate won major jackpot prizes, if the round has ended, the player last played round and the last round or current round if it is still ongoing.
The constraint that makes these data points insufficient is that the pending amounts variable cannot be deleted after being used. This is because it has the dual role of keeping a record of which player won what prize.
An extra data point is needed to resolve this issue. Either create another mapping which holds the information if a user has claimed his major jackpot winnings or add a flag in the already existing player data structure Player. When the player withdraws his pending amounts, set the respective flag.
Change the way the current prize claiming and accumulation works such that the claimableAmounts variable is the only source of truth for what a user has one. Transfers must only originate from it.
If a claim is done and the latest user played round is done, then add the pending amount to the claimableAmounts, mark that the user has claimed pending rewards and withdraw the claimableAmounts amount directly. One of the subtleties that allowed this issue to appear is that the check that claimable rewards existed was done on a different amount then what was subsequently transferred to user.
While taking into consideration the above, also change the way getClaimableAmount, getCurrentClaimableAmounts and getActualClaimableAmounts work so that they also correctly handler claimable rewards.
Blast the balloon SC Audit