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

Possible Precision Loss in `PuppyRaffle::selectWinner` Leads to Loss of Funds

krisrenzo

Summary

Setting the entranceFee to an extremely low value can result in the winner and the protocol losing some or all of their funds due to number rounding errors.

Vulnerability Details

If the entranceFee is set to a very low value, such as 1 wei, the prizePool and fee calculations in the selectWinner function are susceptible to rounding errors. This is because Solidity does not handle decimals well.

function selectWinner() external {
// ...
uint256 totalAmountCollected = players.length * entranceFee;
- uint256 prizePool = (totalAmountCollected * 80) / 100;
- uint256 fee = (totalAmountCollected * 20) / 100;
totalFees = totalFees + uint64(fee);
// ...
}

Proof of Concept

The provided test demonstrates the validity and severity of this vulnerability.

How to Run the Test

Requirements:

  • Install Foundry.

  • Clone the project codebase into your local workspace.

Step-by-step Guide to Run the Test:

  1. Ensure the above requirements are met.

  2. Copy the test below and add it to PuppyRaffleTest.t.sol tests.

  3. Execute the following command in your terminal to run the test:

forge test --match-test "testWinnerLossesSomeFunds"

Code

function testWinnerLossesSomeFunds() public playersEntered {
uint256 balanceBefore = address(playerFour).balance;
​
vm.warp(block.timestamp + duration + 1);
vm.roll(block.number + 1);
​
uint256 expectedPayout = (((entranceFee * 4) * 80) / 100);
/// @notice Actual result is (((1 wei * 4) * 80) / 100) == 3.2
/// @notice But Solidity returns 3
​
puppyRaffle.selectWinner();
assertEq(address(playerFour).balance, balanceBefore + expectedPayout);
assertEq(address(playerFour).balance, balanceBefore + 3);
}

Note that the winner receives 3 wei instead of 3.2 wei due to Solidity's handling of decimals.

Impact

Permanent Loss of Funds for Both the Protocol and the Winner

Tools Used

  • Foundry

Recommendations

To address the precision loss issue, consider using a multiplying factor to deal with rounding errors. Here's an example of how to implement this:

// Declare a constant MULTIPLY_FACTOR and set it to a high value
+ uint256 constant MULTIPLY_FACTOR = 1_000_000_000;
​
// Multiply every wei or ether value by MULTIPLY_FACTOR before performing calculations
// Divide every wei or ether value by MULTIPLY_FACTOR when sending ether/wei
​
function selectWinner() external {
// ...
+ uint256 totalAmountCollected = (players.length * entranceFee) * MULTIPLY_FACTOR;
uint256 prizePool = (totalAmountCollected * 80) / 100;
uint256 fee = (totalAmountCollected * 20) / 100;
totalFees = totalFees + uint64(fee);
​
// ...
+ (bool success, ) = winner.call{value: prizePool / MULTIPLY_FACTOR}("");
require(success, "PuppyRaffle: Failed to send the prize pool to the winner");
_safeMint(winner, tokenId);
}

Alternatively, you could set a lower bound for the allowed entranceFee to prevent precision loss.

Updates

Lead Judging Commences

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

loss of precision

like 1 wei

Support

FAQs

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

Give us feedback!