First Flight #2: Puppy Raffle
First Flight #2
Beginner FriendlyFoundryNFT
100 EXP
View results
Previous Next
Submission Details
Severity: high
Valid

Integer overflow of totalFees loses fees

hueber

[H-3] Integer overflow of total fees loses fees

Description:
When accumulating the fees from entranceFee the variable totalFees can overflow, because its type is uint64 and after for example 20-30 entrances it will overflow and wrap around and start from 0. This will cause loss of fees.

Impact:
Less incentive for the user running the contract.

Tools used:
foundry

Proof of Concept:

function testTotalFeesOverflowCausesFeeLoss() public {
uint256 pseudoAddress = 0;
address[] memory arr = new address[](1);
uint64 maxTotalFees = 2**64-1;
uint256 expectedAccumulatedFees = 0;
// Add players to the raffle until the total fees overflow
while(maxTotalFees > expectedAccumulatedFees) {
pseudoAddress += 1;
arr[0] = address(pseudoAddress);
puppyRaffle.enterRaffle{value: entranceFee}(arr);
expectedAccumulatedFees += entranceFee * 20 / 100;
}
​
vm.warp(block.timestamp + duration + 1);
vm.roll(block.number + 1);
​
puppyRaffle.selectWinner();
assert(puppyRaffle.totalFees() < expectedAccumulatedFees);
}

Recommended Mitigation:

Make the totalFees variable of type uint256.

diff --git a/src/PuppyRaffle.sol b/src/PuppyRaffle.sol
index b37922b..2685900 100644
--- a/src/PuppyRaffle.sol
+++ b/src/PuppyRaffle.sol
@@ -28,7 +28,7 @@ contract PuppyRaffle is ReentrancyGuard, ERC721, Ownable {

 // We do some storage packing to save gas
 address public feeAddress;

  • uint64 public totalFees = 0;

  • uint256 public totalFees = 0;

    uint256 public raffleId = 1;
    mapping(address => uint256) addressToRaffleId;

Updates

Lead Judging Commences

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

overflow-uint64

Support

FAQs

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