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

Prize Pool Calculation Ignores Refunded Players

365smile

Scope: src/PuppyRaffle.sol

Root + Impact

The selectWinner() function calculates totalAmountCollected based on players.length, not actual contract balance. When players refund, their ETH leaves the contract, but they're still counted in the calculation, leading to failed transfers.

Description

  • Normal behavior: The prize pool should be calculated based on actual funds available in the contract.

  • The issue: players.length includes refunded players (now address(0)), but their ETH has already been refunded. This causes prizePool to exceed available balance, making the transfer fail.

function selectWinner() external {
// @> players.length includes refunded players who got their ETH back
uint256 totalAmountCollected = players.length * entranceFee;
// @> prizePool assumes all players paid, but some refunded
uint256 prizePool = (totalAmountCollected * 80) / 100;
// @> Fails if prizePool > actual balance
(bool success,) = winner.call{value: prizePool}("");
require(success, "PuppyRaffle: Failed to send prize pool to winner");
}

Risk

Likelihood:

  • Occurs whenever any player refunds before winner selection

  • Refund functionality is a core feature of the protocol

  • Users are explicitly encouraged to refund per documentation

Impact:

  • selectWinner() reverts, blocking raffle completion

  • All players' funds stuck in contract

  • Protocol functionality completely broken

Proof of Concept

Explanation: The test enters 4 players (4 ETH), then 2 refund (leaving 2 ETH). When selectWinner() runs, it calculates prizePool as 3.2 ETH (4 players × 1 ETH × 80%), but only 2 ETH exists. The transfer fails because there's insufficient balance.

function testVuln8_IncorrectPrizeCalculation() public {
address[] memory players = new address[](4);
players[0] = playerOne;
players[1] = playerTwo;
players[2] = playerThree;
players[3] = playerFour;
puppyRaffle.enterRaffle{value: entranceFee * 4}(players);
​
assertEq(address(puppyRaffle).balance, 4 ether);
​
// 2 players refund - only 2 ETH remains
vm.prank(playerOne);
puppyRaffle.refund(0);
vm.prank(playerTwo);
puppyRaffle.refund(1);
​
assertEq(address(puppyRaffle).balance, 2 ether);
​
vm.warp(block.timestamp + duration + 1);
​
// Find valid winner (not address(0))
address validCaller;
for (uint160 i = 1; i < 1000; i++) {
address testCaller = address(i);
uint256 winnerIndex = uint256(
keccak256(abi.encodePacked(testCaller, block.timestamp, block.difficulty))
) % 4;
if (winnerIndex >= 2) {
validCaller = testCaller;
break;
}
}
​
// Reverts: tries to send 3.2 ETH but only 2 ETH available
vm.prank(validCaller);
vm.expectRevert("PuppyRaffle: Failed to send prize pool to winner");
puppyRaffle.selectWinner();
}

Recommended Mitigation

Explanation: Track active players separately and calculate prize pool based on actual balance minus accumulated fees.

+ uint256 public activePlayerCount;
​
function enterRaffle(address[] memory newPlayers) public payable {
+ activePlayerCount += newPlayers.length;
}
​
function refund(uint256 playerIndex) public {
+ activePlayerCount--;
}
​
function selectWinner() external {
- uint256 totalAmountCollected = players.length * entranceFee;
+ uint256 totalAmountCollected = activePlayerCount * entranceFee;
// Or simply use: address(this).balance - totalFees
Updates

Lead Judging Commences

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

[H-01] Potential Loss of Funds During Prize Pool Distribution

## Description In the `selectWinner` function, when a player has refunded and their address is replaced with address(0), the prize money may be sent to address(0), resulting in fund loss. ## Vulnerability Details In the `refund` function if a user wants to refund his money then he will be given his money back and his address in the array will be replaced with `address(0)`. So lets say `Alice` entered in the raffle and later decided to refund her money then her address in the `player` array will be replaced with `address(0)`. And lets consider that her index in the array is `7th` so currently there is `address(0)` at `7th index`, so when `selectWinner` function will be called there isn't any kind of check that this 7th index can't be the winner so if this `7th` index will be declared as winner then all the prize will be sent to him which will actually lost as it will be sent to `address(0)` ## Impact Loss of funds if they are sent to address(0), posing a financial risk. ## Recommendations Implement additional checks in the `selectWinner` function to ensure that prize money is not sent to `address(0)`

Support

FAQs

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

Give us feedback!