Description
The PuppyRaffle constructor accepts entranceFee, feeAddress, and raffleDuration parameters without validating them. This allows deployment with invalid configurations that break core protocol functionality.
Expected behavior: Constructor should validate critical parameters to prevent misconfiguration at deployment.
Actual behavior: The constructor accepts zero values and zero addresses without any checks, leading to:
-
Zero entrance fee: Players can enter for free, generating no revenue
-
Zero address for fees: All protocol fees are permanently burned to address(0)
-
Zero duration: Winner can be selected immediately, defeating the purpose of a timed raffle
constructor(uint256 _entranceFee, address _feeAddress, uint256 _raffleDuration) ERC721("Puppy Raffle", "PR") {
entranceFee = _entranceFee;
feeAddress = _feeAddress;
raffleDuration = _raffleDuration;
raffleStartTime = block.timestamp;
}
Risk
Likelihood:
-
Low-Medium - Requires owner mistake during deployment
-
Could happen during testing or rushed deployment
-
No way to fix after deployment (immutable values)
Impact:
-
Zero entrance fee: Protocol generates no revenue
-
Zero address: All fees permanently lost (burned)
-
Zero duration: Raffle can be gamed by immediately selecting winner
-
No recovery possible after deployment
Proof of Concept
Add to test/PuppyRaffleTest.t.sol:
* @notice Demonstrates constructor accepts invalid inputs leading to broken protocol
* @dev Zero values for critical parameters break core functionality
*/
function test_constructorMissingValidation() public {
emit log("=== Test 1: Zero Entrance Fee ===");
PuppyRaffle zeroFeeRaffle = new PuppyRaffle(0, address(99), 1 days);
emit log_named_uint("Entrance fee", zeroFeeRaffle.entranceFee());
address[] memory players = new address[](1);
players[0] = address(10);
zeroFeeRaffle.enterRaffle{value: 0}(players);
emit log("Players can enter for FREE - no revenue!");
emit log("");
emit log("=== Test 2: Zero Address for Fees ===");
PuppyRaffle zeroAddressRaffle = new PuppyRaffle(1 ether, address(0), 1 days);
emit log_named_address("Fee address", zeroAddressRaffle.feeAddress());
emit log("All protocol fees will be BURNED to address(0)");
emit log("");
emit log("=== Test 3: Zero Duration ===");
PuppyRaffle zeroDurationRaffle = new PuppyRaffle(1 ether, address(99), 0);
emit log_named_uint("Duration", zeroDurationRaffle.raffleDuration());
address[] memory playersForZeroDuration = new address[](4);
for (uint256 i = 0; i < 4; i++) {
playersForZeroDuration[i] = address(uint160(i + 1));
}
vm.deal(address(1), 4 ether);
vm.prank(address(1));
zeroDurationRaffle.enterRaffle{value: 4 ether}(playersForZeroDuration);
zeroDurationRaffle.selectWinner();
emit log("Winner selected IMMEDIATELY (no waiting period)");
assertEq(zeroFeeRaffle.entranceFee(), 0);
assertEq(zeroAddressRaffle.feeAddress(), address(0));
assertEq(zeroDurationRaffle.raffleDuration(), 0);
}
Run: forge test --match-test test_constructorMissingValidation -vv --offline
Output:
=== Test 1: Zero Entrance Fee ===
Entrance fee: 0
Players can enter for FREE - no revenue!
=== Test 2: Zero Address for Fees ===
Fee address: 0x0000000000000000000000000000000000000000
All protocol fees will be BURNED to address(0)
=== Test 3: Zero Duration ===
Duration: 0
Winner selected IMMEDIATELY (no waiting period)
Recommended Mitigation
Add input validation in the constructor:
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 zero address");
+ require(_raffleDuration > 0, "PuppyRaffle: Duration must be greater than 0");
+
entranceFee = _entranceFee;
feeAddress = _feeAddress;
raffleDuration = _raffleDuration;
raffleStartTime = block.timestamp;
// ...
}
This prevents deployment with invalid configurations that would break the protocol.
