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

Unchecked Math for tracking funds

0xethanol

Vulnerability Details

Because of the dependencies of the contract, the Solidity version of this contract needs to be above 0.7.6 but below of 0.8.0, this posses high risks, in the release of 0.8.0 the Solidity team introduced the 'safeMath' functionality in Solidity, that checked for over and underflows in numbers

Impact

High - With the way of how the program is written the variable totalFees could overflow if enough raffles has passed and the feeAddress hasn't withdrawn, blocking funds in the smart contract for ever.

PoC

  1. We create a function that creates multiple raffles, (In this example we are dealing with 100 players each raffle)

  2. This test should fail at the run number 11.

Code

Copy this code into PuppyRaffleTest.t.sol:

​
function testForTotalFeesOverFlow() public {
​
for (uint256 i = 0; i < 20; i++) {
​
console.log("\n run number: ", i, "\n");
add100PlayersToRaffle(puppyRaffle);
​
uint256 feesBefore = puppyRaffle.totalFees();
console.log("totalFees before: ", feesBefore);
// select winner
vm.warp(puppyRaffle.raffleStartTime() + puppyRaffle.raffleDuration());
puppyRaffle.selectWinner();
uint256 feesAfter = puppyRaffle.totalFees();
console.log("totalFees after: ", feesAfter);
​
require(feesBefore < feesAfter);
}
​
}
​
/// @dev This function is made made for testing the raffle
/// @dev This should always be called for a new raffle with no players to not have problems with douplicates
/// @param _puppyRaffle PuppyRaffle contract
function add100PlayersToRaffle(PuppyRaffle _puppyRaffle) public {
​
uint num = 100;
​
address[] memory players = new address[](num);
for (uint i = 0; i < num; i++) {
players[i] = address(i + 10);
}
_puppyRaffle.enterRaffle{value: entranceFee * num}(players);
​
}
​

Recommendations

The use of SafeMath library

For any manipulation of numbers in Solidity < 0.8.0, it should be use the library SafeMath to ensure that there are no over or underflows.

Change the variable type

Change the variable from uint64 to uint256 to minimize this problem.

With these modifications we ensure that if there's an overflow or underflow the transaction will be reverted. And by using a uint256 it will take a while until totalFees overflows.

Updates

Lead Judging Commences

Hamiltonite Lead Judge about 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.

Give us feedback!