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

withdrawFees function does not need the current require statement

n4thedev01

Summary

The withdrawFees function does not need the current require statement.

Vulnerability Details

it does not allow user to withdraw funds from previous raffle, keep in mind this is only the case as soon as a new raffle is taking place and minimum 1 Entrant STARTs entering.

function testUnableToWithdrawFundsFromPreviousRaffles() external playersEntered {
vm.warp(block.timestamp + duration + 1);
vm.roll(block.number + 1);
​
puppyRaffle.selectWinner();
​
address[] memory players = new address[](1);
players[0] = playerOne;
puppyRaffle.enterRaffle{value: entranceFee}(players);
​
// trying to withdraw totalFees which is your 20% from all the previous raffles
vm.expectRevert("PuppyRaffle: There are currently players active!");
puppyRaffle.withdrawFees();
}

Impact

After a raffle has finished, as soon as a new raffle is taking place and entrants start entering, withdrawFunds will revert when called not allowing the owner of the contract to get his funds.

Tools Used

Manual Review, Foundry

Recommendations

removing the require statement will allow the withdrawFees function to be called whenever, there is no vulnerability in terms of users entrant fee's being stolen because totalFees is only set to their 20% of the contract balance when selectWinner is called.

function withdrawFees() external {
- require(address(this).balance == uint256(totalFees), "PuppyRaffle: There are currently players active!");
uint256 feesToWithdraw = totalFees;
totalFees = 0;
(bool success,) = feeAddress.call{value: feesToWithdraw}("");
require(success, "PuppyRaffle: Failed to withdraw fees");
}
function testOnlyFundsFromPreviousRafflesCanBeWithdrawn() external playersEntered {
vm.warp(block.timestamp + duration + 1);
vm.roll(block.number + 1);
​
uint256 expectedPrizeAmount = ((entranceFee * 4) * 20) / 100;
​
puppyRaffle.selectWinner();
​
address[] memory players = new address[](1);
players[0] = playerOne;
puppyRaffle.enterRaffle{value: entranceFee}(players);
​
puppyRaffle.withdrawFees();
assertEq(address(feeAddress).balance, expectedPrizeAmount);
}
Updates

Lead Judging Commences

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

greifers-send-money-to-contract-to-block-withdrawfees

Support

FAQs

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

Give us feedback!