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

Wrong player.length will cause a revert in the `selectWinner` function.

pratred

Summary

If a player refunds , then the contract will revert when the selectWinner function is called.

Vulnerability Details

Suppose there are 4 players in the raffle and the entry fee is 1ETH. So now the contract will have 4ETh as balance. Now if player0 decides to refund the contract will give 1ETH to player0 and thus the contract balance is now 3ETH.And as the players.length is 4 still now the selectWinner function can be called.

uint256 totalAmountCollected = players.length * entranceFee;// uint256 totalAmountCollected = 4 * 1;

So as per the contract the totalAmountCollected will now be 4ETh but the balance of the SC is 3ETH. Therefore while transferring 80% of 4ETH i.e 3.2ETH the function will revert.

Impact

The selectWinner function will revert.

Poc

NOTE : I have created a playerLen() view function in the contract to get the players array length.

function testSelectWinnerFunctionRevertsWhenPlayerRefunnd() public playersEntered {
vm.prank(playerOne);
puppyRaffle.refund(0);
​
uint plyLenAft = puppyRaffle.playersLen();//NOTE: I have created a playerLen() view function in the contract.
​
assert(plyLenAft==4);//player is refunded and contract balance is now 3ETH but still player.length is 4.Due to this the contract will try to send more amount than it has and it will revert.
vm.warp(block.timestamp + duration + 1);
vm.roll(block.number + 1);
vm.expectRevert("PuppyRaffle: Failed to send prize pool to winner");
puppyRaffle.selectWinner();
}

Tools Used

VS code

Recommendations

Use a counter variable to calculate the length of the players.

Updates

Lead Judging Commences

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

refund-doesnt-reduce-players-array-size-causing-protocol-to-freeze

zero address can win the raffle

Funds are locked to no one. If someone gets the refund issue, they also got this issue. IMPACT: High Likelihood: High

Support

FAQs

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