Submission Details
Severity:
constructor missing input validation checks
Summary
The PuppyRaffle::constructor doesn't check the input parameters and accept also 0 value. The input parameters should always be validated to prevent the creation/initialization of a contract in a wrong/inconsistent state.
Vulnerability Details
@> constructor(uint256 _entranceFee, address _feeAddress, uint256 _raffleDuration) ERC721("Puppy Raffle", "PR") {
entranceFee = _entranceFee;
feeAddress = _feeAddress;
raffleDuration = _raffleDuration;
raffleStartTime = block.timestamp;
...
}
Impact
Case 1: PuppyRaffle::_entranceFee = 0
// The players can enter the raffle without paying anything. The winner will get no prize (prize = 0).
function testCanEnterRaffle_Free() 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(puppyRaffle.players(0), playerOne);
assertEq(puppyRaffle.players(1), playerTwo);
assertEq(puppyRaffle.players(2), playerThree);
assertEq(puppyRaffle.players(3), playerFour);
uint256 balanceBefore = address(playerFour).balance;
vm.warp(block.timestamp + duration + 1);
vm.roll(block.number + 1);
uint256 expectedPayout = ((entranceFee * 4) * 80 / 100);
puppyRaffle.selectWinner();
assertEq(address(playerFour).balance, balanceBefore + expectedPayout);
console.log("The balance of playerFour:", address(playerFour).balance);
}
Case 2: PuppyRaffle::_feeAddress = 0x0000000000000000000000000000000000000000
//The collected fees are being transferred to a zero address, resulting in the loss of funds.
function testWithdrawFeesUsingZeroAddress() public playersEntered {
vm.warp(block.timestamp + duration + 1);
vm.roll(block.number + 1);
uint256 expectedFeeAmount = ((entranceFee * 4) * 20) / 100;
puppyRaffle.selectWinner();
puppyRaffle.withdrawFees();
assertEq(address(feeAddress).balance, expectedFeeAmount);
console.log("The fee address is:", address(feeAddress));
console.log("The fee address balance is:", address(feeAddress).balance);
}
Case 3: PuppyRaffle::_raffleDuration = 0
//If the _raffleDuration is set to 0, the raffle can be ended and a winner can be selected immediately after the raffle //starts and at least 4 players have entered.
function testCanSelectWinnerImmediatelyWhenDurationIsZero() public {
// Enter 4 players into the raffle
address[] memory players = new address[](4);
players[0] = playerOne;
players[1] = playerTwo;
players[2] = playerThree;
players[3] = playerFour;
puppyRaffle.enterRaffle{value: entranceFee * 4}(players);
// Get the current block number and timestamp
uint256 blockNumberBefore = block.number;
uint256 timestampBefore = block.timestamp;
// Try to select a winner immediately
puppyRaffle.selectWinner();
// Get the current block number and timestamp
uint256 blockNumberAfter = block.number;
uint256 timestampAfter = block.timestamp;
// Check that a winner has been selected
assertEq(puppyRaffle.previousWinner(), players[0]);
// Check that the block number is not changed
assertEq(blockNumberBefore, blockNumberAfter);
}
Tools Used
Manual review
Recommendations
Consider implementing the following changes:
constructor(uint256 _entranceFee, address _feeAddress, uint256 _raffleDuration) ERC721("Puppy Raffle", "PR") {
+ require(_entranceFee > 0, "PuppyRaffle: Entrance fee must be greater than 0");
+ require(_feeAddress != address(0), "PuppyRaffle: Fee address cannot be 0");
+ require(_raffleDuration > 0, "PuppyRaffle: Raffle duration must be greater than 0");
entranceFee = _entranceFee;
feeAddress = _feeAddress;
raffleDuration = _raffleDuration;
raffleStartTime = block.timestamp;
...
}
Rewards breakdown
nSLOC
143This contest has a flat reward structure with the following payouts:
100 EXP
20 EXP
2 EXP
Live
The contest is live. Earn rewards by submitting a finding.
Appeals
This is your time to appeal against judgements on your submissions.
Appeals Review
Appeals are being carefully reviewed by our judges.