Beginner FriendlyFoundryNFT
100 EXP
View results
Submission Details
Severity: high
Valid

`PuppyRaffle::selectWinner` function dangerous uint64 cast which can cause `PuppuRaffle::withdrawFees` malfunction

Summary

The PuppyRaffle::selectWinner function accumulates fee in PuppyRaffle::totalFees variable. The problem is we cast fee which is a uint256 in a uint64, therefore if the value of fee is higher than 2**64-1 a big portion of the fees will not be accumulated in PuppyRaffle::totalFees variable and make the function PuppyRaffle:withdrawFees malfunction forever.

Vulnerability Details

The function PuppyRaffle::withdrawFees which has a require such as

require(address(this).balance == uint256(totalFees), "PuppyRaffle: There are currently players active!");

will never be fulfilled if we truncated a portion of the fee.
This is likely to happen if

PuppyRaffle::selectWinner::fee > 2**64-1

which mean

PuppyRaffle::selectWinner::totalAmountCollected > ((2**64-1)*100)/20

depending on value of entraceFee it means this condition will be satisfied if

players.length > (((2**64-1)*100)/20)/entranceFee

If this scenario happen we will always have in PuppyRaffle::withdrawFees function this

address(this).balance > uint256(totalFees)

therefore this function will always revert and the fees will be lost forever in the smart contract.

Impact

If the game is popular and there are lots of player this is likely to happen and the consequences is lost of funds therefore it is a high vulnerability

Tools Used

reading the code

Recommendations

-totalFees = totalFees + uint64(fee);
+totalFees = totalFees + fee;
Updates

Lead Judging Commences

Hamiltonite Lead Judge about 2 years ago
Submission Judgement Published
Validated
Assigned finding tags:

greifers-send-money-to-contract-to-block-withdrawfees

unsafe cast of fee to uint64

Support

FAQs

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

Give us feedback!