Puppy Raffle
AI First Flight #1
Beginner FriendlyFoundrySolidityNFT
EXP
View results
Previous Next
Submission Details
Impact: high
Likelihood: high
Invalid

Prize Pool Calculation Doesn't Account for Refunded Players

muzammilmohammed678

Root + Impact

Description

  • * The `selectWinner()` function calculates `totalAmountCollected` using `players.length * entranceFee`, which includes refunded players (slots set to `address(0)`).

    * When players refund, they receive their `entranceFee` back, but their slot remains in the array. The calculation assumes funds exist that were already returned, leading to incorrect prize pool and fee calculations.

    ```solidity:131:134:src/PuppyRaffle.sol

    uint256 totalAmountCollected = players.length * entranceFee;

    uint256 prizePool = (totalAmountCollected * 80) / 100;

    uint256 fee = (totalAmountCollected * 20) / 100;

    totalFees = totalFees + uint64(fee);

    ```


Risk

Likelihood:

  • * This occurs whenever any player has refunded before `selectWinner()` is called

    * Refunds are allowed until the raffle ends, so this is a common scenario

    * The calculation uses `players.length` which doesn't decrease when players refund

Impact:

  • * Prize pool calculation assumes more funds than actually exist in the contract

    * Contract may attempt to send more ETH than it has, causing transaction to revert

    * Fee calculation is also incorrect, leading to accounting errors

    * Potential for contract drain if calculations are off

    * Winners may receive incorrect prize amounts

Proof of Concept

```solidity
// 4 players enter: Contract has 4 * entranceFee
address[] memory players = new address[](4);
players[0] = alice;
players[1] = bob;
players[2] = charlie;
players[3] = dave;
puppyRaffle.enterRaffle{value: entranceFee * 4}(players);
// Contract balance: 4 * entranceFee
​
// Alice refunds
puppyRaffle.refund(0);
// Contract balance: 3 * entranceFee
// But players.length is still 4
​
// selectWinner() calculates:
uint256 totalAmountCollected = 4 * entranceFee; // Wrong! Only 3 * entranceFee exists
uint256 prizePool = (4 * entranceFee * 80) / 100; // Tries to send more than contract has
// Transaction may revert or send incorrect amounts
```

Recommended Mitigation

```diff
function selectWinner() external {
require(block.timestamp >= raffleStartTime + raffleDuration, "PuppyRaffle: Raffle not over");
require(players.length >= 4, "PuppyRaffle: Need at least 4 players");
+
+ // Count only active (non-zero) players
+ uint256 activePlayers = 0;
+ for (uint256 i = 0; i < players.length; i++) {
+ if (players[i] != address(0)) {
+ activePlayers++;
+ }
+ }
+ require(activePlayers >= 4, "PuppyRaffle: Need at least 4 active players");
+
uint256 winnerIndex = uint256(keccak256(abi.encodePacked(msg.sender, block.timestamp, block.difficulty))) % players.length;
address winner = players[winnerIndex];
- uint256 totalAmountCollected = players.length * entranceFee;
+ uint256 totalAmountCollected = activePlayers * entranceFee;
uint256 prizePool = (totalAmountCollected * 80) / 100;
uint256 fee = (totalAmountCollected * 20) / 100;
// ... rest of function
}
```
Updates

Lead Judging Commences

ai-first-flight-judge Lead Judge 16 days ago
Submission Judgement Published
Invalidated
Reason: Incorrect statement

Support

FAQs

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

Give us feedback!