Puppy Raffle
AI First Flight #1
Beginner FriendlyFoundrySolidityNFT
EXP
View results
Previous Next
Submission Details
Impact: low
Likelihood: medium
Invalid

[L-01] Unvalidated Fee Address Assignment (Operational Issue)

lyes_boudjabout

Description

The contract constructor initializes the feeAddress variable without checking the input that is being assigned to it:

constructor(
uint256 _entranceFee,
address _feeAddress,
uint256 _raffleDuration
) ERC721("Puppy Raffle", "PR") {
entranceFee = _entranceFee;
@> feeAddress = _feeAddress;
raffleDuration = _raffleDuration;
raffleStartTime = block.timestamp;
​
rarityToUri[COMMON_RARITY] = commonImageUri;
rarityToUri[RARE_RARITY] = rareImageUri;
rarityToUri[LEGENDARY_RARITY] = legendaryImageUri;
​
rarityToName[COMMON_RARITY] = COMMON;
rarityToName[RARE_RARITY] = RARE;
rarityToName[LEGENDARY_RARITY] = LEGENDARY;
}

the _feeAddress parameter is assigned directly to the feeAddress storage variable without validation. This means that a zero address or invalid address could be set, potentially causing fees to be lost or transferred into an unattended address, making the contract unusable.

Risk

  • Severity: Low

  • Type: Input Validation / Correctness

  • Impact: Assigning an invalid address could lead to operational issues, such as fees being sent to the zero address or other unintended behavior.

Likelihood

  • Probability of Occurrence: Medium

  • This issue can occur if a developer or deployer mistakenly passes address(0) or an unintended invalid address during deployment. Since the constructor does not enforce validation, this is moderately likely in manual deployments or test scenarios.

Impact

  • Operational correctness: Invalid feeAddress may break fee transfers.

  • Maintainability: Future developers may assume the address is always valid, introducing subtle bugs.

  • Best Practices: Input validation on constructor parameters improves contract reliability and robustness.

Proof of Concept

Current constructor assignment:

feeAddress = _feeAddress;

If _feeAddress is address(0), any fee transfers will be burnt, and therefore lost forever. For example here:

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");
}

Here, the feeAddress is used as a transfer destination, therefore if feeAddress == address(0), the fees will be burnt and lost forever.

Recommended Mitigation

  • Add input validation for _feeAddress to ensure it is not the zero address.

+ error PuppyRaffle_InvalidFeeAddress(); // define the error outside the constructor
​
+ if(_feeAddress == address(0)) {
+ revert PuppyRaffle_InvalidFeeAddress();
+ }

  • Optionally, add similar validation for other critical constructor parameters if needed (e.g., `_entranceFee > 0`).

  • Maintain consistency with other initialization logic to ensure robust contract deployment.

Updates

Lead Judging Commences

ai-first-flight-judge Lead Judge about 4 hours ago
Submission Judgement Published
Invalidated
Reason: Incorrect statement

Support

FAQs

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

Give us feedback!