Fee address could receive less than what it should resulting in blocked funds
Fee address could receive less than what it should resulting in blocked funds
0xethanol
Vulnerability Details
The fees variable undergoes to a casting to a uint64() from a uint256() making it possible to
Let’s suppose that a raffle was correctly executed, having 100 players, with the entranceFee being 1e18,calculating the fee that the feeAddress should receive should be equal to 20000000000000000000, but because of these line in the code:
totalFees = totalFees + uint64(fee);
The fee variable is not correct because of the casting, instead of adding 20000000000000000000 (2e19) to totalFees it could only add this number: 1553255926290448384 (1.56e18),for perspective, it doesn't surpass 8% of what it should be.
And for the way of how the contract is written, it means that some parts of the balance of the contract will be completely unattainable. In the example of 100 players:
100000000000000000000
prizePool: 80000000000000000000 (8e19)
fees: 1553255926290448384 (1.56e18)
blocked funds: 1.84e19 wei (18 % of the balance)
The number of blocked funds increases for each raffle with more players, for a 1000 players the number of the funds (for that raffle) will be the equivalent of 19.8% of the total balance.
Impact
High - block of funds for feeAddress
Recommendations
Change the variable totalFees to be a uint256 to eliminate this issue, in this way;
// this line
totalFees = totalFees + uint64(fee);
// could change to this
totalFees = totalFees + fee;
And the issue will dissapear for calculating the total fees and no funds will be blocked
Lead Judging Commences
[H-05] Typecasting from uint256 to uint64 in PuppyRaffle.selectWinner() May Lead to Overflow and Incorrect Fee Calculation
## Description ## Vulnerability Details The type conversion from uint256 to uint64 in the expression 'totalFees = totalFees + uint64(fee)' may potentially cause overflow problems if the 'fee' exceeds the maximum value that a uint64 can accommodate (2^64 - 1). ```javascript totalFees = totalFees + uint64(fee); ``` ## POC <details> <summary>Code</summary> ```javascript function testOverflow() public { uint256 initialBalance = address(puppyRaffle).balance; // This value is greater than the maximum value a uint64 can hold uint256 fee = 2**64; // Send ether to the contract (bool success, ) = address(puppyRaffle).call{value: fee}(""); assertTrue(success); uint256 finalBalance = address(puppyRaffle).balance; // Check if the contract's balance increased by the expected amount assertEq(finalBalance, initialBalance + fee); } ``` </details> In this test, assertTrue(success) checks if the ether was successfully sent to the contract, and assertEq(finalBalance, initialBalance + fee) checks if the contract's balance increased by the expected amount. If the balance didn't increase as expected, it could indicate an overflow. ## Impact This could consequently lead to inaccuracies in the computation of 'totalFees'. ## Recommendations To resolve this issue, you should change the data type of `totalFees` from `uint64` to `uint256`. This will prevent any potential overflow issues, as `uint256` can accommodate much larger numbers than `uint64`. Here's how you can do it: Change the declaration of `totalFees` from: ```javascript uint64 public totalFees = 0; ``` to: ```jasvascript uint256 public totalFees = 0; ``` And update the line where `totalFees` is updated from: ```diff - totalFees = totalFees + uint64(fee); + totalFees = totalFees + fee; ``` This way, you ensure that the data types are consistent and can handle the range of values that your contract may encounter.
Rewards breakdown
Live
The contest is live. Earn rewards by submitting a finding.
Judging
Submissions are being reviewed by our AI judge. Results will be available in a few minutes.
View all submissionsRewards Distribution
The contest is complete and the rewards are being distributed.