First Flight #2: Puppy Raffle

Summary

The PuppyRaffle contract is vulnerable to integer overflow. As the variable totalFees is declared as uint64, if it ever exceeds the approximate value of 18.5 ether, it will silently overflow. This may lead to the ETH getting stuck in the contract forever.

Vulnerability Details

The PuppyRaffle smart contracts uses the Solidity version < 0.8, which is susceptible to integer overflow and underflow.

The variable totalFees is declared as uint64, therefore its maximum value is 2^64-1, approximetaly 18.5 ether.

This variable is used to store the current amount of fees cumulated from the previous rounds of raffle. At any point anyone can call the withdrawFees(), which will transfer the totalFees amount of ETH to the feeAddress, and set totalFees to zero.

In other words, the fees from raffle rounds get cumulated in the totalFees variable until someone withdraws them to feeAddress.

The problem is that if this sum ever exceeds the maximum capacity of uint64, the overflow will happen and the totalFees will start counting again from zero. Therefore, the variable totalFees, instead of storing the actual sum of not-yet-withdrawn fees, will store the sum of not-yet-withdrawn fees modulo uint64.max.

If this overflow ever happens, meaning that at any point the cumulative sum of not withdrawn totalFees will exceed the value of approximetaly 18.5 ether, the fees will get stuck in the contract forever. This is due to the strong equality check at the beginning of the withdrawFees() method:

If the overflow occured, this condition will never be met. Therefore the fees will be non-recoverable.

Impact

ETH getting stuck forever in the contract.

Tools Used

Manual review

Recommendations

Use 0.8.x Solidity version or utilize OpenZeppelin's SafeMath library for the totalFees calculation.