Summary
totalFees is stored as uint64 which can lead to an integer overflow and loss of funds.
Vulnerability Details
type(uint64).max is 18446744073709551615 which is the maximum value a uint64 can store. Since fees are denominated in Wei, this value translates to approximately 18.5 ether. Given that the used compiler version of pragma solidity ^0.7.6 has no overflow checks, this leads to an overflow of totalFees once the fees exceed this limit, which leads to a irreversible loss of fees. Additionally, given that the withdrawFees function checks whether the contract balance is equal to totalFees, this will revert the withdrawFees function.
address public feeAddress;
@> uint64 public totalFees = 0;
function selectWinner() external {
require(block.timestamp >= raffleStartTime + raffleDuration, "PuppyRaffle: Raffle not over");
require(players.length >= 4, "PuppyRaffle: Need at least 4 players");
uint256 winnerIndex =
uint256(keccak256(abi.encodePacked(msg.sender, block.timestamp, block.difficulty))) % players.length;
address winner = players[winnerIndex];
uint256 totalAmountCollected = players.length * entranceFee;
uint256 prizePool = (totalAmountCollected * 80) / 100;
uint256 fee = (totalAmountCollected * 20) / 100;
@> totalFees = totalFees + uint64(fee);
uint256 tokenId = totalSupply();
uint256 rarity = uint256(keccak256(abi.encodePacked(msg.sender, block.difficulty))) % 100;
if (rarity <= COMMON_RARITY) {
tokenIdToRarity[tokenId] = COMMON_RARITY;
} else if (rarity <= COMMON_RARITY + RARE_RARITY) {
tokenIdToRarity[tokenId] = RARE_RARITY;
} else {
tokenIdToRarity[tokenId] = LEGENDARY_RARITY;
}
delete players;
raffleStartTime = block.timestamp;
previousWinner = winner;
(bool success,) = winner.call{value: prizePool}("");
require(success, "PuppyRaffle: Failed to send prize pool to winner");
_safeMint(winner, tokenId);
}
function withdrawFees() external {
@> require(address(this).balance == uint256(totalFees), "PuppyRaffle: There are currently players active!");
uint256 feesToWithdraw = totalFees;
totalFees = 0;
(bool success,) = feeAddress.call{value: feesToWithdraw}("");
require(success, "PuppyRaffle: Failed to withdraw fees");
}
Impact
Change the code in PuppyRaffleTest.t.sol and run forge test --mt testUint64FeeCastingCanLeadToOverflow and forge test --mt testUint64FeeCastingCanLeadToLossOfFees. The first test fails while the second passes.
PuppyRaffle puppyRaffle;
ReentrancyTest reentrancyTest;
- uint256 entranceFee = 1e18;
+ uint256 entranceFee = 100e18;
address playerOne = address(1);
address playerTwo = address(2);
address playerThree = address(3);
address playerFour = address(4);
address feeAddress = address(99);
uint256 duration = 1 days;
// rest of code of `PuppyRaffleTest`
+ function testUint64FeeCastingCanLeadToOverflow() public playersEntered {
+ uint256 expectedPrizeAmount = ((entranceFee * 4) * 20) / 100;
+ vm.warp(block.timestamp + duration + 1);
+ vm.roll(block.number + 1);
+ puppyRaffle.selectWinner();
+ assertEq(puppyRaffle.totalFees(), expectedPrizeAmount);
+ }
+ function testUint64FeeCastingCanLeadToLossOfFees() public playersEntered {
+ vm.warp(block.timestamp + duration + 1);
+ vm.roll(block.number + 1);
+ puppyRaffle.selectWinner();
+ vm.expectRevert();
+ puppyRaffle.withdrawFees();
+ }
Tools Used
Recommendations
Store totalFees as uint256. Additionally, consider importing SafeMath.sol from OpenZeppelin. Alternatively, use at least pragma solidity 0.8.0 as compiler as it comes with built-in overflow protection.
// We do some storage packing to save gas
address public feeAddress;
- uint64 public totalFees = 0;
+ uint256 public totalFees;
// rest of code of `PuppyRaffle`
function selectWinner() external {
require(block.timestamp >= raffleStartTime + raffleDuration, "PuppyRaffle: Raffle not over");
require(players.length >= 4, "PuppyRaffle: Need at least 4 players");
uint256 winnerIndex =
uint256(keccak256(abi.encodePacked(msg.sender, block.timestamp, block.difficulty))) % players.length;
address winner = players[winnerIndex];
uint256 totalAmountCollected = players.length * entranceFee;
uint256 prizePool = (totalAmountCollected * 80) / 100;
uint256 fee = (totalAmountCollected * 20) / 100;
- totalFees = totalFees + uint64(fee);
+ totalFees += fee;
uint256 tokenId = totalSupply();
// We use a different RNG calculate from the winnerIndex to determine rarity
uint256 rarity = uint256(keccak256(abi.encodePacked(msg.sender, block.difficulty))) % 100;
if (rarity <= COMMON_RARITY) {
tokenIdToRarity[tokenId] = COMMON_RARITY;
} else if (rarity <= COMMON_RARITY + RARE_RARITY) {
tokenIdToRarity[tokenId] = RARE_RARITY;
} else {
tokenIdToRarity[tokenId] = LEGENDARY_RARITY;
}
delete players;
raffleStartTime = block.timestamp;
previousWinner = winner;
(bool success,) = winner.call{value: prizePool}("");
require(success, "PuppyRaffle: Failed to send prize pool to winner");
_safeMint(winner, tokenId);
}
