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

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

lyes_boudjabout

Description

The function PuppyRaffle::changeFeeAddress assigns the newFeeAddress to feeAddress variable without checking the value of input:

function changeFeeAddress(address newFeeAddress) external onlyOwner {
@> feeAddress = newFeeAddress;
emit FeeAddressChanged(newFeeAddress);
}

the newFeeAddress 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 the owner mistakenly passes address(0) or an unintended invalid address during a call of that function. Since the function does not enforce validation, this is moderately likely during calls.

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 function parameters improves contract reliability and robustness.

Proof of Concept

Current constructor assignment:

feeAddress = newFeeAddress;

If newFeeAddress 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 newFeeAddress to ensure it is not the zero address.

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

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

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!