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

Overflow/Underflow vulnerabilty for any version before 0.8.0

azmaeengh

Summary

The PuppyRaffle.sol uses Solidity compiler version 0.7.6. Any Solidity version before 0.8.0 is prone to Overflow/Underflow vulnerability. Short example - a uint8 x; can hold 256 values (from 0 - 255). If the calculation results in x variable to get 260 as value, the extra part will overflow and we will end up with 5 as a result instead of the expected 260 (because 260-255 = 5).

Vulnerability Details

I have two example below to demonstrate the problem of overflow and underflow with versions before 0.8.0, and how to fix it using safemath:

Without SafeMath:

function withoutSafeMath() external pure returns (uint256 fee){
uint8 totalAmountCollected = 20;
fee = (totalAmountCollected * 20) / 100;
return fee;
}
// fee: 1
// WRONG!!!

In the above code,without safeMath, 20x20 (totalAmountCollected * 20) was 400, but 400 is beyond the limit of uint8, so after going to 255, it went back to 0 and started counting from there. So, 400-255 = 145. 145 was the result of 20x20 in this code. And after dividing it by 100, we got 1.45, which the code showed as 1.

With SafeMath:

function withSafeMath() external pure returns (uint256 fee){
uint8 totalAmountCollected = 20;
fee = totalAmountCollected.mul(20).div(100);
return fee;
}
// fee: 4
// CORRECT!!!!

This code didnt suffer from Overflow problem. Because of the safeMath, it was able to calculate 20x20 as 400, and then divided it by 100, to get 4 as result.

Impact

Depending on the bits assigned to a variable, and depending on whether the value assigned goes above or below a certain threshold, the code could end up giving unexpected results.
This unexpected OVERFLOW and UNDERFLOW will result in unexpected and wrong calculations, which in turn will result in wrong data being used and presented to the users.

Tools Used

Got suggestions from AI tool phind. Tested the above code (with and without safeMath) on remix.ethereum.org

Recommendations

Modify the code to include SafeMath:

  1. First import SafeMath from openzeppelin:

import "@openzeppelin/contracts/math/SafeMath.sol";

  1. then add the following line, inside PuppyRaffle Contract:

using SafeMath for uint256;

(can also add safemath for uint8, uint16, etc as per need)

  1. Then modify the require inside enterRaffle() function:

- require(msg.value == entranceFee * newPlayers.length, "PuppyRaffle: Must send enough to enter raffle");
+ uint256 totalEntranceFee = newPlayers.length.mul(entranceFee);
+ require(msg.value == totalEntranceFee, "PuppyRaffle: Must send enough to enter raffle");

  1. Then modify variables (totalAmountCollected, prizePool, fee, and totalFees) inside selectWinner() function:

- uint256 totalAmountCollected = players.length * entranceFee;
+ uint256 totalAmountCollected = players.length.mul(entranceFee);
​
- uint256 prizePool = (totalAmountCollected * 80) / 100;
+ uint256 prizePool = totalAmountCollected.mul(80).div(100);
​
- uint256 fee = (totalAmountCollected * 20) / 100;
+ uint256 fee = totalAmountCollected.mul(20).div(100);
​
- totalFees = totalFees + uint64(fee);
+ totalFees = totalFees.add(fee);

This way, the code is now safe from Overflow/Underflow vulnerabilities.

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.