First Flight #2: Puppy Raffle

Summary

HIGH-6: Rounding issues when calculating fees in the selectWinner function causing dicrepancies in the contract balance handling

Vulnerability Details

The two key variables determining the prizePool and fees are calculated in the following way:
uint256 prizePool = (totalAmountCollected * 80) / 100;
uint256 fee = (totalAmountCollected * 20) / 100;

For this example, we will work with a entranceFee of 34 wei (but can be any other number that after multiplying it with the players number does not round up to a whole 100) and 7 players.

totalAmountCollected = 7 * 34 = 238 wei
prizePool = ( 238 * 80 ) / 100 = 190,40 wei (solidity truncates the remainder) = 190 wei
fee = ( 238 * 20 ) / 100 = 47,60 wei (after truncation of reminder) = 47 wei

The total balance of 238 wei is 190 wei in prize pool, 47 in fees, and 1 wei of rounding error.

Impact

H-6.1: This error causes several issues, primarily with the current state of the "withdrawFees" function that would not be callable as after paying the prizePool, the balance of the contract would be 48 wei, but the fees would only be 47 wei due to rounding error (neglecting the transaction fees). This issue was addressed in H-5.

H-6.2: Secondly, this vulnerability would cause the contract would often have a non-zero balance (unwithdrawn balance) which is not a best practice when withdrawing the remaining funds.

H-6.3: Lastly, the discrepancy may cause up to 99 wei (since we divide by 100 to the whole number) to stay in the contract and not be paid to the prize pool proportionally - a discrepancy that may have an impact on user experience.

Tools Used

Static analysis, local testing, mathematical proof

Recommendations

The H-6.1 was addressed together with H-5 issue.

H-6.2: When withdrawing from the contract (and implementing a recommendation from H-5) use the "address(this).balance" to assign the feesToWithdraw to ensure all the remaining balance is transferred to the feeAddress.

H-6.3: Low-risk vulnerability with limited impact. May be either assigned to the prizePool if the division reminder is greater than zero, or may be kept as is but clarifying this in the raffle conditions to keep the participants informed on the reality of the fee calculation.