First Flight #2: Puppy Raffle

The fee is calculated in PuppyRaffle::selectWinner using a divide by 100. This value can have decimals which will be lost as uint256 cannot handle decimal values. Since fee is a uint256 value, any decimal portion will be truncated due to floor division in Solidity.

Vulnerability Details

In PuppyRaffle::selectWinner, on line 133, the fee is determined using the following calculation:

If totalAmountCollected * 20 is not exactly divisible by 100, there will be a precision loss due to the inability of uint256 to handle decimal values and the fee will be truncated. The remaining funds lost due to the truncation will be locked within the contract.

Impact

The fee will be smaller than the intended amount, up to 99 Wei, leading to a smaller value for totalFees. The remaining funds are locked in the contract. The feeAddress won't be able to withdraw the intended value of totalFees.

This will lead to a denial of service calling withdrawFees() as the contract balance will not be equal to totalFees even if no players are active. See issue "Unable to withdraw fees if contract balance is not equal to totalFees when no players are active" for more details. Based on the values in the deploy script of 1 ether for the entranceFee, this is unlikely but if the script is ever modified to use smaller units of value, this would have a high impact. This is therefore a medium-severity issue.

Recommended Mitigation

To mitigate the precision loss in the fee calculation, use fixed-point arithmetic. Use a FixedPoint library, e.g. from OpenZeppelin, for fixed-point arithmetic. This library is only compatible with Solidity versions 0.8.0 or later so the Solidity version will need to be updated.

Tools Used

• Forge