Puppy Raffle
AI First Flight #1
Beginner FriendlyFoundrySolidityNFT
EXP
View results
Previous Next
Submission Details
Severity: low
Valid

Precision loss in prizePool and fee calculation locks dust amounts

alaaeldinsabib

Summary

The selectWinner function calculates the prizePool (80%) and fee (20%) using integer division. This causes rounding down, meaning the sum of the distributed amounts can be less than the totalAmountCollected. The remaining "dust" ETH is permanently locked in the contract and never distributed to anyone.

Description

In Solidity, integer division truncates any fractional part. When calculating percentages, if totalAmountCollected is not perfectly divisible by 100 (after multiplying by 80 or 20), the remainder is lost. For example, if the total collected is 101 wei, the prize pool becomes 80 wei and the fee becomes 20 wei. The remaining 1 wei is permanently trapped in the contract. While small per raffle, this accumulates over time.

Root Cause

File: src/PuppyRaffle.sol (lines 132-134)

uint256 totalAmountCollected = players.length * entranceFee;
uint256 prizePool = (totalAmountCollected * 80) / 100; // ❌ Truncates remainder
uint256 fee = (totalAmountCollected * 20) / 100; // ❌ Truncates remainder

Risk

Severity: Low
Likelihood: High
Impact: Low

  • ❌ Small amounts of ETH (dust) are permanently locked

  • ❌ Users and the protocol lose funds over time

  • ✅ Impact is minimal if entranceFee is a multiple of 100 wei

Proof of Concept

Scenario: A raffle collects an amount that is not divisible by 100, causing precision loss during the 80/20 split.

function test_PrecisionLossLocksFunds() public {
// Simulate a total collection that causes precision loss
// e.g., 101 wei total (could happen with weird entrance fees or specific player counts)
uint256 totalAmountCollected = 101;
uint256 prizePool = (totalAmountCollected * 80) / 100;
uint256 fee = (totalAmountCollected * 20) / 100;
uint256 distributed = prizePool + fee;
uint256 lockedDust = totalAmountCollected - distributed;
console.log("Total collected:", totalAmountCollected);
console.log("Prize pool:", prizePool);
console.log("Fee:", fee);
console.log("Total distributed:", distributed);
console.log("Dust permanently locked:", lockedDust);
assertEq(lockedDust, 1, "1 wei is permanently locked!");
console.log("VULNERABILITY: Integer division causes permanent fund locking!");
}

Test Output:

Total collected: 101
Prize pool: 80
Fee: 20
Total distributed: 100
Dust permanently locked: 1
VULNERABILITY: Integer division causes permanent fund locking!

What This Proves:

  1. ✅ Integer division truncates remainders

  2. ✅ Sum of prize and fee is less than total collected

  3. ✅ Dust amounts are permanently locked in the contract

Recommended Mitigation

Calculate one of the values using division, and calculate the other by subtracting from the total. This ensures 100% of the funds are distributed.

function selectWinner() external {
// ... validation ...
uint256 totalAmountCollected = players.length * entranceFee;
// ✅ Calculate fee first, then prize gets the exact remainder
uint256 fee = (totalAmountCollected * 20) / 100;
uint256 prizePool = totalAmountCollected - fee;
// OR calculate prize first, fee gets the remainder:
// uint256 prizePool = (totalAmountCollected * 80) / 100;
// uint256 fee = totalAmountCollected - prizePool;
​
totalFees = totalFees + uint64(fee);
// ... rest of logic
}

Why This Fixes It:

  1. ✅ Ensures prizePool + fee == totalAmountCollected exactly

  2. ✅ Prevents any dust amounts from being locked

  3. ✅ 100% of user funds are properly distributed

References

  • CWE-682: Incorrect Calculation

  • Solidity Integer Arithmetic Best Practices

Updates

Lead Judging Commences

ai-first-flight-judge Lead Judge about 8 hours ago
Submission Judgement Published
Validated
Assigned finding tags:

[L-06] Fee should be 'totalAmountCollected-prizePool' to prevent decimal loss

## Description `fee` should be 'totalAmountCollected-prizePool' to prevent decimal loss ## Vulnerability Details ``` uint256 totalAmountCollected = players.length * entranceFee; uint256 prizePool = (totalAmountCollected * 80) / 100; uint256 fee = (totalAmountCollected * 20) / 100; ``` This formula calculates `fee` should be 'totalAmountCollected-prizePool' ## Impact By calculates `fee` like the formula above can cause a loss in `totalAmountCollected' if the `prizePool` is rounded. ## Recommendations ```diff - uint256 fee = (totalAmountCollected * 20) / 100; + uint256 fee = totalAmountCollected-prizePool; ```

Support

FAQs

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

Give us feedback!